Add example of disclosure navigation with top-level links #1614

smhigley · 2020-11-13T08:40:42Z

Adds a new disclosure menu example page with top-level links in addition to disclosures, based on the 10/27 APG meeting. Related to #89. Also updates the disclosure nav js file to use a class pattern based on the updated code style guide.

Preview Link

Review checklist

Preview | Diff

github-actions · 2020-11-13T08:41:34Z

Regression test coverage:

Examples without any regression tests:

dialog-modal/alertdialog.html
disclosure/disclosure-navigation-hybrid.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:

56 example pages found.
2 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 · 2020-11-13T16:02:35Z

Looks good! Keyboard works well, i.e. tab, arrows, home, end, space, enter, escape - and various combinations of tab and arrows - all work intuitively. Will try screen readers later.

Couple of notes:

would be cool to have a CodePen button
the dropdowns show, then hide, on page load - would be great if they were initially hidden

examples/disclosure/disclosure-navigation-hybrid.html

mcking65 · 2020-11-13T21:34:34Z

@smhigley asks:

@mcking65 -- was there ever an issue filed for this one?

No, we didn't create an issue for it. We can just use this PR. I'll add labels here.

spectranaut

This is an approving code review, looks good!

Can you add the open in codepen button?

For tests: so much of the code and functionality is the same between this example and "Disclosure for Navigation Menus" is the same. You can certainly just copy over the relevant tests.

mcking65 · 2020-12-15T21:40:57Z

@smhigley , looking awesome!

I have two questions.

First, The buttons are labeled like "Show About section". The word "section" doesn't quite feel right; makes me think it is going to show a section of content. I'm wondering if it should be something like "show links for About section" or "Show links to About pages"

Second, as part of a quest for consistency among related examples, would you be OK with making the sample content look and feel the way Jon has it set up in the tree example and upcoming new version of the menubar example? See #1558. On those pages, he kind of made a mini web site inside the example. It has a header, footer, and the main content region is labeled by the site name and H1. One of his objectives was to put a couple focusable elements inside the main content region. Note that he used role region, not main. There are comments about that in the source and in the documentation tables.

spectranaut · 2020-12-16T00:14:55Z

Hi @smhigley! Thanks for adding the codepen button. If you notice, the links don't work in a very friendly way in Codepen, it takes you back to the the APG webpage when you click on them. @carmacleod and I looked into ways to make the example work on both the APG webpage and in codepen on this similar example you wrote: #1651 (comment)

What do you think of preventing the default behavior of the onclick button so that if will work in the codepen example?

spectranaut

Just looked over your tests, @smhigley , this is an approving test review!

smhigley · 2020-12-16T00:40:38Z

@spectranaut Thanks for the review! I'll add the preventDefault to the example links, and push it up together with a test fix, as soon as I figure out what's making them fail 😅

nschonni · 2020-12-16T05:35:40Z

I think npm run reference-tables should fix the example failure for you

carmacleod

Not sure if this is the prettiest way to prevent the initial show-hide flash, but it does work. :)

carmacleod · 2021-01-06T04:46:26Z

examples/disclosure/disclosure-navigation-hybrid.html

+            <button type="button" aria-expanded="true" aria-controls="id_about_menu" aria-label="Show About section"> </button>
+            <ul id="id_about_menu">


Can we start all of the buttons off in the collapsed state so that there's no initial flash when the ul is hidden?

Suggested change

<button type="button" aria-expanded="true" aria-controls="id_about_menu" aria-label="Show About section"> </button>

<ul id="id_about_menu">

<button type="button" aria-expanded="false" aria-controls="id_about_menu" aria-label="Show About section"> </button>

<ul id="id_about_menu" style="display: none;">

carmacleod · 2021-01-06T04:47:22Z

examples/disclosure/disclosure-navigation-hybrid.html

+            <button type="button" aria-expanded="true" aria-controls="id_admissions_menu" aria-label="Show Admissions section"> </button>
+            <ul id="id_admissions_menu">


Prevent initial flash.

Suggested change

<button type="button" aria-expanded="true" aria-controls="id_admissions_menu" aria-label="Show Admissions section"> </button>

<ul id="id_admissions_menu">

<button type="button" aria-expanded="false" aria-controls="id_admissions_menu" aria-label="Show Admissions section"> </button>

<ul id="id_admissions_menu" style="display: none;">

carmacleod · 2021-01-06T04:48:17Z

examples/disclosure/disclosure-navigation-hybrid.html

+            <button type="button" aria-expanded="true" aria-controls="id_academics_menu" aria-label="Show Academics section"> </button>
+            <ul id="id_academics_menu">


Prevent initial flash.

Suggested change

<button type="button" aria-expanded="true" aria-controls="id_academics_menu" aria-label="Show Academics section"> </button>

<ul id="id_academics_menu">

<button type="button" aria-expanded="false" aria-controls="id_academics_menu" aria-label="Show Academics section"> </button>

<ul id="id_academics_menu" style="display: none;">

I thought we did this so that if JS weren't enabled or didn't load, it would still work. Is that not a concern we're targeting?

a11ydoer · 2021-01-24T23:10:00Z

@smhigley
The preview link above does not show changes such as label of "Show About section", which @mcking65 commented on. I just want to make sure that I review the most recent example. Thanks for your consideratoin.

a11ydoer · 2021-01-24T23:24:34Z

I checked the current example with VoiceOver and it works well. Keyboard navigation also works as documented.

a11ydoer · 2021-01-24T23:57:55Z

I could not find the same visual discloaure navigation implementation from popular sites yet. However, the closest visual design as well as its coding to current menu example can be found at WAI tutorial, Flyout menu, use button as toogle.

The screen shot was attached.

a11ydoer · 2021-01-25T00:00:18Z

I am waiting for the most updated preview link before I finish accessiblity review.

smhigley · 2021-01-26T18:52:37Z

@a11ydoer the preview link is up to date. I don't see any comment from Matt on the button labels -- what was the feedback?

I'm also not sure I understand your comment on the WAI tutorial. Were you suggesting we add a link to it from the example page? I'd be happy to add one if so; just let me know where you want it :).

a11ydoer · 2021-01-26T19:55:57Z

@smhigley thanks for the updated preview link. I will do the review.
Matt's comment is here

You can ignore my wai example comment. It is simple justification method for visual review by me. I am trying to make sense for APG design with other design example. WAI is the one I could find as the similar one to current example.

carmacleod · 2021-02-03T20:23:33Z

@smhigley Thanks for hiding the dropdowns on load!

When the user clicks on a link, should focus go to the mythical-page-content div instead of going to body or staying on the anchor?

Focusing body (i.e. after clicking link in dropdown) unfortunately causes NVDA to read out the page title (several times in Firefox), and keeping focus on the link (i.e. after clicking one of the top-level links) causes NVDA to say nothing at all in Chrome (and "current page" in Firefox). JAWS behavior is similar.

jscholes · 2021-02-09T18:59:56Z

@smhigley Glad to see an example for this pattern in the APG! IMHO, the labels for the disclosure buttons are very verbose for screen reader users, and all start with the same three syllables ("Show links to") rather than the most significant information being placed upfront.

As a related note, the verb "show" is probably unnecessary, because the use of aria-expanded already indicates that this is a disclosure and it isn't updated when the buttons become expanded anyway.

Not sure on the best wording, but I'd definitely like to see the name of the section come first. Maybe just, "About sub-menu", "About section", "About pages", or similar?

a11ydoer · 2021-02-09T19:40:52Z

@smhigley

Here are two suggestions from APG meeting on Feb 9, 2021

aria-label change: upon @jscholes suggestion, the "show links to about pages" becomes "links to about pages"
Title changes to "Example disclosure navigation menu with top level links"

smhigley · 2021-02-09T19:50:44Z

@jscholes / @a11ydoer the disclosure button label was @mcking65's suggestion, so I want to make sure he gets the chance to review and approve the change. I'm happy to change it to anything :).

Another possibility might be something simple like "Admissions submenu" -- I know "submenu" and "menu" means something specific in ARIA, but for general labeling I think it might fit well here.

@a11ydoer I'll update that title too :)

jscholes · 2021-02-09T20:25:16Z

@smhigley

Another possibility might be something simple like "Admissions submenu" -- I know "submenu" and "menu" means something specific in ARIA, but for general labeling I think it might fit well here.

FWIW, this wording (sub-menu) is what I recommend to clients implementing such a pattern. I've come across quite a few cases of this pattern being implemented mostly correctly in the wild, but with really verbose messaging for the buttons. This example list only has three disclosures, but it's not unusual to have six or more.

smhigley · 2021-02-24T02:05:56Z

@shirsha I can't reproduce with JAWS 2020, NVDA 2020, and Chrome 88. The link behavior is entirely handled by the browser though, so I don't think it would be related to the code in this example.

shirsha · 2021-02-24T02:44:21Z

@smhigley I used JAWS 2021 , NVDA 2020.3 , Chrome 88.0.4324.190

a11ydoer

aria-label, more [name] pages was confirmed through the review.
emphasis on how this example is different from aria menu looks good.
focus styling, having a few pixels out of border line will be researched later.

Thanks for all your work @smhigley !

a11ydoer · 2021-02-24T03:56:43Z

@shirsha
Thanks for the recorded screen reader videos! These are very useful.

I noticed that when the screen reader announced "signup sample content" the focus was already in content section, not in the "sign up" sublink.

…title and h1

mcking65 · 2021-02-24T18:34:25Z

I have finished making editorial changes for consistency. I have also fixed the title of and links to the other disclosure navigation menu example.

There is one part of the text that I'm still not satisfied is sufficiently clear. It is related to high contrast mode documentation. It is:

To ensure the carets that indicate the presence and state of the disclosures are reliably rendered when operating system or browser high contrast settings are enabled, visual indication of the show/hide state is created using CSS ::after pseudo element border styles.

It really doesn't yet explain why this is choice of coding technique matters. Compare to what we have written in the navigation menu bar for similar HCM support.

To ensure the arrow icons used to indicate the expanded or collapsed state have sufficient contrast with the background when high contrast settings invert colors, the CSS currentColor value for the fill and stroke properties of the SVG polygon element is used to synchronize the color with text content. If specific colors are used to specify the fill and stroke properties, these colors will remain the same in high contrast mode, which could lead to insufficient contrast between the icon and the background or even make the icon invisible if its color matches the high contrast mode background.

This text from the navigation menubar explains the feature, why it matters, and why the technique for achieving it was chosen.

Could someone suggest wording that achieves these same objectives for the HCM documentation for this example?

mcking65 · 2021-02-27T22:43:16Z

This is all ready to merge except that I would still like some assistance improving the high contrast documentation in accessibility features as described in my previous comment. Could someone help me understand the rationale for choice of the after pseudo border style and what distinguishes that technique from others when it comes to HCM support?

…er agreement in March 2 meeting

mcking65 · 2021-03-03T19:15:03Z

Per discussion in March 2, 2021 meeting, I removed:

To ensure the carets that indicate the presence and state of the disclosures are reliably rendered when operating system or browser high contrast settings are enabled, visual indication of the show/hide state is created using CSS ::after pseudo element border styles.

Conducting a final read and will merge.

mcking65

This approving editorial review.

Love this addition! Thank you @smhigley!

…3c#1614) * Adds another version of the mythical university navigation that includes links for the top-level pages. * Satisfies one of the steps in the plan for improving navigation patterns described in issue w3c#89. Co-authored-by: Matt King <a11yThinker@gmail.com>

carmacleod reviewed Nov 13, 2020

View reviewed changes

examples/disclosure/disclosure-navigation-hybrid.html Outdated Show resolved Hide resolved

mcking65 added enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy Example Page Related to a page containing an example implementation of a pattern labels Nov 13, 2020

mcking65 changed the title ~~Disclosure navigation w/ top-level links~~ Add example of disclosure navigation with top-level links Nov 13, 2020

spectranaut approved these changes Nov 19, 2020

View reviewed changes

carmacleod mentioned this pull request Nov 26, 2020

Mega menu model #1013

Open

a11ydoer self-requested a review December 15, 2020 20:05

spectranaut approved these changes Dec 16, 2020

View reviewed changes

carmacleod reviewed Jan 6, 2021

View reviewed changes

smhigley force-pushed the disclosure-nav2 branch from 9a79e3a to 66ac674 Compare January 19, 2021 23:27

a11ydoer approved these changes Feb 24, 2021

View reviewed changes

mcking65 added 12 commits February 24, 2021 08:56

Add link to advisement, add missing comma, delete dupe note.

b317fee

partial white space fix

eac99ce

Make title tag consistent with h1

692b620

Editorial revision to tab key description

3c025a3

Editorial revision to aria-controls

eb770be

Editorial clarification of accessibility features

0b56059

Fix link text for the disclosure nav menu example

fc7be57

Add advisement to disclosure nav menu example, remove dupe note, fix …

50d10b4

…title and h1

Make link text consistent

9ea4396

Make link text consistent

ac41d61

Make link text consistent and fix href

13f02cc

Make link text consistent

22abf70

Uupdate index of examples

0ba0a3e

Base automatically changed from master to main March 3, 2021 18:13

mcking65 added this to the 1.2 Release 1 milestone Mar 3, 2021

Remove accessibility feature documentation of high contrast support p…

00c1af9

…er agreement in March 2 meeting

mcking65 approved these changes Mar 3, 2021

View reviewed changes

mcking65 merged commit a060577 into main Mar 3, 2021

mcking65 deleted the disclosure-nav2 branch March 3, 2021 19:23

smhigley mentioned this pull request Apr 19, 2021

disclosure navigation Example: Add "Open in CodePen" button #1651

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add example of disclosure navigation with top-level links #1614

Add example of disclosure navigation with top-level links #1614

smhigley commented Nov 13, 2020 •

edited by pr-preview bot

Loading

github-actions bot commented Nov 13, 2020

carmacleod commented Nov 13, 2020

mcking65 commented Nov 13, 2020

spectranaut left a comment

mcking65 commented Dec 15, 2020

spectranaut commented Dec 16, 2020

spectranaut left a comment

smhigley commented Dec 16, 2020

nschonni commented Dec 16, 2020

carmacleod left a comment

carmacleod Jan 6, 2021

carmacleod Jan 6, 2021

carmacleod Jan 6, 2021

smhigley Jan 12, 2021

a11ydoer commented Jan 24, 2021

a11ydoer commented Jan 24, 2021

a11ydoer commented Jan 24, 2021 •

edited

Loading

a11ydoer commented Jan 25, 2021

smhigley commented Jan 26, 2021

a11ydoer commented Jan 26, 2021 •

edited

Loading

carmacleod commented Feb 3, 2021 •

edited

Loading

jscholes commented Feb 9, 2021

a11ydoer commented Feb 9, 2021

smhigley commented Feb 9, 2021

jscholes commented Feb 9, 2021

smhigley commented Feb 24, 2021

shirsha commented Feb 24, 2021

a11ydoer left a comment

a11ydoer commented Feb 24, 2021

mcking65 commented Feb 24, 2021

mcking65 commented Feb 27, 2021

mcking65 commented Mar 3, 2021

mcking65 left a comment

		<button type="button" aria-expanded="true" aria-controls="id_about_menu" aria-label="Show About section"> </button>
		<ul id="id_about_menu">

		<button type="button" aria-expanded="true" aria-controls="id_admissions_menu" aria-label="Show Admissions section"> </button>
		<ul id="id_admissions_menu">

		<button type="button" aria-expanded="true" aria-controls="id_academics_menu" aria-label="Show Academics section"> </button>
		<ul id="id_academics_menu">

Add example of disclosure navigation with top-level links #1614

Add example of disclosure navigation with top-level links #1614

Conversation

smhigley commented Nov 13, 2020 • edited by pr-preview bot Loading

Review checklist

github-actions bot commented Nov 13, 2020

Regression test coverage:

Examples without any regression tests:

Examples missing some regression tests:

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

SUMMARY:

carmacleod commented Nov 13, 2020

mcking65 commented Nov 13, 2020

spectranaut left a comment

Choose a reason for hiding this comment

mcking65 commented Dec 15, 2020

spectranaut commented Dec 16, 2020

spectranaut left a comment

Choose a reason for hiding this comment

smhigley commented Dec 16, 2020

nschonni commented Dec 16, 2020

carmacleod left a comment

Choose a reason for hiding this comment

carmacleod Jan 6, 2021

Choose a reason for hiding this comment

carmacleod Jan 6, 2021

Choose a reason for hiding this comment

carmacleod Jan 6, 2021

Choose a reason for hiding this comment

smhigley Jan 12, 2021

Choose a reason for hiding this comment

a11ydoer commented Jan 24, 2021

a11ydoer commented Jan 24, 2021

a11ydoer commented Jan 24, 2021 • edited Loading

a11ydoer commented Jan 25, 2021

smhigley commented Jan 26, 2021

a11ydoer commented Jan 26, 2021 • edited Loading

carmacleod commented Feb 3, 2021 • edited Loading

jscholes commented Feb 9, 2021

a11ydoer commented Feb 9, 2021

smhigley commented Feb 9, 2021

jscholes commented Feb 9, 2021

smhigley commented Feb 24, 2021

shirsha commented Feb 24, 2021

a11ydoer left a comment

Choose a reason for hiding this comment

a11ydoer commented Feb 24, 2021

mcking65 commented Feb 24, 2021

mcking65 commented Feb 27, 2021

mcking65 commented Mar 3, 2021

mcking65 left a comment

Choose a reason for hiding this comment

smhigley commented Nov 13, 2020 •

edited by pr-preview bot

Loading

a11ydoer commented Jan 24, 2021 •

edited

Loading

a11ydoer commented Jan 26, 2021 •

edited

Loading

carmacleod commented Feb 3, 2021 •

edited

Loading