Add ARIA attributes to pagy_nav* to improve default accessibility

[wimdavies]

Hiya,

I've just used pagy for a beginner Rails project, and it's great — thank you for all your hard work.

I noticed that while pagy_nav does supply aria-labels for the 'previous' and 'next' buttons, it doesn't provide other recommended ARIA attributes for navigation and pagination. If pagy could implement a more comprehensive set of ARIA attributes, I feel it would significantly improve the default accessibility of pagy_nav output, and therefore (hopefully) that of all the sites that use it.

Suggestions for changes:

• the <nav> element should have aria-label="Pagination" (source)
• the <span> element for the current page should have aria-current="page" (source)
• each should have a dynamic aria-label="Page #{number}" or similar label (source)
• when the elements for the previous, next, or current page are styled as disabled, they should also have the aria-disabled="true" state (source)
• something for further consideration is that the current implementation disables such elements by conditionally not creating the <a> tag in the DOM, and giving its parent <span> the relevant inner text. I'm not sure whether this is good practice from an accessibility standpoint, and maybe this is one of the cases where 'one may want users to find these elements through standard keyboard navigation, though the functionality of that control is removed or "disabled"'. If so, it might be better to shift towards keeping the <a> element and setting it to aria-disabled="true" (along with other relevant changes)?

Disclaimer: I'm a recent bootcamp graduate, so I'm not speaking from a position of any particular expertise; I just care about accessibility. I'd welcome any further discussion on this issue.

[benkoshy]

The discussion would be very welcome.

Feel free to note down any further pointers you have - especially with links to authoritative sources to aria guidelines / specs.

[ddnexus]

@wimdavies Great topic.

I am definitely not an expert about ARIA stuff (and I have no time to become one): I only remember I merged the PR #412 from @sunny (which looks like in contrast with your first suggestion) and it would definitely be good to sort this out thoroughly and ensure a basic ARIA compliant output.

Comments and contributions are welcome.

[sunny]

Thank you for opening this discussion 💜

I only remember I merged the PR #412 from @sunny (which looks like in contrast with your first suggestion)

Good call, my understanding from several articles was that aria-label was only for interactive elements… but landmark roles are also good candidates for aria-label! Sorry about that. 🤦

• aria-label="Pagination" on nav: Yes! Note that this text should allow being translated.
• aria-current="page": Yes!
• aria-label="Page #{number}": Yes! The example on w3.org’s design system opts for a visuallyhidden class instead of aria-label. but I would tend to prefer the aria-label on a elements. On a library like this the aria-label is simpler to use without requiring the presence of specific CSS classes. Note that this text should also allow being translated.
• replacing span elements to a elements with aria-disabled="true": On this it’s not so clear why keyboard access on these disabled elements would be preferrable. Also, this would probably be much more difficult in terms of migrating users of the gem since they can rely on there not being a a tag to style it.

[wimdavies]

@sunny Thank you, my pleasure, and thank you for your previous contribution on #412. I've just read the article you posted there, and it's really interesting. I was just coming back here to argue for aria-label="Pagination" on <nav>, so I'm pleased to see you're already commented here to that effect! 😄

• Another argument for the "Pagination" label would be as a pre-emptive defensive measure, since many sites love a top-level navbar (which should semantically also be a <nav>), and so it seems sensible to have pagy_nav able to disambiguate itself as a landmark 'out of the box'.
• I think in my original post I must have cited the wrong source for aria-labelling each (since as you point out, w3 design system go with visuallyhidden). I can't remember whatever it was I intended to link to there, apologies.
  I'm glad you agree—although I guess I should note that this does seem in contradiction of the Eric Bailey article you posted, where he insists on letting the text content of the element speak for itself... thoughts?
• Yes, I agree, I didn't fully understand Mozilla's line of argument regarding keyboard navigation. And I do take your point about migrating gem users not expecting an anchor tag to be present.
• Nonetheless, should the existing elements which give pagy_nav its de facto disabling receive aria-disabled="true"? Or, semantically-speaking, is that worse—since a span doesn't have an interactivity to be disabled? cf. the Bailey article

