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

Fix focus style being overlapped by summary action links #3862

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

querkmachine
Copy link
Member

@querkmachine querkmachine commented Jun 26, 2023

Fixes the WCAG 2.2 issue of subsequent action links in the summary list component slightly overlapping the currently focused link.

Resolves #3841.

Screenshots

Before After
Narrow layout A long list of action links. The second, 'use', is focused. A small grey line is partially overlapping it from a link on the line below. A long list of action links. The second, 'use', is focused. The link is unobscured.
Wide layout A long list of action links. The eighth, 'upgrade', is focused. A small grey line is partially overlapping it from a link on the line below. A long list of action links. The eighth, 'upgrade', is focused. The link is unobscured.

Changes

  • Summary list action links now receive the isolation: isolate style when focused. This creates a new stacking context, making it appear 'above' the surrounding content.

Thoughts

  • There are a few ways that creating a new stacking context could have been done. position: relative, transform: translate(0, 0), and others. I've used the isolation property as it is intended specifically for this purpose, on the assumption this fix doesn't need to support IE. (Browser support for isolation)
  • This issue doesn't affect the action links on summary cards, as they are already separated using row-gap, preventing the overlap from occuring.
    • Presumably we don't want to do the same thing here, as the action link line spacing is consistent with the content next to it and we probably don't want to change that.

@querkmachine querkmachine self-assigned this Jun 26, 2023
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3862 June 26, 2023 13:00 Inactive
@querkmachine querkmachine force-pushed the fix-summary-list-overlapping-links branch from aa4f024 to 87b2519 Compare June 26, 2023 14:14
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3862 June 26, 2023 14:14 Inactive
@querkmachine querkmachine requested a review from a team June 26, 2023 14:17
@querkmachine querkmachine marked this pull request as ready for review June 26, 2023 14:17
@colinrotherham
Copy link
Contributor

This is great, hadn't seen isolation before

Could it have been solved with "target size" instead as those link heights are quite low on mobile?

@querkmachine
Copy link
Member Author

@colinrotherham My presumption is that the current spacing is intentional, as it means the links have a vertical rhythm consistent with the text next to it.

image

If we increase the size or spacing of the links, we lose that bit of visual consistency.

image

As there aren't usually more than a 2 or 3 action links (this being a contrived, extreme example), the target size issue probably doesn't manifest itself in a problematic way very often.

The links are also (just barely) large enough to not fall foul of the WCAG 2.2 minimum target size in their current form.

Copy link
Contributor

@colinrotherham colinrotherham left a comment

Choose a reason for hiding this comment

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

Let's approve because it fixes what it claims to fix 😎

Regarding link heights (blocks of text are exempt?) there's a similar comment regarding Check your answers "Change" links but it's one for another issue

@querkmachine querkmachine merged commit 15ba686 into main Jun 27, 2023
@querkmachine querkmachine deleted the fix-summary-list-overlapping-links branch June 27, 2023 14:03
colinrotherham pushed a commit that referenced this pull request Jul 19, 2023
…inks

Fix focus style being overlapped by summary action links
@romaricpascal romaricpascal mentioned this pull request Dec 8, 2023
@dav-idc
Copy link

dav-idc commented Jan 5, 2024

Cleanup Q: should this issue be in the govuk-frontend 5.0.0 milestone?

@dav-idc
Copy link

dav-idc commented Jan 5, 2024

Ah never mind, I see the issue associated with this pull request is in the 5.0.0 milestone 👍

Here's a full list of the accessibility issues addressed in 5.0, from what I can tell: https://github.com/alphagov/govuk-frontend/issues?q=milestone%3Av5.0+label%3Aaccessibility

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WCAG 2.2: Summary list ‖ Focus Not Obscured (Enhanced)
4 participants