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

Post visibility panel better keyboard interaction #1885

Closed
afercia opened this issue Jul 13, 2017 · 11 comments
Closed

Post visibility panel better keyboard interaction #1885

afercia opened this issue Jul 13, 2017 · 11 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).

Comments

@afercia
Copy link
Contributor

afercia commented Jul 13, 2017

Splitting this out from #1312 and #1361.

The Post Visibility panel in the sidebar has room for keyboard interaction improvements. There are 3 points emerged during discussion on #1361.

screen shot 2017-07-13 at 18 28 44

  • the panel should close when pressing Escape
  • the panel should close when blurred: it already closes when clicking outside, it should also close when tabbing away
  • when the panel closes, focus should be moved back to the toggle that opened it
@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jul 13, 2017
@dpersing
Copy link

Hi @afercia,

I'm wondering about closing the panel on blur. Non-visual users who tab out of a panel may not realize they're exiting the panel until they've become more familiar with the interface, and may be frustrated if they have to manually tab back and open the previous panel section again.

I could also see the case where a sighted keyboard-only users want to visually check all their settings in the sidebar before saving their post, and closing the panels on blur would require them to check each one manually.

@afercia
Copy link
Contributor Author

afercia commented Jul 14, 2017

Hi @dpersing
yep I have some doubts about closing on blur. However, this visibility "panel" is different from all the other things in the sidebar. I'm calling it "panel" because of the lack of a more specific term :) let's call it "popover". It behaves more like a modal dialog, while all the other ones in the sidebar are more like accordions:

screen shot 2017-07-14 at 08 53 39

With this PR, when the popover closes on blur, focus is moved back to the toggle that opened it (the "Public" button in the screenshot above) and the toggle will have an aria-expanded="false". This way there's some (minimal) feedback for users. Not ideal, I know. Please also note that currently, when tabbing away from the popover, it stays open thus hiding what's behind.

If only the design was a bit different, we could probably try to make it a real dialog: adding a role dialog, constraining tabbing inside the dialog, etc. However, a dialog would need a "Save" or "Close" button because it requires explicit user interaction to complete the task.

I'd consider two options, any feedback and new ideas very welcome.

1
Add a "Close" button and remove closing on blur. This way the close action would always require an explicit user action. We don't have to assume any user workflow. The popover could then be a real dialog. Not to mention the code would be greatly simplified.

2
Keep closing on blur and improve the feedback: maybe adding an audible message with wp.a11y.speak()

Personally, I'd prefer 1, but that would require consensus on the slightly modified design. /cc @mtias
A quick example, quickly editing in-browser:

screen shot 2017-07-14 at 09 13 21

Maybe I'm biased, but ^^ looks even better to me.

@dpersing
Copy link

Ah ha @afercia ! 😄 That makes a lot more sense. Thank you for the clarification! I think closing that popup on blur totally makes sense; visually it looks like a tooltip, which I'd expect to close on blur. That said, sending focus back to the control that launched it might be unexpected, if it closes while a user is just tabbing forward. I would expect focus to just move to the next control in the main panel if I tabbed out of the popup.

Typically we (SA) recommend managing focus for tooltip-type interactions by sending focus to the tooltip container or the first element in it when it opens so users can immediately interact with it, but then letting the focus order progress naturally and closing the tooltip when a user tabs out of it to the next element on the page.

For modal-type interactions (which the Close control would indicate), we do recommend sending focus back to the control that activated it, since the user is explicitly exiting the interaction, but also keeping focus in the modal/browser chrome until its dismissed. This interaction doesn't feel as "urgent" as a modal would usually indicate, since it's not taking over, if that makes sense?

@afercia
Copy link
Contributor Author

afercia commented Jul 17, 2017

@dpersing I've updated the PR #1886 to change the behavior on blur: the "popover" closes but focus is not moved back to the toggle any longer, preserving the natural tab order. Can you please test the PR when you have a chance? Any thoughts and feedback welcome.

@afercia
Copy link
Contributor Author

afercia commented Jul 17, 2017

Note: as mentioned in #1886 (comment). there is an issue with Safari+VoiceOver that's still to address.

@joe-watkins
Copy link

@afercia Nice work :) Joe w/Simply Accessible here.

I tested the PR and things are working very well! The only thing I noticed missing was the recommendation @dpersing made regarding sending focus to the tooltip container or the first focusable element in the tooltip when activated. The tooltip content is next in the DOM but it would communicate a change in the UI if focus was managed programmatically there.

If you do shift focus programmatically there, I'd recommend consideration of converting the <button> to a <a> for the Private control. (It sure does look like a link :) The reason for this is that the role of the link will be communicated to the SR user along with the accessible name and the expected behavior of moving to a part of the page (the tooltip) wouldn't be a surprise.

<a href="#editor-post-public-0" aria-expanded="true" class="editor-post-visibility__toggle button-link">Private</a>

SA generally recommends that links be used for navigation to sections of same page content or new urls, and buttons for submitting data or making a change to the UI.

@afercia
Copy link
Contributor Author

afercia commented Jul 18, 2017

@joe-watkins thanks. I'm always a bit wary of moving focus. In this case, moving focus to the "popover" container or first focusable element would make the aria-expanded state change of the toggle button unavailable. Screen readers simply won't have the time to announce "expanded" because they have to start announcing the new focused element.
Moving focus makes certainly sense in other cases, for example modal dialogs, but I'm not sure it would be ideal in this case, since we're not treating the "popover" as a dialog.

@joe-watkins
Copy link

@afercia Sounds good!

Since the "popover" is next in line within the DOM it sure is discoverable. aria-controls is often bundled with a pattern like this but has such low/mixed support it may not be necessary.

Wish I could help w/Safari VO bug.. that is an odd one!

@afercia
Copy link
Contributor Author

afercia commented Jul 18, 2017

aria-controls

Yep, we discourage its use since... http://www.heydonworks.com/article/aria-controls-is-poop 😂

Wish I could help w/Safari VO bug.. that is an odd one!

Indeed! Any friends at Apple? 😉

@afercia
Copy link
Contributor Author

afercia commented Aug 9, 2017

With the changes in #2160, the PR associated to this issue is outdated. The issue should stay open though, as keyboard interaction with the "Popovers" is still a big problem. See also #2306

@afercia
Copy link
Contributor Author

afercia commented Aug 29, 2017

Closing in favor of #2306

@afercia afercia closed this as completed Aug 29, 2017
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 a pull request may close this issue.

3 participants