WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#33228 closed enhancement (fixed)

Customizer initial focus

Reported by: afercia Owned by: afercia
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.2
Component: Customize Keywords: has-patch needs-testing commit
Focuses: ui, accessibility, javascript Cc:
PR Number:

Description

Hat tip @folletto for pointing this out.

When the customizer loads, focus is moved to the "Close X" link. See screenshot:

https://cldup.com/KOMuTozeWj.png

From a visual/UI perspective, this makes difficult to craft a proper focus style for the "Close X" (and other controls) since using a "strong" style and having this element always initially focused would draw users' attention on this element on Customizer load, which is inappropriate.

Moreover, as far as I can tell, moving initial focus to the Customizer is only necessary when the Customizer gets loaded in the iframe "overlay". Instead, when it loads in the normal customizer.php page, there's no need to change the native focus order.

Attachments (4)

33228.patch (1.1 KB) - added by afercia 4 years ago.
33228.2.patch (2.0 KB) - added by afercia 4 years ago.
33228.3.patch (4.7 KB) - added by afercia 4 years ago.
33228.diff (4.2 KB) - added by afercia 3 years ago.

Download all attachments as: .zip

Change History (26)

@afercia
4 years ago

#1 @afercia
4 years ago

  • Keywords has-patch added

The proposed patch tries to use JavaScript to move focus to the Customizer only when necessary, i.e. when the customizer loader is used. The initial focus is moved on the overlay container, in a similar way WordPress already does for some modal dialogs.
No more need for setTimeout(), just use the customizer loader loaded() callback.

Last edited 4 years ago by afercia (previous) (diff)

This ticket was mentioned in Slack in #core-customize by afercia. View the logs.


4 years ago

#3 @afercia
4 years ago

There's probably a better, simpler, way to do this and it's already used by the Customizer in other places. Using visibility: hidden; and visibility: visible; allows to hide UI sections and have inner sections still visible. It's an interesting method which can be used with "full size" overlays.

In this case, hiding the main body and setting visibility: visible; on the Customizer overlay would make it the only visible and focusable content. It would almost behave like a "normal page", and let browsers handle focus and tab order like in a normal page.

@afercia
4 years ago

#4 @afercia
4 years ago

Refreshed patch with the visibility method. Notice this would also constrain keyboard navigation with the tab key inside the Customizer overlay, which is a big plus.

P.S. about the CSS class names, open to suggestions for better names.

Last edited 4 years ago by afercia (previous) (diff)

#5 @afercia
4 years ago

Testing with NVDA, looks like it will read out all the elements in the overlay as "blank" when moving around with keystrokes (arrows, keystrokes for headings etc.) while everything works when tabbing through focusable elements. Seems related to timings since using a setTimout set to "0" to apply visibility: visible; fixes it.

setTimeout( function() {
	Loader.element.addClass( 'visible-visibility' );
}, 0 );

Will try to investigate about a better way to make sure CSS classes get always applied with the right timing.

#6 @folletto
4 years ago

You're amazing! :)
Thanks for looking into this. The idea I had in mind was to create an out-of-flow invisible element that got the initial focus, so the next "tab" would be correct and it could guide the user, but your path seems more sensible. :D

#7 @afercia
4 years ago

About NVDA reading out "blank", noticed that after refreshing the NVDA buffer pressing NVDA key + F5 all elements get announced correctly and everything works. So, it appears NVDA needs a little help to understand the visibility property has changed. Best way I've found is setting an aria-busy attribute on the body to be dynamically updated.

Thinking this should be done also for the theme installer, which uses a similar "full overlay" and has the initial focus set on the close button (see screenshot below). This would also constrain keyboard navigation and fix #27705.

https://cldup.com/1kdftKmWAp.png

@afercia
4 years ago

#8 @afercia
4 years ago

Refreshed patch fixes also the Theme installer overlay focus, see #27705. Eventually this patch can be split in two but for now I'd recommend to keep all together for testing purposes, will ask the accessibility team to test especially with screen readers running and Safari + VoiceOver.

Three main changes in this patch:

  • no more need of new CSS classes, just use the existing ones
  • in the Theme installer, added a callback for the iframe load event
  • the callback is used also to remove the spinner which currently is always "spinning" in the background and even if it's not visible, is causing high CPU usage especially on old hardware

Any thoughts, improvements, and feedback welcome.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


4 years ago

#10 @afercia
4 years ago

In order to test for accessibility, go in:

  • Admin Dashbord and click on the "Customize Your Site" in the Welcome panel (check the screen options to enable the Welcome panel)
  • Appearance > Themes and click on a Theme's "Live Preview" link
  • Appearance > Themes > Add New and click on a Theme's "Preview" link

The Customizer and the Theme Installer will open in a "full overlay" that... well overlays the current screen. Before the patch, it's still possible to tab to the underlying content. After the patch, the underlying content should be completely hidden and not focusable. Also, the initial focus placed on the overlay "Close X" is no more necessary.

#11 follow-up: @Gab Nino
4 years ago

Hello @afercia,

Rian asked me to test this patch with VoiceOver + Safari (8.0.7).

Unfortunately the patch is not working. When the full overlay content opens, the focus remain on the underlying content. VoiceOver totally ignores the overlay content.

Is there other person that could perform the test with VO to compare my results?

As usual, I prepared a short video to demonstrate the test with VoiceOver.
http://youtu.be/fxVuTn2OMVM

Let me know if I can help you with something else.

Greetings :D

Gab

#12 in reply to: ↑ 11 @afercia
4 years ago

Replying to Gab Nino:
@Gab Nino thanks very much for testing :)
Your video is very clear, weird that Safari+VoiceOver still give focus to hidden elements :( Wondering if there are caching issues (please be sure to empty Safarì's cache) or if it's a Safary/VoiceOver bug. I've prepared a reduced test case, a quick example that does the same thing: hides the main body with visibility: hidden and sets visibility: visible on the modal dialog. Would greatly appreciate testing and feedback by VoiceOver users :)
http://s.codepen.io/afercia/debug/NqoBNR
Please don't focus on the implementation details, it's just a quick example, we should just test if the modal content is the only visible and focusable content in the page. Thanks :)

#13 @Gab Nino
4 years ago

Hello afercia,

The example worked however after clicking the red button I need to do control+option+right arrow twice to move the focus from the underlying content to the modal content.
Going back from the modal content (closing the modal) works fine too.

:D

#14 @celloexpressions
4 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect (bug) to enhancement

@afercia any updates here?

#15 @afercia
3 years ago

  • Keywords needs-testing added
  • Milestone changed from Future Release to 4.7

Looking back at this, just tested on Safari and looks like it wasn't getting the updated themes.css stylesheet even when pressing Shift-Command-R. I had to empty the cache from the Develop menu. So in order to properly test this patch, pleas try this:

If you haven't already, go in Safari > Preferences > Advanced tab and check "Show Develop menu in menu bar". Then just empty the caches from the Develop menu:

https://cldup.com/GBBJGnRnwP.png

After this, everything seems to work nicely even in Safari+VoiceOver and the customize-loader "overlay" is actually the only content available:

  • no more need to set initial focus, any related JS can be removed
  • the overlay behaves like a normal web page, focus management is a native browser thing
  • keyboard navigation is "natively" constrained within the overlay, this fixes also #27705

Needs a bit more testing (Windows) and cleaning. Moving to 4.7.

There are some more improvements to consider in this area but they should probably go in a separate ticket:

  • the "X" link hidden text is different, sometimes it's "Close" sometimes "Cancel", I'd propose to simplify and just use "Close" whether it's the live preview or customizer or theme installer
  • order of elements: the "X" link should probably be the first control in the tab order
  • consider to use wp.a11y.speak() to announce the customize-loader overlay has loaded

This ticket was mentioned in Slack in #design by afercia. View the logs.


3 years ago

#17 @afercia
3 years ago

  • Keywords needs-refresh added

@afercia
3 years ago

#18 @afercia
3 years ago

  • Keywords needs-refresh removed

Refreshed patch.

#19 @celloexpressions
3 years ago

  • Keywords commit added

Thanks for the updqate @afercia. The patch looks good to me. I tested on Chrome on Windows 10 and was able to navigate in and out of the customizer with keyboard-only both with and without customize-loader, with keyboard focus moving and being constrained as expected.

Let's get 33228.diff committed, and we can follow up if any other issues come up.

#20 @afercia
3 years ago

Thanks @celloexpressions can you confirm it fixes also the Theme installer so also #27705 can be closed? 🙂

#21 @celloexpressions
3 years ago

@afercia yes looks good there as well. Hadn't noticed that ticket :)

#22 @afercia
3 years ago

  • Owner set to afercia
  • Resolution set to fixed
  • Status changed from new to closed

In 38520:

Accessibility: Improve the Customizer and Theme Installer initial focus.

The Customizer and Theme Installer open in full overlays that need to receive
focus. Also, keyboard navigation should be constrained within the overlays. Using
CSS visibility to hide all the content except the overlays, makes them the only
available and focusable content and allows browsers to handle focus natively.

See #29158.
Fixes #33228, #27705.

Note: See TracTickets for help on using tickets.