Skip to content
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

Add pagination component #2610

Merged
merged 3 commits into from
Jun 20, 2022
Merged

Add pagination component #2610

merged 3 commits into from
Jun 20, 2022

Conversation

owenatgov
Copy link
Contributor

@owenatgov owenatgov commented May 3, 2022

What

Adds the pagination component to govuk-frontend. See below epic for background on this addition .

Related issues/pull requests

Visuals

Standard view

Screenshot 2022-05-19 at 16 37 45

Standard view item hover state

Screenshot 2022-05-19 at 16 38 16

Standard view item focus state

Screenshot 2022-05-19 at 17 11 48

Standard view previous/next link focus state

Screenshot 2022-05-19 at 17 11 41

Standard view current item hover state

Screenshot 2022-05-19 at 16 38 25

Standard small screen view

Screenshot 2022-05-19 at 17 19 19

Standard view with a large number of pages

Teams have the option to hide several page numbers behind ellipses
Screenshot 2022-05-03 at 16 59 46

Without numbers

Alternative "block" view for pre/next navigation without page numbers
Screenshot 2022-05-19 at 16 38 34

Block view hover state

Screenshot 2022-05-19 at 16 38 39

Block view with labels

Screenshot 2022-05-19 at 16 38 45

Block view with labels hover state

Screenshot 2022-05-19 at 16 38 52

Block view with labels focus state

Screenshot 2022-05-19 at 17 12 07

@edwardhorsford
Copy link
Contributor

Is there a review app for this? From the screenshots it looks like there might be errant whitespace in the previous and next links - the underlines look oddly sized for the text content.

@querkmachine
Copy link
Member

The title and label lines on the "block" view are also not aligned with one another, possibly due to unruly whitespace again.

image

@edwardhorsford We don't have review apps at the moment due to the GitHub/Heroku outage. You need to pull the branch and run it locally for the time being.

@edwardhorsford
Copy link
Contributor

@querkmachine that's out of alignment too - but what I was talking about was this:
Screenshot 2022-05-04 at 13 28 01

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @owenatgov – looking really good so far. I have added quite a few comments but most of them are relatively minor. Very happy to talk through it if that'd be useful.

src/govuk/components/pagination/_index.scss Outdated Show resolved Hide resolved
src/govuk/components/pagination/_index.scss Show resolved Hide resolved
src/govuk/components/pagination/_index.scss Outdated Show resolved Hide resolved
src/govuk/components/pagination/_index.scss Outdated Show resolved Hide resolved
src/govuk/components/pagination/_index.scss Outdated Show resolved Hide resolved
src/govuk/components/pagination/_index.scss Outdated Show resolved Hide resolved
src/govuk/components/pagination/_index.scss Outdated Show resolved Hide resolved
src/govuk/components/pagination/pagination.yaml Outdated Show resolved Hide resolved
@owenatgov owenatgov force-pushed the add-pagination-component branch from 365f2fb to 349639a Compare May 9, 2022 13:54
@christopherthomasdesign
Copy link
Member

Just had a look through, a few more comments...

  1. +1 to Ed's question about the whitespace before 'Previous' and after 'Next' – is this a workaround for something? It's quite noticeable that there's an extra space in there, it should deffo be flush with the line below
  2. Should the bold 'current page' numbers have a hover state? To me that indicates you can interact with them...
  3. Think the pagination link labels might be a bit small – IMO the bold of the 'previous / next' bit is enough of a visual differentiator between the two. So I'd probs bump up the govuk-pagination__link-label to 19px on bigger screens to match the rest of the link. GOV.UK do it this way too, see https://www.gov.uk/adoption-records. If we do this, it might help to add an extra bit of spacing between the two rows so they're not bunched up too closely – no more than 5px

@owenatgov owenatgov force-pushed the add-pagination-component branch from 349639a to 020448c Compare May 10, 2022 11:31
@edwardhorsford
Copy link
Contributor

I suspect the whitespace issues are caused by the new lines. That can likely be fixed using Nunjucks whitespace control.

@owenatgov owenatgov force-pushed the add-pagination-component branch from 020448c to ca83550 Compare May 11, 2022 08:40
@owenatgov
Copy link
Contributor Author

@edwardhorsford was spot on that this was a whitespace issue. All sorted now thanks to Ed's helpful link and some light flexbox styling (thanks again @36degrees).

@christopherthomasdesign I believe I've covered all your points. Please take another look at let me know if you spot anything else.

I've reorganised the sass so it's a bit more sensible and tidy so some of the comments above will be listed as outdated even though they're still relevant. I'll still work through them in order.

@owenatgov owenatgov force-pushed the add-pagination-component branch 4 times, most recently from 59a59a2 to 7936cad Compare May 11, 2022 09:47
@EoinShaughnessy
Copy link
Contributor

Macro options still need to be 2i'd

@EoinShaughnessy
Copy link
Contributor

@owenatgov Macro options have passed 2i!

Only changes recommended:

@owenatgov owenatgov force-pushed the add-pagination-component branch 2 times, most recently from 92cad34 to 1e90e04 Compare May 25, 2022 10:52
@owenatgov
Copy link
Contributor Author

@EoinShaughnessy I've managed to reorder the tables on the website by re-ordering the options in the tech docs in this repo. Tech docs are all sorted!

CHANGELOG.md Outdated

#### Help users navigate through pages with pagination

You can now use [pagination](https://design-system.service.gov.uk/components/pagination/) to help users navigate forwards and backwards through a series of pages. For example, in search results or guidance that's divided ito multiple website pages.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linked guidance here doesn't exist yet so this is a broken link, but this is the page it is going to be once it gets published. There's a decision here about if we want to merge this now or once the guidance is deployed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can merge this in, even with the broken link. While it's unreleased, no one should really be looking at / expect the Changelog to be fully working anyway. When we do the release, it does mean the release notes will contain a broken link for a short period of time until the guidance has been published, but we try to do them in quick succession to limit the length of time this happens. Ideally, most people will become aware of the release via our comms too, which we'll only put out once the guidance has also been published.

TLDR: it's all good to be merged, after v4.1.1 has been released.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2610 May 30, 2022 10:12 Inactive
@vanitabarrett vanitabarrett added this to the v4.2.0 milestone Jun 8, 2022
@owenatgov owenatgov force-pushed the add-pagination-component branch from db212da to 4c093dc Compare June 13, 2022 10:20
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2610 June 13, 2022 10:20 Inactive
@owenatgov owenatgov force-pushed the add-pagination-component branch from 4c093dc to 9fbf8b9 Compare June 20, 2022 09:35
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2610 June 20, 2022 09:35 Inactive
CHANGELOG.md Outdated Show resolved Hide resolved
@owenatgov owenatgov force-pushed the add-pagination-component branch from 9fbf8b9 to 30e9422 Compare June 20, 2022 14:50
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2610 June 20, 2022 14:51 Inactive
@owenatgov owenatgov merged commit da32bb0 into main Jun 20, 2022
@owenatgov owenatgov deleted the add-pagination-component branch June 20, 2022 14:55
@domoscargin domoscargin mentioned this pull request Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.