WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#31575 closed defect (bug) (fixed)

Press This: post options toggle accessibility improvements

Reported by: afercia Owned by: azaozz
Milestone: 4.2 Priority: normal
Severity: normal Version: 4.2
Component: Press This Keywords: has-patch dev-feedback
Focuses: ui, accessibility, javascript Cc:

Description

Currently, there are 2 buttons to toggle the post options panel,

  1. button with "tag icon" + "Show post options" hidden text
  2. button with "Done" text

and their visibility is toggled depending on the panel visibility.

For screen reader users, having controls that "appear and disappear" is confusing, I would propose to simplify and use just one button. Worth noting the "add category" button is a very similar case and uses just one button.

The proposed patch tries to:

  • use just one button to toggle the post options panel
  • one button, one JS function: toggleSidebar() instead of openSidebar() and closeSidebar()
  • add aria attributes and screen-reader-text
  • minor CSS tweak to make the add category button clickable area a bit bigger, see before/after screenshot

Feedback about the JavaScript part would be greatly appreciated :)

Outstanding issues:

  • the "aria-expanded" attribute should probably be used on the options panel, not on the button
  • the options panel should use a proper ARIA role and have a label

Would like to discuss these points with the accessibility team, not sure about what to do here.

add category clickable area:

https://cldup.com/eqQb0BQsw3.png

Attachments (3)

31575.patch (6.7 KB) - added by afercia 6 years ago.
31575.2.patch (6.2 KB) - added by afercia 5 years ago.
31575.3.patch (4.8 KB) - added by azaozz 5 years ago.

Download all attachments as: .zip

Change History (18)

@afercia
6 years ago

#1 @afercia
6 years ago

  • Keywords has-patch dev-feedback added

#2 @stephdau
5 years ago

  • Keywords needs-refresh added

@afercia
5 years ago

#3 @afercia
5 years ago

Patch applied cleanly for me (using TortoiseSVN) refreshed anyway against 31898.

#4 @afercia
5 years ago

  • Keywords needs-refresh removed

#5 @stephdau
5 years ago

31575.2.patch works: https://cloudup.com/c3SQEI99jZb
31575.patch didn't: https://cloudup.com/cYkNELBHvsR

I'll test the new one.

#6 follow-up: @stephdau
5 years ago

31575.2.patch is working well, and as advertised. I don't have much feedback on the JS part, it could be done in countless ways, but I do not see anything wrong with the one you chose.

@azaozz?

#7 in reply to: ↑ 6 @azaozz
5 years ago

Replying to stephdau:

Don't think this approach is good. Currently we have two buttons and two simple JS functions to handle open/close. This is all pretty simple, easy to read, understand, etc. Why should we switch to one complex "dynamic" button with many spans in it and one combined JS function that is longer and harder to read than the two? I'm not sure if all screen readers are aware of showing/hiding components on the page but having two buttons to control open/close is pretty standard. Also "toggle" usually implies we don't care of the current state much, just want to change it. That's not true in this case though :)

If the word "Done" is not enough for screen readers, lets add the "Close post options" screen-reader-text there too. Moving the aria-expanded to the panel makes sense. Don't thing it should be on the button as well...

Also, keep in mind all of this is only needed on small screens: tablets, phones, etc. Not even sure how useful the screen readers are there.

Last edited 5 years ago by azaozz (previous) (diff)

#8 @afercia
5 years ago

Things have changed a bit now that the Post Options panel automatically closes when it loses focus but still there are issues I would recommend to take into consideration. Using a screen reader you can do things you usually can't with just a browser. For example, you can leave focus "in place" and make the screen reader read out other parts of the screen.

  • Tab to the Post Options button, press Enter, the panel opens and focus is moved to the panel
  • do your things in the panel
  • now you can keep focus on the panel (it will still be open) and explore other parts of the page, for example you can make the screen reader start reading out from the beginning of the page
  • you stop on the Post Options button which now says just "Done", you have no idea what "Done" refers to
  • activate the button, the button "disappears" and at this point there's a focus loss, which is one of the most annoying things for screen reader users

This is the main reason why there should be just one button: focus would still be on the button, then we should make the button change state and label.
I would still recommend to have just one button, if possible :)

two buttons to control open/close is pretty standard

I'm not sure in WordPress there are two buttons for close/open in any place, but I may be wrong. Worth noting that in Press This itself, the "Add Category" button is just one "toggle" button and maybe having 2 different design paradigms in the same interface is not the best choice?

About the JS part, I just tried to make it work :) happy to see different, better, simpler, implementations :)

@azaozz
5 years ago

#9 @azaozz
5 years ago

Replying to afercia:

Okay, sounds good. How about 31575.3.patch? It's a bit simpler and a bit more readable.

#10 @afercia
5 years ago

Thanks very much @azaozz :) tested with Firefox + NVDA and Chrome + ChromeVox the button works nicely. Just one thing: setting aria-expanded on a non-focusable element makes this information not available for screen readers, they just can't focus that div:

<div class="options-panel" tabindex="-1" aria-expanded="true">

Thinking a bit about this, I'd say either change this approach and do the aria-expanded thing on the button or further simplify and remove the aria-expanded thing. After all, the button status is implicitly conveyed by its text because it will be announced as "Show post options" or "Hide post options". I would be in favor of the latter option, but I'd like to have a feedback from the accessibility team.

@michaelarestad: sorry for bothering :) maybe the Add category button clickable area needs some love:
https://cldup.com/eqQb0BQsw3.png

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


5 years ago

#12 follow-up: @joedolson
5 years ago

Since the text clearly indicates the correct action, having aria-expanded on the button would act to provide a repeat of that information, and is probably not really necessary. But it's definitely not appropriate to have it on the object that actually gets expanded; the purpose of the attribute is to indicate whether an object is currently expanded or not, and should be read out at the time that the control to perform the expand/collapse action is focused, so that the user knows what the control will do.

#13 @azaozz
5 years ago

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

In 31925:

Press This: fix accessibility for the post options "sidebar". Fix size of the Add Category button.
Part props afercis, joedolson. Fixes #31575.

#14 in reply to: ↑ 12 @azaozz
5 years ago

Replying to afercia:
Replying to joedolson:

Sure. Removed the aria-expanded attribute and added the size fix for the Add Category button.

#15 @DrewAPicture
5 years ago

  • Milestone changed from Awaiting Review to 4.2
Note: See TracTickets for help on using tickets.