Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Provide external loading triggers #2533

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

aneeshsharma
Copy link
Contributor

Description

Any changes that were external to the table that would trigger a new data fetch, couldn't be accounted for when displaying the loading state in the table. Also, since the data source is external and a stream, it is not possible for the table to know when a new data fetch might have been triggered.

So, added loadingTrigger: Observable to the TableDataSource as an optional field, which can be provided by the user which should emit whenever a change happens that will trigger a new data fetch.

This will require the user to provide the trigger but I couldn't find any other way for the table to know by itself if a new data fetch is going to be triggered.

@aneeshsharma aneeshsharma requested a review from a team as a code owner November 21, 2023 10:01
Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (e743904) 71.26% compared to head (9967e6f) 81.92%.
Report is 8 commits behind head on main.

Files Patch % Lines
projects/components/src/table/table.component.ts 66.66% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2533       +/-   ##
===========================================
+ Coverage   71.26%   81.92%   +10.65%     
===========================================
  Files         927      928        +1     
  Lines       20870    20908       +38     
  Branches     3313     3324       +11     
===========================================
+ Hits        14874    17129     +2255     
+ Misses       5749     3641     -2108     
+ Partials      247      138      -109     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Nov 21, 2023

Test Results

       4 files  ±0     316 suites  ±0   32m 9s ⏱️ +12s
1 135 tests ±0  1 135 ✔️ ±0  0 💤 ±0  0 ±0 
1 145 runs  ±0  1 145 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 9967e6f. ± Comparison against base commit e743904.

♻️ This comment has been updated with latest results.

@aneeshsharma
Copy link
Contributor Author

Instead of passing the loadingTrigger$ as metadata to the data source, we can now pass the trigger as input to the table component itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant