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 Content: Add menu button to examples to implement WCAG bypass blocks requirement #1923

Merged
merged 17 commits into from
Jul 18, 2021

Conversation

jongund
Copy link
Contributor

@jongund jongund commented May 26, 2021

This is an update to pull #1860 that got too out of synch with main branch, but still has a failing regression tests for the menu button link example.
NOTE: The menu button link example and tests are not modified in this pull request.

Features:

  • Added jumpto.js
  • Uses color palette of the ARIA APG
  • Uses app.js to add the appropriate reference to each example.

preview link for index of examples
preview link for button example
preview link for checkbox 1 example

@jongund
Copy link
Contributor Author

jongund commented May 26, 2021

@mcking65
This version has the changes based on the 5/25 call:

  1. Removed aria-describedby from button to the tooltip, and VoiceOver, Jaws and NVDA all announce the accesskey attribute information.
  2. Update selectors for landmarks to only include first navigation landmark

@jongund jongund changed the title Jump To Content: Add menu button to examples to implement WCAG bypass blocks requirement (2) Jump To Content: Add menu button to examples to implement WCAG bypass blocks requirement May 26, 2021
@jongund
Copy link
Contributor Author

jongund commented May 26, 2021

@jesdaigle @mcking65
I don't know why the regression test for menu button link example is failing, since it has not been modified. The test is replacing href values of the links before the link is activated with the enter key, so maybe some strange side effects I don't understand in this process.
The test passed all checks on my local machine.
Is this something Jes Daigle can look into?

@jongund
Copy link
Contributor Author

jongund commented May 27, 2021

I have completed another test to see if I can identify the problem with menu button link test failing.
I basically disabled the loading of JumpTo script in app.ja and I get the same error for the menu button link test, so not sure what is going on with that test.

@mcking65
Copy link
Contributor

mcking65 commented Jun 1, 2021

@zcorpan @jesdaigle

I merge #1921 and rebased this PR and the menu button tests are still failing. Thoughts?

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jongund

I love how this works on Windows with JAWS and NVDA in Chrome and Firefox. On example pages, if you know where you want to go, it is super fast. For instance, if you want to go to the keyboard section, just press the jump key and then the letter 'k' and enter. Because of the letter key navigation in the menu, it is much faster than any screen reader technique. Well, you could list headings, but that is a more difficult key press and, depending on screen reader, more steps.

Unfortunately, on MacOS with VoiceOver running, it is not as good. In Chrome, ctrl-opt-0 works if you pass the key through with ctrl-opt-tab first, which is quite awkward. But, in Safari, there is no way to get it to work that I can find.

I wonder if we should consider scripting the key to be opt-0 or alt-0 regardless of browser or operating system. Then it would work regardless of what screen reader is running. That would mean that we could never use alt+0 in an example, but I don't think that is an issue. Would it have any other downsides? Could it create other conflicts with example code?

@a11ydoer
Copy link
Contributor

a11ydoer commented Jun 4, 2021

@jongund
Could you kindly let me know the origianl repo of this widget? I am trying to undertand some details for this widget. For example, Is the visual user with mouse click for this "jump to" menu part of use case or not?

If I use the mouse click for the jump to menu, there is no focus ring around the landmark and heading items.

@a11ydoer
Copy link
Contributor

a11ydoer commented Jun 4, 2021

@carmacleod @smhigley
I am assigning both functional review and visual review to myself since you two have other priorites. I hope that is ok with you.

@jongund
Copy link
Contributor Author

jongund commented Jun 6, 2021

@mcking65 @a11ydoer
I am open to using a scripted shortcut and the click to open is useful to everyone, but especially to voice recognition users to quickly navigate to specific parts of a page. I am on vacation right now, so we hopefully can discuss on Tuesday

@zcorpan
Copy link
Member

zcorpan commented Jun 7, 2021

@mcking65

I merge #1921 and rebased this PR and the menu button tests are still failing. Thoughts?

Thanks. I'm a bit perplexed by that test. I've filed #1937 and opened a PR to comment out the test for now: #1938

@jongund
Copy link
Contributor Author

jongund commented Jun 8, 2021

@a11ydoer
The original pull #1860, it is referenced in the first comment.

@a11ydoer
Copy link
Contributor

a11ydoer commented Jun 8, 2021

Visual review -

  • more spacing underneath the "Jump to content" button
  • bigger font
  • better color contrast for "Jump to content"

@jongund
Copy link
Contributor Author

jongund commented Jun 10, 2021

@mcking65
I have updated the example to use a scripted shortcut key:

  1. For Windows and Linux: Alt+0
  2. macOS: Command+0

The appropriate keyboard short cut is now part of the button label.

Mobile devices currently do not have the scripted shortcut, or do want them to?

I also investigated the keyboard focus ring issue on the target landmark or heading raised by Jemma, not sure why it shows for keyboard activation and not for mouse click o a menu item since it uses the same code to move focus. But functionally it still scrolls to the location in the page and tab navigation starts from that point. So if it is important maybe someone else can look into it.

@a11ydoer
Copy link
Contributor

Android mobile testing- since the page is not mobile responsive, there is challenge for touch point size.

@a11ydoer
Copy link
Contributor

Android mobile testing- since the page is not mobile responsive, there is challenge for touch point size.

APG group decided on pushing this issue aside since APG redesign project is going on.

Copy link
Contributor

@a11ydoer a11ydoer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APG will take care of 1) mouse click focus issue, 2)mobile touch due to lack of responsive design later.

@jongund
Copy link
Contributor Author

jongund commented Jun 15, 2021

@mcking65
Jump To has been updated to use option+0 on macOS.

@jongund
Copy link
Contributor Author

jongund commented Jun 21, 2021

@mcking65
Scripting the option+0 key for macOS may cause internationalization issues, since the the keypress for option+0 generates some character other than zero. I account for this special character in the current script, but if someone is not using English keyboard it could generate another character may not not work for them.

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in chrome/windows, FF/Windows, Chrome/macOS, and Safari/macOs. Functionality with screen readers is good. The scripted key command works much better on Mac when VO is running. QuickNav has to be off, but that is a minor constraint beyond our control.

@mcking65 mcking65 removed the request for review from shirsha June 22, 2021 21:09
@jongund
Copy link
Contributor Author

jongund commented Jul 15, 2021

@mcking65
Is there anything else that needs to be done before this pull request is merged?

@mcking65 mcking65 merged commit b7383b7 into main Jul 18, 2021
@mcking65 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 Jul 18, 2021
@mcking65 mcking65 added this to the 1.2 Release 1 milestone Jul 18, 2021
@jongund
Copy link
Contributor Author

jongund commented Jul 19, 2021

@mcking65
Thank you for merging this pull request.

@jongund jongund deleted the jump-to branch July 19, 2021 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants