-
Notifications
You must be signed in to change notification settings - Fork 51
Showcase: convert Pagination page to gts #3283
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
12ada6d to
f45b8ed
Compare
showcase/app/components/page-components/pagination/sub-sections/base-elements.gts
Show resolved
Hide resolved
showcase/app/components/page-components/pagination/code-fragments/with-numbered-and-events.gts
Show resolved
Hide resolved
| onTableSort = (sortBy: string, sortOrder: HdsTableThSortOrder) => { | ||
| this.currentSortBy = sortBy; | ||
| this.currentSortOrder = sortOrder; | ||
| // should we reset the selected page? |
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.
[Note] I think it makes sense to reset it, but also fine to leave it as is since that's what's been done currently.
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.
Yeah, I agree we should reset it. @didoo do you have thoughts? This comment was brought over from the existing pagination showcase page.
showcase/app/components/page-components/pagination/code-fragments/with-compact-and-events.gts
Show resolved
Hide resolved
showcase/app/components/page-components/pagination/code-fragments/with-compact-and-events.gts
Outdated
Show resolved
Hide resolved
showcase/app/components/page-components/pagination/code-fragments/with-compact-and-events.gts
Show resolved
Hide resolved
showcase/app/components/page-components/pagination/code-fragments/with-compact-and-routing.gts
Outdated
Show resolved
Hide resolved
|
Approved the Percy diff, as it looked all as expected 🟢 |
e8ea7bf to
7fd0728
Compare
📌 Summary
If merged, this PR would convert the pagination showcase page to gts.
This page is one of the few examples where we need to keep the
controllerfile because it usesqueryParamsin 2 of the demos. An unexpected benefit of breaking up the page into sub-components means these demos are actually more in line with how consumers implement these UIs.The source of truth for the state is in the URL, the controller only contains the list of queryParams + tracked copy of the param. Then in the component, when something (like page size) changes, you transition to the same URL and update the appropriate params which causes the tracked state in the controller to change and then makes the component re-render.
Passing the controller to the sub components is the easiest way to get the current state of the query params to the appropriate place.
Note: I had to update the acceptance tests a bit to match the new query param names + ids.
🔗 External links
Jira ticket: HDS-5348
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.
📋 PCI review checklist
Examples of changes to controls include access controls, encryption, logging, etc.
Examples include changes to operating systems, ports, protocols, services, cryptography-related components, PII processing code, etc.