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

Jump to main content, headings and more for the ARIA examples #1635

Closed
wants to merge 18 commits into from

Conversation

jongund
Copy link
Contributor

@jongund jongund commented Nov 23, 2020

The jumpto.js script dynamically generates a list of links to important landmarks and headings within each example page, and provide a menu button menu to navigate to the structure.
We don't have a skip to main feature for the examples and this would be an easy way to implement the feature, plus provide more functionality than simple skip to main links found on many pages.

Links to examples with the JumpTo feature included:

@github-actions
Copy link
Contributor

Regression test coverage:

Examples without any regression tests:

  • dialog-modal/alertdialog.html

Examples missing some regression tests:

  • dialog-modal/datepicker-dialog.html:
    • textbox-aria-describedby
  • menu-button/menu-button-actions-active-descendant.html:
    • menu-up-arrow
    • menu-down-arrow
    • menu-character
  • toolbar/toolbar.html:
    • toolbar-tab
    • toolbar-right-arrow
    • toolbar-left-arrow
    • toolbar-home
    • toolbar-end
    • toolbar-toggle-esc
    • toolbar-toggle-enter-or-space
    • toolbar-radio-enter-or-space
    • toolbar-radio-down-arrow
    • toolbar-radio-up-arrow
    • toolbar-button-enter-or-space
    • toolbar-menubutton-enter-or-space-or-down-or-up
    • toolbar-menu-enter-or-space
    • toolbar-menu-down-arrow
    • toolbar-menu-up-arrow
    • toolbar-menu-escape
    • toolbar-spinbutton-down-arrow
    • toolbar-spinbutton-up-arrow
    • toolbar-spinbutton-page-down
    • toolbar-spinbutton-page-up
    • toolbar-checkbox-space
    • toolbar-link-enter-or-space
    • toolbar-spinbutton-role

Example pages with Keyboard or Attribute table rows that do not have data-test-ids:

  • dialog-modal/alertdialog.html
    • "Keyboard Support" table(s):
      • Tab
      • Shift + Tab
      • Escape
      • Command + S
      • Control + S
    • "Attributes" table(s):
      • alertdialog
      • aria-labelledby=IDREF
      • aria-describedby=IDREF
      • aria-modal=true
      • alert

SUMMARY:

55 example pages found.
1 example pages have no regression tests.
3 example pages are missing approximately 27 out of approximately 780 tests.

ERROR - missing tests:

Please write missing tests for this report to pass.

@carmacleod
Copy link
Contributor

I'm not sure of the APG policy on using external code?

Anyhow, here's a few initial review notes:

  1. Perhaps the "Skip To Content" button should be at the top left (on LTR pages) instead of centered at the top of the page, only because that's where people usually expect to find skip links (would like others' opinion on this before actually moving it).
  2. I think we should use "navigation" and not "Menu" for navigation landmarks, so that if a sighted screen reader user chooses "Menu: Related links" they aren't confused when they end up on "Related links navigation".
  3. I think the landmark label should come before the landmark type, so that it matches what screen readers say, and because that looks/sounds more natural. So for example, I think "Menu: Related links" should be "Related links navigation".
  4. I think it would be clearer to indent nested landmarks (i.e. similar to heading levels) instead of prepending with the parent landmark type, e.g. if you Show All Landmarks in the Carousel examples, "Main: Highlighted television shows" is listed, but this is not a Main region - it's just a region. I think it should be, "Highlighted television shows region". (The way it currently is, it looks like there are 2 main regions, which is not accurate.)
  5. If the landmark has an aria-roledescription, then consider listing the roledescription instead of the landmark type, e.g. "Highlighted television shows region" becomes "Highlighted television shows carousel".
  6. If the user chooses Show All, they probably want Show All to still be selected the next time they drop down the Skip To Content menu.
  7. The title/description of the Skip To Content menu button is a bit verbose: Keyboard Navigation Accesskey is "Alt+0". VoiceOver says Keyboard Navigation Accesskey is "Control+Option+0" access key available: '0' Can we maybe change it to just be the access key in brackets, e.g.: (Alt+0), which is a typical way that authors expose keyboard shortcuts?
  8. For some reason (JAWS bug?), JAWS reads only the description/title of the menu button in Chrome instead of the name (i.e. it doesn't read the visible "Skip To Content" text in Chrome, but it does in Firefox).

Other questions specifically about general APG example page markup (not quite related to Skipto, but makes me wonder):

  • Should we consider putting the Example itself in a landmark region labelled "Example"?
  • Should we label the unlabelled nav at the bottom of the page (containing the "Pattern" link) since there's another nav at the top of the page? (the bottom nav is flagged as needing a label by our automated tooling). Or should we remove the nav? Or maybe change it to a footer?

@JAWS-test
Copy link

JAWS-test commented Nov 24, 2020

In Firefox the numbers in front of the headings are not displayed in the same line as the brackets

In IE 11 the button does not work, the menu cannot be opened (just for info, I know: there is no IE 11 support anymore)

Perhaps the "Skip To Content" button should be at the top left (on LTR pages) instead of centered at the top of the page, only because that's where people usually expect to find skip links (would like others' opinion on this before actually moving it).

I consider the current position to be ok. Left would be ok too

I think we should use "navigation" and not "Menu" for navigation landmarks, so that if a sighted screen reader user chooses "Menu: Related links" they aren't confused when they end up on "Related links navigation".

Absolutely. Menu is the wrong name

I think the landmark label should come before the landmark type, so that it matches what screen readers say, and because that looks/sounds more natural. So for example, I think "Menu: Related links" should be "Related links navigation".

I do not know if all screenreaders output regions in this order. If so, then I agree

I think it would be clearer to indent nested landmarks (i.e. similar to heading levels) instead of prepending with the parent landmark type, e.g. if you Show All Landmarks in the Carousel examples, "Main: Highlighted television shows" is listed, but this is not a Main region - it's just a region. I think it should be, "Highlighted television shows region". (The way it currently is, it looks like there are 2 main regions, which is not accurate.)

+1

If the landmark has an aria-roledescription, then consider listing the roledescription instead of the landmark type, e.g. "Highlighted television shows region" becomes "Highlighted television shows carousel".

+1

If the user chooses Show All, they probably want Show All to still be selected the next time they drop down the Skip To Content menu.

+1

The title/description of the Skip To Content menu button is a bit verbose: Keyboard Navigation Accesskey is "Alt+0". VoiceOver says Keyboard Navigation Accesskey is "Control+Option+0" access key available: '0' Can we maybe change it to just be the access key in brackets, e.g.: (Alt+0), which is a typical way that authors expose keyboard shortcuts?

Accesskey 0 should be perceptible for keyboard users. This is currently not the case. Should be visually visible.
The title attribute can be removed.
The visual hint can be hidden for AT with aria-hidden=true, because AT output the accesskey attribute

For some reason (JAWS bug?), JAWS reads only the description/title of the menu button in Chrome instead of the name (i.e. it doesn't read the visible "Skip To Content" text in Chrome, but it does in Firefox).

This is a JAWS bug (see FreedomScientific/standards-support#471) that would no longer occur if the unnecessary title attribute is removed. Chrome correctly transfers the label to the Accessibility API

@JAWS-test
Copy link

JAWS-test commented Nov 24, 2020

the groupings in the menu are not recognized by JAWS and NVDA, and the group labels ("Landmark", Headings", "Actions") are not output. The labels are marked with role=separator.

The brackets at the headlines are not very nice (in the screenreader output) and should be removed or e.g. replaced by a dot

@jongund
Copy link
Contributor Author

jongund commented Nov 24, 2020

@carmacleod
@JAWS-test
Thanks for all the great feedback!

Jon

@jongund
Copy link
Contributor Author

jongund commented Nov 24, 2020

@carmacleod
@JAWS-test
In the case of the skip to menu is primarily for sighted keyboard only users, even though screen reader users can benefit from the important landmarks and headings defined by the author by providing shorter lists than what they would view their screen reader functions for landmarks and headings.
So for sighted keyboard users I think the landmark label at the beginning of the menu item makes it easier for them to identify the type of content in that region.
I do agree than using navigation is better than menu.

@jongund
Copy link
Contributor Author

jongund commented Nov 24, 2020

@carmacleod
The list of landmarks right now is by importance of where a keyboard user would most likely want to navigate, not the document order or nesting of the landmarks. So this is the current ordering of landmarks in the skip to menu:

  1. main landmarks
  2. navigation landmarks
  3. complementary
  4. region
  5. header
  6. footer

The priority is based on what keyboard users would want at the beginning of the menu.

@jscholes
Copy link
Contributor

@JAWS-test

the groupings in the menu are not recognized by JAWS and NVDA, and the group labels ("Landmark", Headings", "Actions") are not output.

This is a critical issue and makes the menu pretty difficult to understand if you can't see it. In my exploration of accessible, labelled groupings for menu items, I didn't find a solution which worked with many screen reader and browser combinations, so this seems like a blocker. I'd also be interested in any user research which unequivocally declares this menu construct to be more useful than a simple skip link for keyboard users (e.g. it doesn't just introduce more keystrokes for no gain, especially on smaller pages).

@carmacleod

Should we label the unlabelled nav at the bottom of the page (containing the "Pattern" link) since there's another nav at the top of the page?

This is directly related to skipto, because at present it renders a single item confusingly called "Menu" for it. Navigating this construct with a screen reader makes it sound like a badly implemented menu pattern because of all the repetition.

Overall, I'd vote for a simple skip link, given the work required to make this menu more accessible and user-friendly. I don't think it should be an option to consider something which exhibits less than perfect accessibility for an APG example, and the skipto defaults aren't ideal.

@mcking65
Copy link
Contributor

mcking65 commented Dec 1, 2020

First, Jon, thank you for raising this. I think we should eventually do something like this. However, I am reluctant to do it now given that the APG redesign project will be starting the transition to the WAI web site early next year. APG 1.2 should be the last publication to TR. Then, we will be in continuous publication mode and say good bye to versioning!

I think the best time to add a feature like this is after we have done the transition and start the actual redesign work. This would become part of the site infrastructure.

The great thing about this PR is the discussion it is raising. There is clearly plenty to figure out.

@carmacleod, we made an explicit decision a long time back to not put landmark regions inside of main, and in particular, not around the example. We tried it for a while. It just felt very confusing, especially when the example includes regions. Instead, we have the separator that clearly marks the beginning and end of the example for screen reader users, which seems like a good non-visual alternative to the visual design. The most important reason, though, is that we wanted to clearly model the difference between regions of a page and sections of content, the latter being represented by the IA exposed via headings.

@jongund
I agree with nearly all of @carmacleod's feedback about the menu itself. If we go this path, we'll want to carefully implement it so that the copy in the menu is ideal for each type of element supported.

I would add that I'm personally not a fan of the heading level numbers in the menu, they sound like unhelpful clutter. I'm also not enthusiastic about using access keys to provide a keyboard shortcut. I'd rather have a js implementation so it's more reliable and consistent across browsers.

I would love to see more experimentation in this PR. It'd be fabulous to have agreement on an approach/design before it is time to design. Then, the designer assigned to the APG redesign project would be able to incorporate it into the redesign proposal.

@jongund
Copy link
Contributor Author

jongund commented Dec 1, 2020

@carmacleod @JAWS-test
I have indented nested landmark regions and changes the label form "Menu" to "Nav" for the navigation landmarks.

@jongund
Copy link
Contributor Author

jongund commented Dec 1, 2020

@mcking65
I think it is a very simple addition to the examples, and gives us a chance to get more feedback on the functionality and the purpose. While I think SkipTo is useful to screen reader users by allowing the author to suggest important regions on the page, it's primary audience is keyboard only and speech recognition users to give them some of the functionality that screen reader users have. So in terms of labeling menu items, I think the primary is the visual keyboard only user viewing the menu options, we could make improvements for screen reader users by adding aria-label to the menuitems.

@jongund
Copy link
Contributor Author

jongund commented Dec 2, 2020

@mcking65 @carmacleod @JAWS-test
I have made some updates to SkipTo:

  • Added support for role-description for landmark labels (see carousel examples)
  • Indenting nested landmarks
  • Using aria-label on heading menu items to make them sound more like screen readers

Also the primary users I believe are keyboard only users and voice recognition users, so the visible labels should be geared for sighted users. It is still useful to screen reader users, by allowing the author to suggest the important regions of the page, since in a screen reader the list of landmarks and headings can get quite long.

@jongund jongund changed the title Skip to main content, headings and more for the ARIA examples Jump to main content, headings and more for the ARIA examples Dec 18, 2020
@jongund
Copy link
Contributor Author

jongund commented Dec 18, 2020

I updated the code to use JumpTo, instead of SkipTo, and used the W3C license for JumpTo.

Base automatically changed from master to main March 3, 2021 18:13
@jongund
Copy link
Contributor Author

jongund commented Apr 14, 2021

This pull request is superseded by PR #1860

@jongund jongund closed this Apr 14, 2021
@zcorpan zcorpan deleted the skip-to-example branch May 31, 2021 12:29
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.

5 participants