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

Circular Color Picker: Improve HTML Semantics and Interactions #20372

Open
jeryj opened this issue Feb 21, 2020 · 6 comments
Open

Circular Color Picker: Improve HTML Semantics and Interactions #20372

jeryj opened this issue Feb 21, 2020 · 6 comments

Comments

@jeryj
Copy link
Contributor

@jeryj jeryj commented Feb 21, 2020

The Circular Color Picker is currently marked up as grouping of <div>s.

To reproduce
Steps to reproduce the behavior:

  1. Go to the circular color picker
  2. Tab moves to the next color

Expected behavior
A clear and concise description of what you expected to happen.

  1. Go to the circular color picker
  2. Right/Left/Up/Down should move to the relevant next/previous items
  3. Tab moves to the next tabbable item (like the custom color button)

Screenshots
Circular Color Picker - Group of colored circles with a custom color link/button and clear button

Proposal to be discussed
I think the ideal is to convert this to a <fieldset> with a <legend> and group of radio buttons (styled to look identical to the current implementation). This HTML structure would be semantic, and give us all the expected keyboard interactions. Because it's ideal that each radio grouping has at least one selection, we could add in a "Custom Color" circular option as well that is the value of whatever is in the color selector.

If this isn't possible, then we could convert this to a grouping of buttons using an <ul> and each color <button> being inside the <li>. There may be better aria states to communicate this grouping as well.

@marekhrabe

This comment has been minimized.

Copy link
Contributor

@marekhrabe marekhrabe commented Mar 1, 2020

I've tried building a super simple experiment with radios:

Screenshot 2020-03-01 at 11 41 45
https://jsfiddle.net/marek/hrno5w6j/

Arrow keys work (almost) as expected and they definitely feel better than the current implementation where the arrow keys move your cursor in the editor, possibly changing focus to a different block and completely hiding the color picker. I haven't tested yet with an assistive technology but I expect radios to have a more logical structure than separate buttons.

Advantages:

  • radio inputs seem like a logical choice as they are all focused as "one control" (single tab) and further interactions work using arrow keys
  • it makes sense semantically as we are selecting one value from a list of things that are all visible

Things I've noticed:

  • arrow keys don't move you in two axis but they only move you back/forward in the list of radios in the order they appear in the markup. That means both Left key and Up key do the same - select a previous option and the same is true for Right/Down - moving you forward. I don't mind this much but it could be a surprising interaction.
  • there is a feature of the current implementation that clears the color setting if you click a color swatch that is already selected. Standard radio inputs don't work this way. We still have the same clearing feature accessible using the "Clear" button so maybe we don't need to repeat it at individual swatches

I don't mind either of these but we need to evaluate it in context of already having an implementation live and we need to look at it from accessibility perspective too. We can most likely code these non-standard interactions on top of radio buttons but I think we might be better sticking with the most standard way possible.

Would you think this could be a good way forward?

@jeryj

This comment has been minimized.

Copy link
Contributor Author

@jeryj jeryj commented Mar 2, 2020

Thanks for putting this together, @marekhrabe!

I like the idea of using radio buttons.

arrow keys don't move you in two axis but they only move you back/forward in the list of radios in the order they appear in the markup. That means both Left key and Up key do the same - select a previous option and the same is true for Right/Down - moving you forward. I don't mind this much but it could be a surprising interaction.

I agree with you, but my guess is that most people who are not familiar with this arrow interaction are not people who generally use keyboards to interact with the inputs.

We can most likely code these non-standard interactions on top of radio buttons but I think we might be better sticking with the most standard way possible.

100% agree. Coding non-standard interactions feels like digging ourselves into a hole.

My biggest concern with using radio buttons is that the default radio button interaction is to move to and select the next radio button. So, if someone had a custom color set already, then cycling through the radio buttons would clear their selection. Since these don't actually look like radio buttons:

  • I don't expect this would be clearly indicated to the user what is happening (their cycling through is selecting the color)
  • I'd also be worried that if they did know it was selecting it, they wouldn't actually want that interaction.

I'm going to have to think over this some more... I'm so glad you put together that little interaction and your thoughts on it. It's very, very helpful!

@marekhrabe

This comment has been minimized.

Copy link
Contributor

@marekhrabe marekhrabe commented Mar 3, 2020

My biggest concern with using radio buttons is that the default radio button interaction is to move to and select the next radio button. So, if someone had a custom color set already, then cycling through the radio buttons would clear their selection.

This is an important one, thanks for mentioning! I kinda like that cycling through the options provides you with an immediate "preview". What might be concerning is possibly creating way too much undo operations than expected. I believe each arrow key press would leave an undo operation so in order to go back to whatever you started with, you might need to trigger undo for each color selected on the way.

@ZebulanStanphill

This comment has been minimized.

Copy link
Contributor

@ZebulanStanphill ZebulanStanphill commented Mar 3, 2020

Creating a bunch of undo operations would happen with any radio option, though, wouldn't it? You don't need to use the arrow keys anyway unless you want to change the option, as far as I can tell. If you just want to get to the next control on the page, you would just press tab. I would assume there's no reason to use the arrow keys to navigate through radio options other than to select a new option, so if it's consistent with other radio controls I would consider that a plus.

I also think that having to tab through each option (which is the current behavior) before you can get to the next control is not good. At the very least, I think we should change this so that the arrow keys are used for switching focus between options within a color picker, and tab/shift-tab is used for moving in/out of the control as a whole.

@jeryj

This comment has been minimized.

Copy link
Contributor Author

@jeryj jeryj commented Mar 3, 2020

Creating a bunch of undo operations would happen with any radio option, though, wouldn't it? You don't need to use the arrow keys anyway unless you want to change the option, as far as I can tell. If you just want to get to the next control on the page, you would just press tab. I would assume there's no reason to use the arrow keys to navigate through radio options other than to select a new option, so if it's consistent with other radio controls I would consider that a plus.

All excellent points 🙌

I'm 100% in favor of using whatever the most semantic HTML element for this is and not rebuilding the interactions. If that's a radio, then let's give it a try :)

@jeryj jeryj changed the title Circular Color Picker: Improve HTML Semantics and Accessibility Interactions Circular Color Picker: Improve HTML Semantics and Interactions Mar 5, 2020
@enriquesanchez

This comment has been minimized.

Copy link
Contributor

@enriquesanchez enriquesanchez commented Mar 13, 2020

This issue was discussed today during the Accessibility Team's weekly meeting.

We found that the color picker component is already using a fieldset and legend.

The consensus is that the team has no strong preference over which method to use. We think Radio buttons could work, since they have a similar interaction as the current implementation.

One concern was that with the current implementation, you can press a button to disable the color (and reset to default color), but with radio buttons that’s not possible natively. The only option would be the “Clear” button or add a radio option of "default".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.