fix: add count loading spinners to dynamic tables#2449
Open
abdimo101 wants to merge 9 commits into
Open
Conversation
…roposal details page
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
ProposalEffects.fetchRelatedProposals$, the innermergeMapthat returns an array of actions isn’t needed—usingmaphere keeps the stream flatter and easier to read while still allowing multiple actions to be dispatched. - The new
isLoadingpropagation into pagination configs is duplicated across files, instruments, samples, proposals, and related proposals components; consider extracting a shared helper for buildingTablePaginationobjects with the loading flag to keep this logic consistent and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ProposalEffects.fetchRelatedProposals$`, the inner `mergeMap` that returns an array of actions isn’t needed—using `map` here keeps the stream flatter and easier to read while still allowing multiple actions to be dispatched.
- The new `isLoading` propagation into pagination configs is duplicated across files, instruments, samples, proposals, and related proposals components; consider extracting a shared helper for building `TablePagination` objects with the loading flag to keep this logic consistent and easier to maintain.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR introduces count-specific loading state across several NgRx feature areas and threads that state through selectors into dynamic table pagination configs so the UI can show a spinner while count requests are pending (mirroring existing facet-loading spinner patterns).
Changes:
- Add
*CountIsLoadingflags to feature states (samples/files/instruments/proposals) and initialize them in the corresponding stores. - Update reducers/effects/selectors to set/reset these flags for count request lifecycles (including a new related-proposals count trigger action).
- Pass
isLoadingintoTablePaginationconfigs in relevant components so dynamic tables can reflect count-loading state.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/state-management/state/samples.store.ts | Adds loading flags for samples and related datasets count requests. |
| src/app/state-management/state/proposals.store.ts | Adds loading flags for proposal datasets count and related proposals count. |
| src/app/state-management/state/instruments.store.ts | Adds loading flag for instruments count requests. |
| src/app/state-management/state/files.store.ts | Adds loading flag for files count requests. |
| src/app/state-management/selectors/samples.selectors.ts | Exposes count loading selectors and includes isLoading in samples view models. |
| src/app/state-management/selectors/proposals.selectors.ts | Exposes count loading selectors and includes isLoading in proposal datasets + related proposals view models. |
| src/app/state-management/selectors/instruments.selectors.ts | Includes count loading selector and returns isLoading in dashboard selector output. |
| src/app/state-management/selectors/files.selectors.ts | Includes count loading selector and returns isLoading in dashboard selector output. |
| src/app/state-management/reducers/samples.reducer.ts | Sets/resets samples and sample-datasets count loading flags on fetch/complete/fail. |
| src/app/state-management/reducers/proposals.reducer.ts | Sets/resets proposal-datasets and related-proposals count loading flags on fetch/complete/fail. |
| src/app/state-management/reducers/instruments.reducer.ts | Sets/resets instruments count loading flag on fetch/complete/fail. |
| src/app/state-management/reducers/files.reducer.ts | Sets/resets files count loading flag on fetch/complete/fail. |
| src/app/state-management/effects/proposals.effects.ts | Dispatches a dedicated related-proposals count fetch after related-proposals list completes; count effect now listens to that count action. |
| src/app/state-management/actions/proposals.actions.ts | Adds fetchRelatedProposalsCountAction to trigger related proposals count fetching explicitly. |
| src/app/samples/sample-detail/sample-detail.component.ts | Passes count-loading state into datasets table pagination config. |
| src/app/samples/sample-dashboard/sample-dashboard.component.ts | Passes count-loading state into samples table pagination config and updates pagination helper signature. |
| src/app/proposals/related-proposals/related-proposals.component.ts | Passes count-loading state into related proposals table pagination config. |
| src/app/proposals/proposal-datasets/proposal-datasets.component.ts | Passes count-loading state into proposal datasets table pagination config. |
| src/app/instruments/instruments-dashboard/instruments-dashboard.component.ts | Passes count-loading state into instruments table pagination config and updates pagination helper signature. |
| src/app/files/files-dashboard/files-dashboard.component.ts | Passes count-loading state into files table pagination config and updates pagination helper signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds loading spinners for count requests in the samples, files, instruments, samples related datasets, proposals related proposals, and proposals datasets tables. The spinner appears while the count request is pending, following the same pattern used for facet loading spinners for datasets and proposals tables.
Motivation
Background on use case, changes needed
Fixes:
Please provide a list of the fixes implemented in this PR
Changes:
Please provide a list of the changes implemented by this PR
Tests included
Documentation
official documentation info
If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included
Backend version
Summary by Sourcery
Add loading-state support for count requests across dynamic tables to enable count-specific loading spinners.
New Features:
Bug Fixes:
Enhancements: