-
Notifications
You must be signed in to change notification settings - Fork 4.9k
feat: Persistent Page Size UI #21627
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
Conversation
Signed-off-by: bupd <[email protected]>
Signed-off-by: bupd <[email protected]>
Signed-off-by: bupd <[email protected]>
Signed-off-by: bupd <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #21627 +/- ##
==========================================
+ Coverage 45.36% 46.37% +1.00%
==========================================
Files 244 253 +9
Lines 13333 14242 +909
Branches 2719 2928 +209
==========================================
+ Hits 6049 6605 +556
- Misses 6983 7286 +303
- Partials 301 351 +50
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
* update all other pages to have same size factors Signed-off-by: bupd <[email protected]>
'PAGINATION.PAGE_SIZE' | translate | ||
}}</clr-dg-page-size> | ||
<clr-dg-page-size | ||
[clrPageSizeOptions]="[15, 25, 50, 100]" |
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.
I see a lots of same array in the following pages [15, 25, 50, 100]
I am just wondering if we can define a const variable to keep this static array [15, 25, 50, 100]
Then other pages can import it into its own component page or may be create a BasePage which can be extends by other components.
By do it, we can easily to control and update the page number list in one place in the future if needed.
What do you think?
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.
Sure that's a good idea.
ready to merge |
Signed-off-by: bupd <[email protected]>
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.
LGTM
Thank you for contributing to Harbor!
Comprehensive Summary of your change
15, 25, 50, 100
Issue being fixed
Fixes #21117
LocalStorage of Browser Before Changes
LocalStorage of Browser After Changes
Please indicate you've done the following: