[wp-trac] [WordPress Trac] #29958: collapse menu keyboard accessibility

[WordPress Trac]

#29958: collapse menu keyboard accessibility
-------------------------------------+-------------------------------------
 Reporter:  afercia                  |       Owner:  jorbin
     Type:  defect (bug)             |      Status:  assigned
 Priority:  normal                   |   Milestone:  4.3
Component:  Administration           |     Version:  4.0
 Severity:  normal                   |  Resolution:
 Keywords:  needs-testing has-patch  |     Focuses:  ui, accessibility,
  needs-refresh                      |  administration
-------------------------------------+-------------------------------------
Changes (by GaryJ):

 * keywords:  needs-testing has-patch => needs-testing has-patch needs-
               refresh


Comment:

 Observations on patches:

 .4:
 * Button content is `<span class="collapse-button-label">' . __( 'Collapse
 menu' ) . '</span>' . '<span class="screen-reader-text">' . __( 'Expand
 menu' ) . '</span>'`. With no JS / CSS, this is going to read both labels
 for certain groups of users.
 * Since the menu can't be manually collapsed without JS, arguably the
 whole list item markup that contains the button should only be added via
 JS anyway, even for the default state.

 .6:
 * If all the markup is going to be added via JS, then the click event
 should be delegated from a parent element that will always be present.
 * Overall, I like the JS refactoring.
 * What's the point of `return respWidth || ''; `? The variable `respWidth`
 should always be populated, and non-zero. If it's not, then returning an
 empty string doesn't seem like the best option.
 * Instead of the logic being in `getMenuState()`, I'd rather see a `data-`
 attribute set at the time the body classes are amended, so that all logic
 is handled in one place. This can then simply be read as needed.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/29958#comment:40>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform