search

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.19k stars 328 forks source link

Pagination in stacked mode relies on whitespace for correct text alignment #3554

Closed matteason closed 4 months ago

matteason commented 1 year ago

Description of the issue

When in stacked (block) mode (no page numbers displayed), the pagination component relies on whitespace in the HTML to align the 'Previous' and 'Next' text with the labels below.

If there's no whitespace between the govuk-pagination__icon SVG and the govuk-pagination__link-title span, the 'Previous' or 'Next' link is displayed about 4 pixels too far to the left.

With whitespace:

Previous and Next links stacked on top of each other, each with an example label below them and an arrow to the left. The text and labels are almost aligned vertically, demonstrated with a red line

Without whitespace:

Previous and Next links stacked on top of each other, each with an example label below them and an arrow to the left. The text and labels are unaligned, demonstrated with a red line

(The text is actually slightly misaligned even in the first version - 'Previous' aligns with the end of the underline on the label rather than the first letter - I'm not sure if this is intentional)

Steps to reproduce the issue

Open this Design System pagination example in Firefox, right-click > Inspect the link, expand the <a> tag, click the whitespace node and press Delete

HTML code from GOV.UK pagination component

Actual vs expected behaviour

I would expect the text alignment to not rely on whitespace in the HTML. In my particular case (a Vue component library) I have no control over whether consumers of the library will have the whitespace compression option enabled or not, though by default it's enabled

There's been a previous issue with whitespace causing layout changes in the header component, and there appears to be a fix for a similar issue when pagination is used in list mode:

https://github.com/alphagov/govuk-frontend/blob/eed94f8b29072325542f2ca4c4d0ffec6c4c543d/src/govuk/components/pagination/_index.scss#L57-L61

Environment (where applicable)

matteason commented 1 year ago

I'm happy to have a go at fixing this if you agree it's worth doing. I know you've just hit a code freeze for v5 so I wouldn't expect it to be merged any time soon.

querkmachine commented 1 year ago

@matteason Ah, the code freeze we had was for last week's 4.6.0 release. You're welcome to contribute a fix if you want!

matteason commented 1 year ago

Ah sorry! I misinterpreted this comment. I'll see what I can do!

frankieroberto commented 1 year ago

Just a ➕1 to say I spotted this bug too – working on a service which uses this component on almost every page.