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

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

Closed
matteason opened this issue Apr 26, 2023 · 4 comments · Fixed by #5066
Closed

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

matteason opened this issue Apr 26, 2023 · 4 comments · Fixed by #5066
Assignees
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) pagination

Comments

@matteason
Copy link
Contributor

matteason commented Apr 26, 2023

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:

// Use flex to get around a whitespace issue between the arrow svg and the link text
// without having to rely on whitespace control from backend tooling
.govuk-pagination__link {
display: flex;
align-items: center;

Environment (where applicable)

  • Operating system: Windows 10
  • Browser: Firefox/Chrome
  • Browser version: Firefox 112.0, Chrome 112.0.5615.138
  • GOV.UK Frontend Version: 4.6.0
@matteason matteason added awaiting triage Needs triaging by team 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) labels Apr 26, 2023
@querkmachine querkmachine added pagination and removed awaiting triage Needs triaging by team labels Apr 26, 2023
@matteason
Copy link
Contributor Author

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
Copy link
Member

@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
Copy link
Contributor Author

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

@frankieroberto
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) pagination
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants