Add disclosure menu example for issue 1028

[smhigley]

Resolves #1028.

Continuation of PR #1036, which contains a long thread of relevant discussion.

Preview Link for WIP

View WIP on disclosure navigation menu example in the issue1028-disclosure-menu-example branch

Relevant comments from mcking65 in issue 1028

@smhigley, I am done with editorial revisions to the issue1028-disclosure-menu-example feature branch.

You may want to review the test code. I added documentation for aria-current, which currently does not have a test. I also expanded the documentation for arrow keys, home, and end. Now, there are multiple rows with the same data-test-id, which is OK. However, it might be good to sanity check to make sure the testing covers all the documented behaviors.

My revisions to the accessibility features section were the most significant. They are worth a close read. I attempted to adjust wording and rationale to be understandable by a broader audience. I'm curious if you agree whether I achieved that objective.

I am wondering if the checkbox for optional navigation keys should be unchecked by default. Given there are both side effects and extra code driven by the optional behaviors, shouldn't they be off by default?

Edited to add preview link: https://raw.githack.com/w3c/aria-practices/issue1028-disclosure-menu-example/examples/disclosure/disclosure-navigation.html

---

Preview | Diff

[smhigley]

@mcking65 regarding tests:
I didn't add aria-current tests originally since they are not part of the core disclosure menu javascript, and would be expected to be part of the static html of a page in a normal example (i.e. one without contrived page content). I'll add some for this anyway, so that functionality doesn't regress on this repo.

About the package.json changes: I noticed that the vnu-jar script was being run twice in succession inside the test script, so I fixed it.

I'll add a couple non-substantive changes to some formatting and typos before Tuesday. I also have some qualms about specifically calling out certain screen reader behaviors -- I think it might be better to keep it general (e.g."screen readers will usually intercept arrow key presses within this example markup") rather than get into specifics that don't apply across all screen readers. I'd also prefer to leave the sentence mentioning screen reader behavior out of the following list item, since I think it could read as if tabbing is only implemented for screen reader users:

If implemented, the optional navigation keys supplement, but do not replace, tabbing among buttons and links.

Thanks again for the thorough editorial review and additions!

[mcking65]

@smhigley wrote:

I didn't add aria-current tests originally since they are not part of the core disclosure menu javascript, and would be expected to be part of the static html of a page in a normal example (i.e. one without contrived page content). I'll add some for this anyway, so that functionality doesn't regress on this repo.

Thank you! Our testing is to ensure the reliability of our examples and their documentation, so we still have to test the stuff that is "contrived" for illustrative purposes.

That said, I don't think this pattern is limited to navigation among static pages. It could be used in a single-page app type of architecture.

@smhigley wrote:

I also have some qualms about specifically calling out certain screen reader behaviors -- I think it might be better to keep it general (e.g."screen readers will usually intercept arrow key presses within this example markup") rather than get into specifics that don't apply across all screen readers.

We have consistent descriptions of widget-related mode switching throughout the examples. True, some screen readers like Mac-VO and Narrator are manual switching while JAWS and NVDA are automatic. But, this widget-related behavior is still relevant for all screen readers that run on desktop systems where keyboards are a primary input method. One thing that is still in our backlog, though, is an appendix section that describes the mode switching features in more detail. When that is done, we will link to it from notes like this.

I do appreciate your concern though. Rather than be more general though, I think it may be better to refine it further.

@smhigley wrote:

I'd also prefer to leave the sentence mentioning screen reader behavior out of the following list item, since I think it could read as if tabbing is only implemented for screen reader users:

If implemented, the optional navigation keys supplement, but do not replace, tabbing among buttons and links.

We could explain this without mentioning screen reader users as part of the justification. However, I think the point begs for explanation since it is contrary to every other pattern that provides arrow, home, and end key support. I'll propose another wording.

[spectranaut]

@smhigley @mcking65 these tests look great! There is just one missing test and two minor issues. If you don't have time tomorrow, Sarah, I can push a commit.

[spectranaut]

@mcking65 I fixed and added tests! Should be good to go :)

[mcking65]

I think this is now ready to go. @smhigley , would you like to give it one more look?

[smhigley]

I don't want to block merging, just have an optional suggestion: the javascript that controls aria-current will function the same regardless of which link is clicked, so it's only necessary to test on one link click and cut down on test time a little.

Otherwise, looks good! Thanks for writing this! (Also, changing the loops is entirely up to you, I don't want to hold up merging the PR)

[smhigley]

Thanks @spectranaut and @mcking65! It looks like the Travis build failed on a timeout in an unrelated test, so I restarted it. If it passes, let's merge!