WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#33754 closed defect (bug) (fixed)

Post meta box toggles accessibility improvements

Reported by: afercia Owned by: wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version: 4.4
Component: Administration Keywords: has-patch commit
Focuses: ui, accessibility, javascript Cc:

Description

Thanks to the work done on #33544 the major accessibility issues are now fixed. There's still room for some improvements, most noticeably properly setting the aria-expanded initial attribute value on page load accordingly to their stored state. Without this aria-expanded can be set to true while the panel is actually closed.

Attachments (2)

33754.patch (5.5 KB) - added by afercia 5 years ago.
33754.2.patch (5.5 KB) - added by afercia 5 years ago.

Download all attachments as: .zip

Change History (10)

@afercia
5 years ago

#1 @afercia
5 years ago

  • Keywords has-patch added

First pass.

  • set proper aria-expanded initial attribute value on page load
  • toggle the buttons aria-expanded also when clicking on the headings
  • avoid setting aria-expanded on the headings
  • add type="button" attribute on the buttons, no need for preventDefault()
  • remove the title attribute
  • maybe better translatable string (paired with the ones used in the Customizer), open to suggestions
  • fix for a Firefox + NVDA bug as already done in the Customizer
  • completely hide the buttons when JS is off
  • minor CSS adjustments

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


5 years ago

#3 @joedolson
5 years ago

'Toggle panel: %s' seems fine to me; it's functional and concise, which is all I think is particularly important.

I was unaware that adding 'type="button" would make preventDefault() irrelevant. That seems incredibly nonintuitive to me; but definitely good to know!

(Off to revise a whole ton of old code...)

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


5 years ago

#5 @wonderboymusic
5 years ago

  • Owner set to afercia
  • Status changed from new to assigned

If you're going to use $(this) a bunch of times, set it to $el once and then use that. Also, if you can, use $(e.target) instead, makes porting procedural code to methods easier.

@afercia
5 years ago

#6 @afercia
5 years ago

  • Keywords commit added

Refreshed patch as per @wonderboymusic suggestion. Noticed $( this ) and $( event.target ) are different in this case (the buttons have a span inside) so used the $el approach. Except for the each() loop where maybe setting variables doesn't make much sense and $( this ) is more appropriate.

#7 @wonderboymusic
5 years ago

  • Owner changed from afercia to wonderboymusic

#8 @wonderboymusic
5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 34893:

Meta Boxes: reboot some of the code in postbox.js to support aria-expanded attribute toggling and to properly reference static class properties.

Props afercia, wonderboymusic.
Fixes #33754.

Note: See TracTickets for help on using tickets.