[ddnexus]

About the aria-label="Page #{number}... aren't we going too far with that? The text of the links IS the page number!

What would be the added value of specifying "page" (that I suppose should also be translated) when the links are clearly already in a nav labeled "pagination"?

Is that really a benefit that is worth an extra entry translated in all the dictionaries and the performance cost of an extra ruby interpolation for every page link, adding an extra i18n lookup?

[wimdavies]

Hi @ddnexus, I'm sorry I missed the notification about your comment.

I think you're right. Like I said in my reply to @sunny, while I was pleased he agreed with me, aria-label="Page #{number} appears to be in contravention of the (apparently well-founded) article he posted in the original PR.

The text of the links IS the page number!

Yup. As the article says, to follow my original suggestion of aria-label="Page #{number} is "a failure of WCAG Success Criterion 2.5.3: Label in Name".

I'm thoroughly persuaded of this, and take back that suggestion.

[ddnexus]

@wimdavies @sunny ok, so we can certainly include the changes not affecting CPU time, which are (so far):

• the element should have aria-label="Pagination"
• the element for the current page should have aria-current="page"
• when the elements for the previous, next, or current page are styled as disabled, they should also have the aria-disabled="true" state

The concern about the optimal HTML structure to support aria-disabled="true" applies only to the basic pagy style/structure, because I guess not all the supported frameworks have the same structure. For the styles that are compatible we should definitely add it; for those that are not we skip it; for pagy... maybe: we might break some app and very likely quite a lot of tests (even just by adding an attribute in the output).

But if we tag the ARIA changes as a mayor version, we can change whatever without the risk of "surprising" anybody with unannounced breaking changes.

If everyone agrees @benkoshy may want to create a new branch so we can coordinate easily and everyone can start working on one style at the time.

Thoughts?

[sunny]

we can certainly include the changes not affecting CPU time

I guess having aria-label="Pagination" translatable would still affect CPU time slightly, though?

[ddnexus]

Yes. I thought about that last week, and I totally forgot the issue today while writing the last post.

I have to think about how to mitigate it, however it is a single iteration per pagination bar, not one per page, and if you use pagy native i18n (or default en) it's negligible.

Thanks for pointing it out.

[ddnexus]

And the other issue related to the translation that I thought and forgot today (twice) is the addition of one entry and translation in all the current dictionaries...

A bit tedious to pull out.

[ddnexus]

The "Pagination" label is really important to be present and translated, it's a single one and the advantages are worth such a negligible cost. We can use Google translate for the languages that we are not sure about and invite the original contributors to review our changes.

I would organize the changes in one PR per locale and one PR per style (which would include also the tests for the specific style).

That will be a lot of PRs, but they should have practically no overlapping/conflicts and they will be based on a separate branch, so quite easy to handle.

[wimdavies]

The "Pagination" label is really important to be present and translated, it's a single one and the advantages are worth such a negligible cost.

Thank you for taking this attitude, it's really laudable 🙏

[ddnexus]

The new html-validate in the e2e tests complains about the "unique-landmark" rule.

┌─────────┬───────────────────┬──────────────────────────────────────────────────────────────────────────────────────────────┬──────────────────────┐
│ (index) │       rule        │                                           message                                            │       selector       │
├─────────┼───────────────────┼──────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────┤
│    0    │ 'unique-landmark' │ 'Landmarks must have a non-empty and unique accessible name (aria-label or aria-labelledby)' │        '#nav'        │
│    1    │ 'unique-landmark' │ 'Landmarks must have a non-empty and unique accessible name (aria-label or aria-labelledby)' │      '#nav-js'       │
│    2    │ 'unique-landmark' │ 'Landmarks must have a non-empty and unique accessible name (aria-label or aria-labelledby)' │ '#nav-js-responsive' │
│    3    │ 'unique-landmark' │ 'Landmarks must have a non-empty and unique accessible name (aria-label or aria-labelledby)' │   '#combo-nav-js'    │
└─────────┴───────────────────┴──────────────────────────────────────────────────────────────────────────────────────────────┴──────────────────────┘

I temporarily disabled the rule in 724f491

@wimdavies @sunny @benkoshy : we are going to start the refactoring ASAP. Hoping to have your help.

[ddnexus]

ARIA milestone ready!

Please, let me know if I missed something.

[ddnexus]

ARIA compliance is only missing a few translations and a bit of documentation for the extra key arguments of the helpers.

The helpers have been refactored keeping into account the ARIA milestone.

Please, try the 7.0.pre0 branch and let me know (ruby 3.1+)
(CHANGELOG)

[sunny]

Here’s a pagy_nav output:

<nav class="pagy-nav pagination" aria-label="Pages"><span class="page prev disabled"><a role="link" aria-disabled="true" aria-label="Prev">&lt;</a></span> <span class="page active"><a role="link" aria-disabled="true" aria-current="page">1</a></span> <span class="page"><a href="/fr/cat%C3%A9gories/art?page=2" rel="next">2</a></span> <span class="page"><a href="/fr/cat%C3%A9gories/art?page=3">3</a></span> <span class="page next"><a href="/fr/cat%C3%A9gories/art?page=2" rel="next" aria-label="Next">&gt;</a></span></nav>

• Links to the previous and next pages are rendered as "<" and ">" and are aria-labelled as translated "Prev" and "Next".

✅ (Although they would benefit from translations.)

• The page numbers don't have any aria-label attribute, since their text is a simple number (read by the readers) and an explicit attribute would be redundant and performance inefficient.

✅

• All the disabled links have the aria-disabled="true" attribute.

✅

• The current page has the aria-current="page" attribute.

✅

• The role="link" is added to the link elements of the styles that don't use an a element or don't have a href attribute.

✅

[ddnexus]

(Although they would benefit from translations.)

Yes and no IMO. If you see the nav, a simple < and > is enough to understand their functions without actual spelling, they look more consistent and symmetric with the numbers and occupy less space. Personally I always hated prev/next and regretted that choice for all that reasons. BTW, they are just a default easy to edit in the dictionary.

If you don't see the screen (i.e. use a reader) the label is unequivocal.

[wimdavies]

Hi folks, sorry I haven't been able to make contributions to this lately. @ddnexus thanks for your hard work on this!

• In my little heart, I feel the loss of aria-label="pagination", since it's such a well-established naming convention. However, as has been remarked upon, the word seems to be an i18n nightmare; and having a custom descriptive label is better practice anyway — so it's great that this is encouraged with the page_label helper argument. 👍
• FWIW I agree that characters like "<< < > >>" as previous and next navigations are preferable to abbreviated words like "prev" for "previous". Maybe it's fair to say that "prev" was abbreviated for the sake of visual symmetry, without consideration of the potential cognitive accessibility cost for abbreviating "previous"?
• Now that symmetry has been achieved without words, I'd suggest that the dictionary should have the full word "previous" for the prev_label, rather than the abbreviation (because I don't know how a screen reader would handle "prev").
• I have just checked in with a friend who is a native Dutch speaker, and can provide a PR for the nl dictionary entry.

[ddnexus]

Now that symmetry has been achieved without words, I'd suggest that the dictionary should have the full word "previous" for the prev_label, rather than the abbreviation (because I don't know how a screen reader would handle "prev").

You are 100% right! I mechanically converted the previous en text to the labelling in the dictionary without even noticing a so obvious improvement. Thank you!

[wimdavies]

@ddnexus ah great! I can easily submit a PR for that simple en change now, if you like? (don't want to mess up your milestone workflow though, since the equivalent issue is marked completed...)

PR is in for the nl dictionary entry 👍

edit: I also opened a PR for the aforementioned change to the en prev_label