ToggleGroupControl: Refactor to handle de-selection and multiple selections #44168
Conversation
|
@jasmussen @jameskoster We're ready for design review 🙏 |
I also prefer the borderless version. The larger point though is just that there needs to be some consistent way of distinguishing between the two controls so that the user knows when to Tab between buttons vs when to use →. (I know you already know this, just saying it out loud for any new readers 😀) |
|
Thanks for the summary. This is looking good, and the mutually exclusive but not deselectable version looks excellent. What are cases where we have to use the not-deselectable version? Setting defaults in an onboarding dialog where you have to make a choice come to mind, but not many more. Both with regards to the borderless version, and the conversation of vertical separators between the "always de-selectable" version, it's important we don't look at these components only in isolation, there will always be a context. In the case of the Inspector, there will be a label and a controls grid that sets implicit boundaries of whitespace. In the case of the block toolbar, the toolbar itself provides framing and grouped separation. So in that case, additional separators there would probably not add much value. Nice work! |
|
Thanks for the design check!
@jasmussen I was thinking of something like these controls in the Group block:
|
There was a problem hiding this comment.
Thank you for working on this, Lena!
I agree with the split that you made between ToggleGroupControl and ToggleMultipleGroupControl, as it avoids overcomplicating the APIs.
some recent information from two accessibility experts regarding almost our exact problem. According to the article, it's not only ok but preferable that segmented controls are implemented with button and aria-pressed.
I went through the original article and the follow-up article from the a11y expert, here's a few notes:
- The articles seem to be aimed at marking up exclusive button group. We'd be taking that advice and applying it also to non-exclusive button groups (although it's probably ok?)
- My main worry is about the fact that, for exclusive toggle groups, a non-sighted user may not easily understand that pressing one button in the group would un-press the previously pressed button (using radio group would solve this problem at the code, since it's the way the component works). This doubt has been mostly answered in a series of tweets, although I don't think the answer is comprehensive — it still looks like a screen reader user won't get an announcement stating that the previously selected button is not selected anymore.
- If we agree on using pressable buttons so that the visual presentation matches the actual functionality, should we update all versions of
ToggleGroupControlandToggleMultipleGroupControlto always use buttons, and stop using radios altogether?
In short, I'm not strongly opposed to using buttons — I just think that by switching from radios to buttons we may improve the situation for certain users, while making it slightly worse for others.
However, some initial feedback from the Components Squad noted that it was visually unclear that this was a group of buttons, and was suggested that maybe we should keep the border
Lena summarized this quite well — without a border, I'm not sure that users would be able to easily understand that those icons are all grouped together. IMO, clear grouping becomes particularly important when there's no "selected" state.
it's important we don't look at these components only in isolation, there will always be a context.
While Gutenberg is the primary consumer of the package, these components should be thought and designed in such a way that makes them self-sufficient and independent. Assuming the context of the block editor when designing them, will make them tightly coupled to it, and it will make harder to use outside of the Gutenberg app / project.
| <View | ||
| aria-label={ label } | ||
| { ...otherProps } | ||
| ref={ useMergeRefs( [ containerRef, forwardedRef ] ) } | ||
| role="group" | ||
| > | ||
| { resizeListener } | ||
| <ToggleGroupControlBackdrop | ||
| state={ groupContext.state } | ||
| containerRef={ containerRef } | ||
| containerWidth={ sizes.width } | ||
| isAdaptiveWidth={ isAdaptiveWidth } | ||
| /> | ||
| <HStack spacing={ 1 }>{ children }</HStack> | ||
| </View> |
There was a problem hiding this comment.
The recommended way to group buttons seems to be to use section + aria-label or fieldset + legend
There was a problem hiding this comment.
In our case I think it would be better to stick to group because a lone ToggleGroupControl is likely never going to be worthy of an ARIA landmark in the Gutenberg app. And fieldset we need to avoid because of the nesting issue. Any consumer who wants a landmark or a fieldset could do this outside of the component.
|
In the case of these components, we cannot look at them in isolation and expect them to be intrinsically intuitive without any surrounding context. This is something we have to be mindful of in implementation, because no matter how accessible any one component is out of the box, it can be used in an inaccessible way. This is where documentation, best practices, and do/dont's come into play. Here's an example of menu items in TextEdit showing both exclusive, non-exclusive, and toggle buttons: Screen.Recording.2022-09-20.at.10.12.44.movIn this example, context and behavior becomes implicit by virtue of the text label, by the parent menu item, by separators, by grouping, and by curation. In other words, these menus feel intuitive and obvious in practical usage, because they were carefully crafted to be intutive. The block toolbar similarly contains both transforms (mutually exclusive, non-deselectable), alignments (mutually exclusive), inline formatting (multiple selections possible). The segmentation into toolbar groups is what helps imply each behavior:
In other words, as tools in our toolbox, separators and borders are already being used to help convey intrinsic behaviors of our components. But we have to zoom out and look at the broader context when choosing how to apply these. |
I tried this at first, but I realized that the current visual design of ToggleGroupControl (bordered with "flat" buttons) does not really have the affordance of a button group to begin with. Just me? Especially with that single tab stop, I find myself naturally going to the arrow keys to navigate, similar to a tablist. It's only when you remove the border and make discrete tab stops that it starts to feel like a row of buttons. So I don't quite think we should replace the default ToggleGroupControl implementation with a button group, unless we're also going to rethink the visual design.
Yes, I think that's a reasonable assessment. The exclusivity is not communicated as succinctly as a radio, but a button group isn't inherently inaccessible, I guess is the point. And we can probably enhance the screen reader experience by adding more cues (e.g. visually hidden descriptions, live regions, etc.) if we find ourselves in a context where this creates substantial usability problems for SR users. |
- otherProps are forwarded to div, not `input` - ToggleGroupControl should not accept a `disabled` prop, but it was included via the FormElementProps type
- Removed unused `separatorActive` - Don't lighten button when deselectable
mirka
force-pushed
the
toggle-group-control-refactor
branch
from
ad06af4
to
201bb03
Compare
|
Split into the following PRs for easier code review: |
Follow-up to #44067
Fixes icon color issue noted in #44065 (comment)
I'll probably split this for code review.
🚩 In this PR, let's first discuss the visual/interaction design concerns.
What?
The goal of this refactor is to add de-selection and multi-selection support to ToggleGroupControl, in a way that is:
Why?
Discussions started in #36735 and many possible approaches were thought through. Eventually, we arrived at the point where product requirements pretty much necessitated a "de-selectable radio control", which initially seemed kind of impossible to implement accessibly. Our impasse was broken when @noisysocks found some recent information from two accessibility experts regarding almost our exact problem. According to the article, it's not only ok but preferable that segmented controls are implemented with
buttonandaria-pressed.So with the semantics issue unblocked, all we have to do is work out the visual design and API.
How?
To recap, our component needs to support three combinations of behaviors:
Mutually exclusive, not de-selectable
First, this is our basic "radio-like" behavior. This is how ToggleGroupControl works today. The entire control is one tab stop, and options are toggled immediately when navigating with the arrow keys.
CleanShot.2022-09-16.at.04.36.30.mp4
Mutually exclusive, de-selectable
Next, we have a mutually exclusive, but de-selectable behavior. The button options are now distinct, and separate tab stops.
CleanShot.2022-09-16.at.03.58.09.mp4
There are actually already a good number of ad hoc components in Gutenberg that pretty much look like ☝️, sans backdrop animation. However, some initial feedback from the Components Squad noted that it was visually unclear that this was a group of buttons, and was suggested that maybe we should keep the border. After trying it, I think I prefer the borderless version, but here's a bordered one for reference:
CleanShot.2022-09-16.at.03.55.59.mp4
Not mutually exclusive (always de-selectable)
Finally, this is the multi-select version. As far as I know, the only similar UI that currently exists in Gutenberg is the formatting buttons in the Rich Text toolbar:
CleanShot.2022-09-16.at.03.59.15.mp4
(☝️ Sorry the icons are a smidge off-center. I'll fix that)
Here there was also some initial feedback from the Components Squad that the "group"-ness was visually unclear. A version with some kind of separator was suggested, so here's a rough prototype for reference:
CleanShot.2022-09-16.at.04.13.31.mp4
Testing Instructions
The docs are incomplete, but you can do
npm run storybook:devand see the stories for ToggleGroupControl and ToggleMultipleGroupControl if you want to try it out yourself.