Skip to content

ToggleGroupControl: support disabled options #63450

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.

Sign up for GitHub

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

Merged
ciampo merged 11 commits into from
Jul 16, 2024
Merged

ToggleGroupControl: support disabled options #63450

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c01f777
Add disabled option tests
ciampo Jul 11, 2024
9546243
Fix handling of disabled prop on ToggleGroupControl options
ciampo Jul 11, 2024
ceebf66
Add basic disabled styles
ciampo Jul 11, 2024
a1cf315
Update snapshots
ciampo Jul 11, 2024
ea67533
CHANGELOG
ciampo Jul 11, 2024
62959f0
Support accessibleWhenDisabled for ratio option items
ciampo Jul 15, 2024
1d71066
Support accessibleWhenDisabled for button option items
ciampo Jul 15, 2024
8515e58
Update snapshots
ciampo Jul 15, 2024
0ee013e
Update tests to reflect that options are keyboard focusable while dis…
ciampo Jul 15, 2024
8172ca6
Move CHANGELOG entry to enhancements section
ciampo Jul 15, 2024
fe854cf
Revert "Support accessibleWhenDisabled for ratio option items"
ciampo Jul 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix handling of disabled prop on ToggleGroupControl options
  • Loading branch information
@ciampo
ciampo committed Jul 16, 2024
commit 95462434b647d3a0c3ccd442749b1f5e8c07f774
7 changes: 6 additions & 1 deletion packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ function ToggleGroupControlOptionBase(
false
>,
// the element's id is generated internally
'id'
| 'id'
// due to how the component works, only the `disabled` prop should be used
| 'aria-disabled'
Comment on lines +56 to +57
Copy link
Contributor Author

@ciampo ciampo Jul 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think it makes sense to have the options accessibleWhenDisabled given how the component works cc @mirka

Copy link
Member

@mirka mirka Jul 11, 2024

Choose a reason for hiding this comment

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

I actually thought this was one of the cases where we always want the option to be focusable when disabled, similar to ToolbarButton (#63102) or tabs in a tablist. It's also a composite widget where there will be a limited number of options, and the number of tab stops it potentially adds to the tab sequence is either zero or negligible.

Implementing that in the radio case for ToggleGroupControl would require some additional design work though 🤔 But at least Ariakit supports the behavior nicely (StackBlitz demo).

Copy link
Contributor Author

@ciampo ciampo Jul 12, 2024

Choose a reason for hiding this comment

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

Implementing that in the radio case for ToggleGroupControl would require some additional design work though

Yep. Currently, since disabled options are not really supported, the backdrop indicator also works as a focus indicator.
If we make the options accessibleWhenDisabled, then we'll need to add separate focus rings.

But at least Ariakit supports the behavior nicely (StackBlitz demo).

Yup. We'll probably need to add the custom logic when rendering individual buttons

>,
forwardedRef: ForwardedRef< any >
) {
Expand Down Expand Up @@ -82,6 +84,7 @@ function ToggleGroupControlOptionBase(
children,
showTooltip = false,
onFocus: onFocusProp,
disabled,
...otherButtonProps
} = buttonProps;

Expand Down Expand Up @@ -130,6 +133,7 @@ function ToggleGroupControlOptionBase(
{ isDeselectable ? (
<button
{ ...commonProps }
disabled={ disabled }
onFocus={ onFocusProp }
aria-pressed={ isPressed }
type="button"
Expand All @@ -139,6 +143,7 @@ function ToggleGroupControlOptionBase(
</button>
) : (
<Ariakit.Radio
disabled={ disabled }
render={
<button
type="button"
Expand Down