[wp-trac] [WordPress Trac] #39029: Twenty Seventeen: consider to remove the onResizeARIA() function in navigation.js

[WordPress Trac]

#39029: Twenty Seventeen: consider to remove the onResizeARIA() function in
navigation.js
-------------------------------------+-------------------------------------
 Reporter:  afercia                  |      Owner:
     Type:  defect (bug)             |     Status:  new
 Priority:  normal                   |  Milestone:  Awaiting Review
Component:  Bundled Theme            |    Version:  trunk
 Severity:  normal                   |   Keywords:  has-screenshots needs-
  Focuses:  accessibility,           |  patch
  javascript                         |
-------------------------------------+-------------------------------------
 I know some of the functionalities in Twenty Seventeen are ported and
 adjusted from Twenty Sixteen, but that doesn't mean they shouldn't be
 improved or completely reconsidered :)

 The `onResizeARIA()` function is meant to update some aria attributes on
 the navigation menu and the menu toggle. By the way, there are significant
 differences in the way it works on Twenty Sixteen and on Twenty Seventeen
 and probably it should be completely removed. You can see its current
 revision here: https://core.trac.wordpress.org/browser/trunk/src/wp-
 content/themes/twentyseventeen/assets/js/navigation.js?rev=39419#L107

 As far as I see, it also doesn't work correctly under some circumstances,
 for example when expanding the menu and then resizing a few times, the
 `aria-expanded` attributes may be not synced and have a wrong value, see
 screenshot below:

 [[Image(https://cldup.com/WfPNj06i5F.png)]]

 Starting from the aria attributes set '''on the navigation menu''': on
 Twenty Sixteen they were set on the `<nav>` element while on Twenty
 Seventeen they're set on the unordered list `<ul>`. On Twenty Sixteen this
 makes a bit more sense, since the `<nav>` element has an aria role so it
 is supposed to be available to assistive technologies:

 `<nav id="site-navigation" class="main-navigation" role="navigation" aria-
 label="Primary Menu" aria-expanded="true">`

 Anyway, this `aria-expanded` attribute is not announced (see screenshots
 below, using VoiceOver and NVDA) so it has no practical effect.

 [[Image(https://cldup.com/I7gSRDxImX.png)]]

 [[Image(https://cldup.com/FdzdRyu9Hh.png)]]

 On Twenty Seventeen, it makes even less sense because the `<ul>` element
 is not available to assistive technologies in any way: it's not focusable
 and doesn't have any aria role so any aria attribute set on it doesn't
 have any effect.

 To start with, I'd propose to remove anything that refers to
 `siteNavigation` from this function. After doing that, here's what's left:

 {{{
 function onResizeARIA() {
         if ( 'block' === $( '.menu-toggle' ).css( 'display' ) ) {

                 if ( menuToggle.hasClass( 'toggled-on' ) ) {
                         menuToggle.attr( 'aria-expanded', 'true' );
                 } else {
                         menuToggle.attr( 'aria-expanded', 'false' );
                 }
         } else {
                 menuToggle.removeAttr( 'aria-expanded' );
                 menuToggle.removeAttr( 'aria-controls' );
         }
 }

 $( document ).ready( function() {
         $( window ).on( 'load.twentyseventeen', onResizeARIA );
         $( window ).on( 'resize.twentyseventeen', onResizeARIA );
 });
 }}}

 On `load` and `resize` events, the menu toggle is displayed when the
 viewport width is under '48em' and hidden otherwise. By the way, I don't
 see the point in updating its `aria-expanded` and `aria-controls`
 attributes here.

 Trying to explain my point in the simplest possible way, please correct me
 if I'm wrong or I'm missing something. Let's read that `if/else` like:
 "when the menu toggle is displayed or hidden":
 - the `aria-controls` attribute shouldn't be removed (also notice that
 currently it gets removed but never restored): when the menu toggle is
 hidden, it's not available to assistive technologies so there's no need to
 remove `aria-controls`
 - all that's left now is the `aria-expanded` attribute:
  - on load: whether visible or hidden, the navigation menu is always
 collapsed and the menu toggle already has a default `aria-
 expanded="false"` so why all this should run on load?
  - on resize: the menu toggle `aria-expanded` attribute should be updated
 just when clicking on it; when it gets hidden or visible again, for
 example when changing a tablet orientation from landscape to portrait or
 vice-versa, its previous state should persist: if it was expanded, then it
 should still be expanded and vice-versa so why all this should run?

 If I'm not missing something, I'd propose to completely remove this
 function and also any other handling of aria attributes on the
 `siteNavigation` element, as they don't bring in any accessibility
 benefit.
 Any thoughts welcome.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/39029>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform