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

Color Palette: improve keyboard navigation #35292

Closed
3 tasks
Tracked by #35093
ciampo opened this issue Oct 1, 2021 · 10 comments
Closed
3 tasks
Tracked by #35093

Color Palette: improve keyboard navigation #35292

ciampo opened this issue Oct 1, 2021 · 10 comments
Labels
[Feature] Component System WordPress component system [Feature] UI Components Impacts or related to the UI component system [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components

Comments

@ciampo
Copy link
Contributor

ciampo commented Oct 1, 2021

What

Rewrite the Color Palette component to use the Composite component and to have role="listbox", so that each Color Palette becomes one tab stop.

Why

Tabbing through the many options in a Color Palette can be tedious, as each color represents a tab stop:

Kapture.2021-10-01.at.17.03.37.mp4

The Color Palette component is a good candidate for the listbox role and Reakit’s Composite. With Composite, users will be able to tab to the list of color options but then use the arrow keys to navigate within the list of options. This enables users navigating with the keyboard to more efficiently navigate the UI.

A/C

  • Partially rewrite the Color Palette component so that it uses the Composite component internally
  • Give the Color Palette component a role="listbox"
  • Each color palette should represent one tab stop. Inside each palette, each color can be selected by using the arrow keys.
@ciampo ciampo added [a11y] Keyboard & Focus [Feature] Component System WordPress component system [Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). labels Oct 1, 2021
@ciampo ciampo moved this to Todo in Andrew's onboarding May 3, 2023
@andrewhayward
Copy link
Contributor

I'm going to work on this, I'll be opening a PR soon // cc @ciampo

@andrewhayward
Copy link
Contributor

Bit of a brain dump...

My initial consideration was that perhaps the most appropriate semantics would be a set of radio inputs (aria-role="radio" or otherwise), wrapped in a radiogroup. However, the expected standard keyboard interaction for radio inputs is to select on focus, which would be different to the current behaviour.

It's quite possible that this change would not be an issue. In all likelihood most people don't use the keyboard to navigate this component, so would not be impacted. Those that do would probably understand the way radio controls work, particularly if using a screen reader which would present the appropriate semantics.

However, given that listbox has its own interaction model, which does not require (but does recommended) selection on focus, I think it makes sense to proceed with this idea first.

Happy to hear your thoughts though, @ciampo.

@andrewhayward
Copy link
Contributor

Given that @ciampo will be somewhat slower to respond for the foreseeable future, I'm wondering if @afercia / @alexstine / @joedolson / @mirka might have any input.

@alexstine
Copy link
Contributor

I think radio buttons are fine for this. Just need to make it clear to users that once an option is selected, there isn't a submit button or anything to confirm the change. Maybe an aria-live via wordpress/a11y.speak() to let the user know the colors were updated for X?

@mirka
Copy link
Member

mirka commented Jun 29, 2023

I am assuming that this is technically about the CircularOptionPicker component, which may or may not be used in a ColorPalette. For example, it's also used in DuotonePicker.

And the way that CircularOptionPicker is designed, it doesn't necessary have to be a single-select, and options can be deselected by clicking again. The consumer has freedom over that behavior. So radio semantics seem too restrictive to me.

@joedolson
Copy link
Contributor

@alexstine Technically, nothing is saved until the user saves their changes to the post or template, so I think that may not be necessary. And might be inaccurate, if it implied something had been saved when it hadn't. If the normal semantics of a radio selection are maintained, I think it should be fairly clear.

@alexstine
Copy link
Contributor

Sounds good. No alert needed then.

@andrewhayward
Copy link
Contributor

I'd already started working on a listbox fix for this, so despite the consensus seemingly being that radio options would probably be sufficient, I figured I'd put it up. I'll work on making a radio version next, which should be pretty similar.

@andrewhayward
Copy link
Contributor

Ended up doing more work on the listbox work for this, based on conversations around the various restraints and permutations about how the CircularOptionPicker can be used.

@ciampo ciampo moved this from Todo to In Progress in Andrew's onboarding Aug 30, 2023
@ciampo
Copy link
Contributor Author

ciampo commented Sep 4, 2023

Closing as fixed by #52255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Component System WordPress component system [Feature] UI Components Impacts or related to the UI component system [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components
Projects
Status: Done 🎉
Development

No branches or pull requests

6 participants