alphagov / govuk-frontend

GOV.UK Frontend contains the code you need to start building a user interface for government platforms and services.
https://frontend.design-system.service.gov.uk/
MIT License
1.14k stars 320 forks source link

Pagination component numbering issue on small screens #3324

Open jackheslop96 opened 1 year ago

jackheslop96 commented 1 year ago

Description of the issue

For smaller screen sizes the pagination numbering stops obeying the rules as defined here.

CleanShot 2023-02-22 at 09 44 50@2x

Steps to reproduce the issue

This issue can be reproduced by going to https://www.staging.tax.service.gov.uk/auth-login-stub/gg-sign-in?continue=/manage-transit-movements/draft-declarations using enrolment:

For a smaller screen size you will see the behaviour as demonstrated in the above screenshot.

Actual vs expected behaviour

In the above screenshot I would expect to see

Environment (where applicable)

Additional information

See the following thread in Slack for more info: https://hmrcdigital.slack.com/archives/CJMUM9AG3/p1676914677343619

oscarduignan commented 1 year ago

alternative steps to reproduce:

  1. have a pagination component with more than 3 pages, where the current page is the 3rd page, for example https://design-system.service.gov.uk/components/pagination/last-page/index.html
  2. shrink viewport to mobile size
  3. the second pagination item will be hidden, but since there are no extra items between the first pagination item and the immediate sibling before the current one, there are no ellipsis between them so it shows [1] [3] rather than the [1] ... [50] (that it would show if there was an ellipsis if current page was 50) - the pagination item immediately after the first page should probably not be hidden on a small screen (whether it's an ellipsis or normal pagination item, so it would show [1] [2] [3] instead of [1] [3])
querkmachine commented 1 year ago

Thank you both for bringing this to our attention.

This seems to be an oversight on our part. Our code expects the ellipsis to be present in the HTML on all breakpoints, and doesn't anticipate it only being needed on mobile views. I imagine the same will happen if the 'current' page is third-from-last, too.

querkmachine commented 1 year ago

We've given this some pokes, however we've come to the conclusion that we can't satisfactorily fix this issue without introducing a breaking change into the component. As such we're going to push this out until at least the v5 release.

querkmachine commented 1 year ago

I've been giving this a little ponder and I think I may have come upon a possible solution, but it's one that's a fairly drastic change from what we currently have.

Instead of Nunjucks macro users having to explicitly define the paging numbers and ellipses; we 'automate' how the pagination is generated.

For example, given a configuration akin to this:

{{ govukPagination({
  totalPages: 15,
  currentPage: 6,
  nearestPagesToCurrent: 2
}) }}

We could programatically create a pagination that looks like this:

← Previous | 1 | … | 4 | 5 | 6 | 7 | 8 | … | 15 | Next →

As we would be creating the items in code, we would have the ability to contextually include or exclude items, or assign features to each item based on what we want them to do. For example, we could add a class to '4' and '8' in this example to hide them on narrow viewports, or add a class to the ellipsis if we only need it on narrow viewports.

We can do this more smartly because we have context for the whole pagination, not just the subset of pages that a service team has set in the component.

It does present its own issues: if items are not being set explicitly then neither can their links. We could do something fancy with placeholders—like we do on the character count's textarea description, where we use the Nunjucks replace filter to inject a value into a configured string—but there's no guarantee that the page number is what a team actually needs in that context.

A middle-ground solution might be to add 'flag' parameters for each item, allowing for the aforementioned show/hide on narrow viewport functionality but requiring teams to explicitly define them. In this situation we'd need to remove the current CSS that automatically hides all items except for first/last/current.

Do either of these ideas sound worth prototyping?

timsb commented 1 year ago

Before the pagination component was released in govuk-frontend we were working on our own component in hmrc-frontend. We added some of the automation logic that you describe above as we thought it would stop service teams having to repeatedly write their own logic. Our code can be found at https://github.com/hmrc/hmrc-frontend/tree/pagination/src/components/pagination. We were working on it when the govuk-frontend component was released so it's still a WIP and some of the template code is a little messy. I believe there's still some edge case bugs where the truncation wasn't quite working but I wanted to share in case it is useful.

We based this on the ONS pagination component which can be found at https://service-manual.ons.gov.uk/design-system/components/pagination

querkmachine commented 1 year ago

@timsb That's given me some ideas for how to mash our current and possibly more 'automated' approaches together. Thanks for sharing!