Add paginator presenter #924
Merged
Conversation
This file contains 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
https://eaflood.atlassian.net/browse/WATER-4445 > Part of a series of changes to migrate the legacy view bill runs page into this project The legacy view bill runs page has its own custom pagination control. We understand why, the [pagination component](https://design-system.service.gov.uk/components/pagination/) didn't exist in the design system at the time. But now we have the component there is no need to migrate the custom one built. We wouldn't want to! One of the key reasons to look at migrating the bill runs page is because we have other pages that need updating that also use some form of custom pagination. If we can crack how to implement the GOV.UK component we have a solution for all of them. This `PaginatorPresenter` we're adding will build the 'object' that has to be passed to `govukPagination()` in the view. There are a number of scenarios it has to consider, for example - when there are not enough results to warrant pagination - whether to show the previous or next controls - when to show the ellipsis to highlight skipped pages To do this all it needs are - the total number of records for the thing being paginated - how many results to show per page - what page of results is selected The presenter we've built is intended to be used by all pages that need the component, not just bill runs. Because of this we've tried to make sure the testing and documentation covers all possible scenarios.
Cruikshanks
requested review from
Demwunz,
robertparkinson,
Jozzey,
Beckyrose200 and
rvsiyad
robertparkinson
previously approved these changes
Apr 22, 2024
Jozzey
reviewed
Apr 22, 2024
| const component = { items } | ||
|
|
||
| if (selectedPageNumber !== 1) { | ||
| component.previous = { href: `/system/bill-runs?page=${selectedPageNumber - 1}` } |
There was a problem hiding this comment.
More of a question really. If the intention is to reuse this presenter on other pages that require this component, should we be hardcoding the URL /system/bill-runs? I guess currently it's fine as it is the only page that uses this presenter. But is it worth setting up the URL as an additional parameter now, or is that something to do if/when we need to?
There was a problem hiding this comment.
🎯 Brilliant spot. Yes, I should be making this a param.
Jozzey
previously approved these changes
Apr 22, 2024
Default is now 25 plus there was a typo in the example.
Thanks to a great spot by @Jozzey we realised that the paginator, though intended for reuse, had hard code the links to be `/system/bill-runs`. If other pages are to be able to reuse it that path must be provided as an arg. This change updates the presenter. The tests will fail but we wanted to confirm that before fixing them.
Cruikshanks
dismissed stale reviews from Jozzey and robertparkinson
via
0033599
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.
https://eaflood.atlassian.net/browse/WATER-4445
The legacy view bill runs page has a custom pagination control. We understand why, the pagination component didn't exist in the design system at the time it was built. But now we have the component there is no need to migrate the custom one.
One of the key reasons to look at migrating the bill runs page is because we have other pages that need updating that also use a form of custom pagination. If we can crack how to implement the GOV.UK component we have a solution for all of them.
This
PaginatorPresenterwe're adding will build the 'object' that has to be passed togovukPagination()in the nunjucks view. There are several scenarios it has to consider, for exampleTo do this all it needs are
The presenter we've built is intended to be used by all pages that need the component, not just bill runs. Because of this, we've tried to ensure the testing and documentation cover all possible scenarios.