-
Notifications
You must be signed in to change notification settings - Fork 35
refactor: proposal-dashboard table refactor before replacing the instruments table #1791
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
refactor: proposal-dashboard table refactor before replacing the instruments table #1791
Conversation
src/app/proposals/proposal-dashboard/proposal-dashboard.component.ts
Outdated
Show resolved
Hide resolved
bpedersen2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except for one minor remark this looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @martin-trajanovski - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider extracting the logic inside
getTablePaginationConfigandgetTableSortinto a shared utility function since they are very similar. - It looks like you're subscribing to
this.route.queryParamsand then immediately dispatching an action - consider usingswitchMapto combine these operations.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@bpedersen2 can you check this once more if you have some time as the typo that you pointed out is fixed now? |
1e38d77 to
6a42cf7
Compare
Description
Refactors the proposal dashboard component to improve data fetching and table configuration. It enhances the component's responsiveness to route parameters for pagination and search, and streamlines the table initialization process.
Motivation
There was quite much repetitive logic in the proposal-dashboard component for the table itself that could be simplified.
Changes:
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
Refactors the proposal dashboard component to improve data fetching and table configuration. It enhances the component's responsiveness to route parameters for pagination and search, and streamlines the table initialization process.
Enhancements:
Summary by Sourcery
Refactors the proposal dashboard component to improve data fetching and table configuration. This change enhances the component's responsiveness to route parameters for pagination and search and streamlines the table initialization process by subscribing to route changes and dispatching actions accordingly.
Enhancements: