#42110 closed defect (bug) (fixed)
Keyboard focus in Heads up! modal dialog under Appearance >> Editor
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | 4.9 |
Component: | Customize | Keywords: | has-patch |
Focuses: | accessibility | Cc: |
Description
When you open Appearance >> Editor Heads up! modal dialog opens. That's really cool!
But I noticed that keyboard focus doesn't stay inside the modal dialog when I use Tab or Shift+Tab.
Here is more info about modal dialogs.
Attachments (1)
Change History (17)
#2
@
7 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to 4.9
- Type changed from enhancement to defect (bug)
#3
@
7 years ago
In this case initial focus could be set to dialog title. But we can discuss these later on in a11y team. I need sleep now :)
#4
@
7 years ago
- Owner set to afercia
- Status changed from new to assigned
Yes this would need some a11y treatment not just constraining tabbing but also to make the text being announced automatically by screen readers.
@helen I think the button needs also the .button
class together with .button-primary
otherwise it has the wrong focus style (on macOS webkit, I know you're mostly on FF) and it's not responsive.
#5
@
7 years ago
@afercia while you're at it, there is some tab constraining also needed for the global notification overlays new in the Customizer. See todo. A quick way to throw up such an overlay is to put this in your console:
wp.customize.notifications.add( new wp.customize.OverlayNotification( 'test_tab_constraining', { message: 'Will it be constrained?', dismissible: true } ) );
The gotcha here is that without dismissible
there aren't actually any tabbable elements in the notification's container. So I was not sure the best way to go about that.
#6
@
7 years ago
@westonruter since it's full-screen I think the easier way in this case would be making the body visibility: hidden
and the notification visibility: visible
. This way, the only perceivable content in the page would be the notification content.
As per the interaction, I'd need to see a real use case. For example, adding a role="alert"
seems more correct to me instead of using wp.a11y.speak
, but this really depends on the nature of the notification content.
#7
@
7 years ago
@afercia thanks for the reminder on using visibility
. The three current overlay notifications are:
- Downloading a theme to install
- Switching to a different theme in the Customizer to preview
- Trashing/reverting unpublished changes
These global overlay notifications are really just different styles of the other notifications previously added to the Customizer in #35210, #38794, and #34893.
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
7 years ago
This ticket was mentioned in Slack in #core by melchoyce. View the logs.
7 years ago
#12
@
7 years ago
Working on a patch for the theme/plugin editor modal and fighting a bit with IE11 and JAWS 17. Seems JAWS doesn't like a modal dialog that appears during page load.
Comparing with this example: http://pauljadam.com/demos/aria-role-alertdialog.html (linked from https://www.w3.org/WAI/GL/wiki/Using_aria-alertdialog_to_Identify_Errors), the code is basically the same I'm using on the patch, the only difference is the alertdialog
in this example appears after user activation and it works nicely.
Instead, when the dialog appears on page load, JAWS doesn't announce anything.
Worth noting the default screen readers behavior on page load is starting reading the page content from the beginning. They probably have troubles in getting the alertdialog because there's no change in the DOM, the dialog is there since the beginning.
Needs more work and testing but I'd suggest to display the modal on DOM ready to start with.
#13
@
7 years ago
- Keywords has-patch added; needs-patch removed
Spent an insane amount of time trying all the possible variations of ARIA roles (dialog, alertdialog, and also alert), properly targeting elements within the modal with aria-labelledby
and aria-describedby
but wasn't able to get consistent results across different combinations of browsers and screen readers. Tested all the variations with Firefox + NVDA, Firefox + JAWS 17, IE 11 + JAWS 17, IE11 + NVDA, Safari 11 and VoiceOver.
There are a couple reasons that might explain this. First of all, this is not exactly a dialog (not as intended in ARIA) because it gets displayed on page load and it's not activated by an user action. It looks like a modal dialog, but it's just page content rendered during page load.
Secondarily, there's a lot going on in the background including a full instantiation of the editor. That's a lot of DOM manipulation that happens at the same time the modal gets displayed and could easily confuse screen readers.
Long story short, I'd propose to not use ARIA in this very peculiar case and just ensure the warning text is available to all users.
42110.diff does the following:
- hides all the page content from assistive technologies using
aria-hidden="true"
, except the modal - uses
wp.a11y.speak()
to make screen readers announce the modal message - constrain tabbing within the modal
This way, even if users miss the speak
message, the modal content is actually the only perceivable content in the page. When closing the modal, aria-hidden="true"
gets removed, making the page content available again to assistive technologies.
Also, when closing the modal, focus is not moved within the editor any longer. I'd propose to don't do anything with focus. Content navigation should start from the document root to match the native behavior when normally visiting the plugin/theme editor pages.
I didn't do anything for focus handling in the initial commit besides initial setting on load and on close, neither of which I'm even sure is in a good spot. Right now it initially focuses on the "I understand" button, but I don't know if that interferes with reading order, and on close focuses inside the code editor, which doesn't completely trap you in but maybe is confusing? Pinging @afercia for opinions on those + whatever needs to be done to "trap" focus.