StackExchange / Stacks

Stack Overflow’s Design System
https://stackoverflow.design
MIT License
607 stars 89 forks source link

fix(pagination):improve focus style to meet WCAG 2.2 AA #1612

Closed dancormier closed 7 months ago

dancormier commented 7 months ago

Addresses STACKS-544

Technical notes

You'll notice that this PR achieves the focus style mainly with box-shadow:

   box-shadow: inset 0 0 0 var(--su-static2) var(--theme-secondary-400), inset 0 0 0 var(--su-static4) var(--black-050);
    …
   outline: var(--su-static2) solid transparent;

This is because box-shadow tends to align more consistently. Additionally, a transparent outline is used to ensure that an outline doesn't render when expected but can still be rendered in forced-color modes (see https://blogs.windows.com/msedgedev/2020/09/17/styling-for-windows-high-contrast-with-new-standards-for-forced-colors/#gist105426765).

netlify[bot] commented 7 months ago

Deploy Preview for stacks ready!

Name Link
Latest commit 5c9661f7a4cc616b32a85e23a187b2d777b8c638
Latest deploy log https://app.netlify.com/sites/stacks/deploys/65b3e335b3e96c0008a33912
Deploy Preview https://deploy-preview-1612--stacks.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

dancormier commented 7 months ago

Visually, everything looks good, from other examples I'm seeing online I think we need Focus to land on the selected/active page number as well. In the preview, it skips over page 1 when I tab through the elements.

The current page is indicated by aria-current="page".

Other resource

Unless I'm missing something...

I've added the focus style on the pagination item with .is-selected, but it would only be focused if it were rendered as a link. We've historically omitted the link on the active element, though I can change that (it would only change that in the docs and consumers of the component would need to update if that want to match that). I'll update the docs to include a linked selected pagination item.

image

dancormier commented 7 months ago

@CGuindon I've update the pagination docs to include a linked selected pagination item and I've added the visually hidden "page" text inspired by the W3C design system:

For all pagination links excluding the current page, page is added to provide additional context to the link wording for users of Assistive Technology.

CGuindon commented 7 months ago

@dancormier Would it be possible to add a test for this? So that any pagination buttons that are missing this piece in core would then show up in our automated tests list of issues with exact locations? (or am I dreaming to big)

dancormier commented 7 months ago

@dancormier Would it be possible to add a test for this? So that any pagination buttons that are missing this piece in core would then show up in our automated tests list of issues with exact locations? (or am I dreaming to big)

Dreaming big is good! That said, I think automated accessibility testing may catch failures like aria-current="page" missing, but I'm not 100% sure about that. As far as using an anchor for the current page, I don't believe that's an explicit requirement for WCAG AA (just a decent practice) so automated tests for sure wouldn't call that out.

It's pretty hard to enforce our components to be marked up exactly as we'd want in our consumer code unless they're using something like a razor or svelte component that handles the markup for the consumer. At some point, we'll probably want to work with Core to help design their accessibility tests broadly which may help with this, but I think this will probably need to be manual for the moment.