Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve publish panel accessibility; Add new publish landmark region; #7552

Merged
merged 1 commit into from
Jul 3, 2018

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Jun 26, 2018

Description

This PR makes a series of changes to make publish panel more accessible. The PR tries to follow the ideas discussed during WCEU contributors day and Accessibility chats on Wordpress slack.
Tries to address most parts of #4187.

Types of changes

Adds a new Publish landmark. When publish panel is visible the publish landmark contains the Publish panel. When the panel is hidden nothing is visible, unless we focus the landmark using Gutenberg landmark navigation (control + < or control + `) or we focus the button using tab, in this cases a button to open publish panel appears.

We automatically focus the contents of the publish panel when we open it. The first element of the Panel is the publish button, so it gets focused.

We now don't render other sidebars if the publish panel is visible. The other sidebars were not visible, but they were tabbable making keyboard users experience sub-optimal.

Added aria-expanded true to the close button on publishing panel. This button allows collapsing the panel so I feel it should have this aria property.

jun-26-2018 17-46-58

jun-26-2018 17-43-35

How has this been tested?

I did some smoke testing and verified publish works as before.
I used voice over and verified the new publish landmark appears.
I used Gutenberg landmark navigation ( control + < or `) and checked I can now see a new zone that allows opening the publishing panel. I verified that if I press that button the publish panel appears and the publish button is focused.

@jorgefilipecosta jorgefilipecosta added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jun 26, 2018
@jorgefilipecosta jorgefilipecosta self-assigned this Jun 26, 2018
@jorgefilipecosta jorgefilipecosta changed the title Improve publish panel accessible; Add new publish landmark region; Improve publish panel accessibility; Add new publish landmark region; Jun 27, 2018
Copy link
Contributor

@afercia afercia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From an a11y perspective, seems to me a very good improvement! Code-wise I'd defer to others with more expertise than me. The only things I can think of are related to some CSS tweaking, which could also be iterated later. More details in the comments.

@afercia
Copy link
Contributor

afercia commented Jun 29, 2018

@jorgefilipecosta thanks, seems to me this is a very good step forward 🎉

We now don't render other sidebars if the publish panel is visible. The other sidebars were not visible, but they were tabbable making keyboard users experience sub-optimal.

Super helpful, this helps a lot also when tabbing normally through the top bar

Added aria-expanded true to the close button on publishing panel. This button allows collapsing the panel so I feel it should have this aria property.

Makes perfectly sense.

Couple things about styling. When using the shortcut for the navigable regions, the blue outline baffled me because I had the impression the button was already focused. Pressing Enter or Spacebar did nothing though:

screen shot 2018-06-29 at 17 26 36

Then I realized I had to press Tab to actually focus the button and activate it. Would it be possible to add some spacing between the region and the button? Something like:

screen shot 2018-06-29 at 17 37 31

In the same area, there's also the "Skip to the selected block" button. To make it appear, select a block, then jump to the sidebar using Ctrl + ` and tab to the bottom of the sidebar. I have the impression the new landmark region partially overlies this button, as the region is always there. Maybe not a real issue because the button can be used with a keyboard anyway, but it's not possible to click on part of it.

screen shot 2018-06-29 at 18 01 38

More importantly, the region will cover any element in the bottom part of the sidebar, which may be a problem with the panels open and with medium / small screens:

screen shot 2018-06-29 at 18 03 23

Lastly, making the button style match as much as possible the "Skip" button would be a very nice touch.

@jorgefilipecosta jorgefilipecosta force-pushed the fix/make-plublish-flow-accessible branch from 65c31ca to 2baf324 Compare July 2, 2018 15:01
@jorgefilipecosta
Copy link
Member Author

Hi @afercia thank you for your feedback. It was addressed 👍

@jorgefilipecosta jorgefilipecosta merged commit 1b2db4e into master Jul 3, 2018
@jorgefilipecosta jorgefilipecosta deleted the fix/make-plublish-flow-accessible branch July 3, 2018 10:57
@afercia
Copy link
Contributor

afercia commented Jul 3, 2018

@jorgefilipecosta just noticed the "Open publish panel" button doesn't appear in Firefox when navigating to the new region. It's just me or it happens there too? When you have a chance, thanks very much!

@jorgefilipecosta jorgefilipecosta added this to the 3.2 milestone Jul 5, 2018
@jorgefilipecosta
Copy link
Member Author

Hi @afercia I was able to replicate the problem in Firefox. I will look into a fix. Thank you for reporting this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants