Skip to content

Commit

Permalink
Revert "Support accessibleWhenDisabled for ratio option items"
Browse files Browse the repository at this point in the history 
This reverts commit 62959f0.
  • Loading branch information
@ciampo
ciampo committed Jul 16, 2024
1 parent 8172ca6 commit fe854cf
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 202 deletions.
108 changes: 12 additions & 96 deletions packages/components/src/toggle-group-control/test/__snapshots__/index.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,23 +132,9 @@ exports[`ToggleGroupControl controlled should render correctly with icons 1`] =
border: 0;
}
.emotion-12[disabled],
.emotion-12[aria-disabled='true'] {
cursor: default;
}
.emotion-12[disabled] .emotion-14,
.emotion-12[aria-disabled='true'] .emotion-14 {
.emotion-12[disabled] {
opacity: 0.4;
}
.emotion-12[disabled][role='radio']:focus-visible::after,
.emotion-12[aria-disabled='true'][role='radio']:focus-visible::after {
position: absolute;
content: '';
inset: var( --wp-admin-border-width-focus ) 4px;
border-radius: 2px;
outline: var( --wp-admin-border-width-focus ) solid var(--wp-components-color-accent, var(--wp-admin-theme-color, #3858e9));
cursor: default;
}
.emotion-12:active {
Expand Down Expand Up @@ -232,23 +218,9 @@ exports[`ToggleGroupControl controlled should render correctly with icons 1`] =
border: 0;
}
.emotion-18[disabled],
.emotion-18[aria-disabled='true'] {
cursor: default;
}
.emotion-18[disabled] .emotion-14,
.emotion-18[aria-disabled='true'] .emotion-14 {
.emotion-18[disabled] {
opacity: 0.4;
}
.emotion-18[disabled][role='radio']:focus-visible::after,
.emotion-18[aria-disabled='true'][role='radio']:focus-visible::after {
position: absolute;
content: '';
inset: var( --wp-admin-border-width-focus ) 4px;
border-radius: 2px;
outline: var( --wp-admin-border-width-focus ) solid var(--wp-components-color-accent, var(--wp-admin-theme-color, #3858e9));
cursor: default;
}
.emotion-18:active {
Expand Down Expand Up @@ -490,23 +462,9 @@ exports[`ToggleGroupControl controlled should render correctly with text options
border: 0;
}
.emotion-12[disabled],
.emotion-12[aria-disabled='true'] {
cursor: default;
}
.emotion-12[disabled] .emotion-14,
.emotion-12[aria-disabled='true'] .emotion-14 {
.emotion-12[disabled] {
opacity: 0.4;
}
.emotion-12[disabled][role='radio']:focus-visible::after,
.emotion-12[aria-disabled='true'][role='radio']:focus-visible::after {
position: absolute;
content: '';
inset: var( --wp-admin-border-width-focus ) 4px;
border-radius: 2px;
outline: var( --wp-admin-border-width-focus ) solid var(--wp-components-color-accent, var(--wp-admin-theme-color, #3858e9));
cursor: default;
}
.emotion-12:active {
Expand Down Expand Up @@ -734,23 +692,9 @@ exports[`ToggleGroupControl uncontrolled should render correctly with icons 1`]
border: 0;
}
.emotion-12[disabled],
.emotion-12[aria-disabled='true'] {
cursor: default;
}
.emotion-12[disabled] .emotion-14,
.emotion-12[aria-disabled='true'] .emotion-14 {
.emotion-12[disabled] {
opacity: 0.4;
}
.emotion-12[disabled][role='radio']:focus-visible::after,
.emotion-12[aria-disabled='true'][role='radio']:focus-visible::after {
position: absolute;
content: '';
inset: var( --wp-admin-border-width-focus ) 4px;
border-radius: 2px;
outline: var( --wp-admin-border-width-focus ) solid var(--wp-components-color-accent, var(--wp-admin-theme-color, #3858e9));
cursor: default;
}
.emotion-12:active {
Expand Down Expand Up @@ -834,23 +778,9 @@ exports[`ToggleGroupControl uncontrolled should render correctly with icons 1`]
border: 0;
}
.emotion-18[disabled],
.emotion-18[aria-disabled='true'] {
cursor: default;
}
.emotion-18[disabled] .emotion-14,
.emotion-18[aria-disabled='true'] .emotion-14 {
.emotion-18[disabled] {
opacity: 0.4;
}
.emotion-18[disabled][role='radio']:focus-visible::after,
.emotion-18[aria-disabled='true'][role='radio']:focus-visible::after {
position: absolute;
content: '';
inset: var( --wp-admin-border-width-focus ) 4px;
border-radius: 2px;
outline: var( --wp-admin-border-width-focus ) solid var(--wp-components-color-accent, var(--wp-admin-theme-color, #3858e9));
cursor: default;
}
.emotion-18:active {
Expand Down Expand Up @@ -1086,23 +1016,9 @@ exports[`ToggleGroupControl uncontrolled should render correctly with text optio
border: 0;
}
.emotion-12[disabled],
.emotion-12[aria-disabled='true'] {
cursor: default;
}
.emotion-12[disabled] .emotion-14,
.emotion-12[aria-disabled='true'] .emotion-14 {
.emotion-12[disabled] {
opacity: 0.4;
}
.emotion-12[disabled][role='radio']:focus-visible::after,
.emotion-12[aria-disabled='true'][role='radio']:focus-visible::after {
position: absolute;
content: '';
inset: var( --wp-admin-border-width-focus ) 4px;
border-radius: 2px;
outline: var( --wp-admin-border-width-focus ) solid var(--wp-components-color-accent, var(--wp-admin-theme-color, #3858e9));
cursor: default;
}
.emotion-12:active {
Expand Down
82 changes: 39 additions & 43 deletions packages/components/src/toggle-group-control/test/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ describe.each( [
expect( expectedFocusTarget ).toHaveFocus();
} );

it( 'should keep disabled radio options focusable but not selectable', async () => {
it( 'should ignore disabled radio options', async () => {
const mockOnChange = jest.fn();

render(
Expand All @@ -375,37 +375,35 @@ describe.each( [
await sleep();
await press.Tab();

const pizzaOption = screen.getByRole( 'radio', {
name: 'Pizza',
} );
expect( pizzaOption ).toHaveFocus();
expect( pizzaOption ).toBeChecked();

// Arrow navigation focuses the disabled option
await press.ArrowRight();
const riceOption = screen.getByRole( 'radio', {
name: 'Rice',
} );
expect( riceOption ).toHaveFocus();
expect( riceOption ).toHaveAttribute( 'aria-disabled', 'true' );
expect( riceOption ).not.toBeChecked();
expect( mockOnChange ).not.toHaveBeenCalled();
expect(
screen.getByRole( 'radio', { name: 'Pizza' } )
).toBeChecked();
expect(
screen.getByRole( 'radio', { name: 'Rice' } )
).toBeDisabled();

// Arrow navigation focuses the next enabled option
// Arrow navigation skips the disabled option
await press.ArrowRight();
const pastaOption = screen.getByRole( 'radio', {
name: 'Pasta',
} );
expect( pastaOption ).toHaveFocus();
expect( pastaOption ).toBeChecked();
expect(
screen.getByRole( 'radio', { name: 'Pasta' } )
).toBeChecked();
expect( mockOnChange ).toHaveBeenCalledTimes( 1 );
expect( mockOnChange ).toHaveBeenLastCalledWith( 'pasta' );

// Clicks don't cause the option to be selected nor to be focused.
await click( riceOption );
expect( pastaOption ).toHaveFocus();
expect( pastaOption ).toBeChecked();
expect( mockOnChange ).toHaveBeenCalledTimes( 1 );
// Arrow navigation skips the disabled option
await press.ArrowLeft();
expect(
screen.getByRole( 'radio', { name: 'Pizza' } )
).toBeChecked();
expect( mockOnChange ).toHaveBeenCalledTimes( 2 );
expect( mockOnChange ).toHaveBeenLastCalledWith( 'pizza' );

// Clicks don't cause the option to be selected
await click( screen.getByRole( 'radio', { name: 'Rice' } ) );
expect(
screen.getByRole( 'radio', { name: 'Pizza' } )
).toBeChecked();
expect( mockOnChange ).toHaveBeenCalledTimes( 2 );
} );
} );

Expand Down Expand Up @@ -478,7 +476,7 @@ describe.each( [
).toHaveFocus();
} );

it( 'should keep disabled button options focusable but not selectable', async () => {
it( 'should ignore disabled options', async () => {
const mockOnChange = jest.fn();

render(
Expand All @@ -500,20 +498,15 @@ describe.each( [
name: 'Pizza',
pressed: true,
} )
).toHaveFocus();

// Tab key navigation focuses the disabled option
await press.Tab();
await press.Space();
).toBeVisible();
expect(
screen.getByRole( 'button', {
name: 'Rice',
pressed: false,
} )
).toHaveFocus();
expect( mockOnChange ).not.toHaveBeenCalled();
).toBeDisabled();

// Tab key navigation focuses the next enabled option
// Tab key navigation skips the disabled option
await press.Tab();
await press.Space();
expect(
Expand All @@ -525,18 +518,21 @@ describe.each( [
expect( mockOnChange ).toHaveBeenCalledTimes( 1 );
expect( mockOnChange ).toHaveBeenLastCalledWith( 'pasta' );

// Clicks don't cause the option to be selected nor to be focused.
// Tab key navigation skips the disabled option
await press.ShiftTab();
expect(
screen.getByRole( 'button', {
name: 'Pizza',
pressed: false,
} )
).toHaveFocus();

// Clicks don't cause the option to be selected.
await click(
screen.getByRole( 'button', {
name: 'Rice',
} )
);
expect(
screen.getByRole( 'button', {
name: 'Pasta',
pressed: true,
} )
).toHaveFocus();
expect( mockOnChange ).toHaveBeenCalledTimes( 1 );
} );
} );
Expand Down
52 changes: 10 additions & 42 deletions packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,6 @@ const REDUCED_MOTION_TRANSITION_CONFIG = {

const LAYOUT_ID = 'toggle-group-backdrop-shared-layout-id';

const handleDisabledMouseEvent = (
event: React.MouseEvent,
disabled?: boolean
) => {
if ( disabled ) {
event.stopPropagation();
event.preventDefault();
}
};

const WithToolTip = ( { showTooltip, text, children }: WithToolTipProps ) => {
if ( showTooltip && text ) {
return (
Expand Down Expand Up @@ -119,37 +109,21 @@ function ToggleGroupControlOptionBase(
);
const backdropClasses = useMemo( () => cx( styles.backdropView ), [ cx ] );

const buttonOnClick = () => {
if ( isDeselectable && isPressed ) {
toggleGroupControlContext.setValue( undefined );
} else {
toggleGroupControlContext.setValue( value );
}
};

const commonProps = {
...otherButtonProps,
className: itemClasses,
'data-value': value,
ref: forwardedRef,
};

const buttonOnMouseDown: React.MouseEventHandler< HTMLButtonElement > = (
event
) => {
handleDisabledMouseEvent( event, disabled );

commonProps.onMouseDown?.( event );
};

const buttonOnClick: React.MouseEventHandler< HTMLButtonElement > = (
event
) => {
handleDisabledMouseEvent( event, disabled );

if ( ! disabled ) {
if ( isDeselectable && isPressed ) {
toggleGroupControlContext.setValue( undefined );
} else {
toggleGroupControlContext.setValue( value );
}
}

commonProps.onClick?.( event );
};

return (
<LabelView className={ labelViewClasses }>
<WithToolTip
Expand All @@ -159,18 +133,16 @@ function ToggleGroupControlOptionBase(
{ isDeselectable ? (
<button
{ ...commonProps }
aria-disabled={ disabled }
disabled={ disabled }
onFocus={ onFocusProp }
aria-pressed={ isPressed }
type="button"
onClick={ buttonOnClick }
onMouseDown={ buttonOnMouseDown }
>
<ButtonContentView>{ children }</ButtonContentView>
</button>
) : (
<Ariakit.Radio
accessibleWhenDisabled
disabled={ disabled }
render={
<button
Expand All @@ -181,11 +153,7 @@ function ToggleGroupControlOptionBase(
if ( event.defaultPrevented ) {
return;
}
if ( ! disabled ) {
toggleGroupControlContext.setValue(
value
);
}
toggleGroupControlContext.setValue( value );
} }
/>
}
Expand Down
Loading

0 comments on commit fe854cf

Please sign in to comment.