From c01f777dea5260037f9751e591310edeca88049c Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 18:53:41 +0200 Subject: [PATCH 01/11] Add disabled option tests --- .../src/toggle-group-control/test/index.tsx | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/packages/components/src/toggle-group-control/test/index.tsx b/packages/components/src/toggle-group-control/test/index.tsx index eadbb498ad92a5..098407feef1493 100644 --- a/packages/components/src/toggle-group-control/test/index.tsx +++ b/packages/components/src/toggle-group-control/test/index.tsx @@ -80,6 +80,13 @@ const optionsWithTooltip = ( /> ); +const optionsWithDisabledOption = ( + <> + + + + +); describe.each( [ [ 'uncontrolled', ToggleGroupControl ], @@ -351,6 +358,53 @@ describe.each( [ expect( expectedFocusTarget ).toHaveFocus(); } ); + + it( 'should ignore disabled radio options', async () => { + const mockOnChange = jest.fn(); + + render( + + { optionsWithDisabledOption } + + ); + + await sleep(); + await press.Tab(); + + expect( + screen.getByRole( 'radio', { name: 'Pizza' } ) + ).toBeChecked(); + expect( + screen.getByRole( 'radio', { name: 'Rice' } ) + ).toBeDisabled(); + + // Arrow navigation skips the disabled option + await press.ArrowRight(); + expect( + screen.getByRole( 'radio', { name: 'Pasta' } ) + ).toBeChecked(); + expect( mockOnChange ).toHaveBeenCalledTimes( 1 ); + expect( mockOnChange ).toHaveBeenLastCalledWith( 'pasta' ); + + // 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 ); + } ); } ); describe( 'isDeselectable = true', () => { @@ -421,6 +475,66 @@ describe.each( [ } ) ).toHaveFocus(); } ); + + it( 'should ignore disabled options', async () => { + const mockOnChange = jest.fn(); + + render( + + { optionsWithDisabledOption } + + ); + + await sleep(); + await press.Tab(); + + expect( + screen.getByRole( 'button', { + name: 'Pizza', + pressed: true, + } ) + ).toBeVisible(); + expect( + screen.getByRole( 'button', { + name: 'Rice', + pressed: false, + } ) + ).toBeDisabled(); + + // Tab key navigation skips the disabled option + await press.Tab(); + await press.Space(); + expect( + screen.getByRole( 'button', { + name: 'Pasta', + pressed: true, + } ) + ).toHaveFocus(); + expect( mockOnChange ).toHaveBeenCalledTimes( 1 ); + expect( mockOnChange ).toHaveBeenLastCalledWith( 'pasta' ); + + // 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( mockOnChange ).toHaveBeenCalledTimes( 1 ); + } ); } ); } ); } ); From 95462434b647d3a0c3ccd442749b1f5e8c07f774 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 11 Jul 2024 19:00:12 +0200 Subject: [PATCH 02/11] Fix handling of disabled prop on ToggleGroupControl options --- .../toggle-group-control-option-base/component.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx b/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx index b2b94bd6df233a..30d74e68913f22 100644 --- a/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx +++ b/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx @@ -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' >, forwardedRef: ForwardedRef< any > ) { @@ -82,6 +84,7 @@ function ToggleGroupControlOptionBase( children, showTooltip = false, onFocus: onFocusProp, + disabled, ...otherButtonProps } = buttonProps; @@ -130,6 +133,7 @@ function ToggleGroupControlOptionBase( { isDeselectable ? ( ) : ( } diff --git a/packages/components/src/toggle-group-control/toggle-group-control-option-base/styles.ts b/packages/components/src/toggle-group-control/toggle-group-control-option-base/styles.ts index 999a25df8bdd40..dde3b71f552308 100644 --- a/packages/components/src/toggle-group-control/toggle-group-control-option-base/styles.ts +++ b/packages/components/src/toggle-group-control/toggle-group-control-option-base/styles.ts @@ -64,9 +64,28 @@ export const buttonView = ( { border: 0; } - &[disabled] { - opacity: 0.4; + &[disabled], + &[aria-disabled='true'] { cursor: default; + ${ ButtonContentView } { + opacity: 0.4; + } + + /** + * Show an additional focus ring for disabled radio items, since the + * indicator won't follow keyboard focus. This is not an issue for non-radio + * items, since the backdrop is already decoupled from keyboard focus. + */ + &[role='radio']:focus-visible::after { + position: absolute; + content: ''; + + /* y-axis inset matches the border width for good sub-pixel alignment */ + inset: var( --wp-admin-border-width-focus ) 4px; + border-radius: 2px; + outline: var( --wp-admin-border-width-focus ) solid + ${ COLORS.theme.accent }; + } } &:active { From 1d710665376309cca14e2a1b656c95d2991d6060 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 15 Jul 2024 11:41:04 +0200 Subject: [PATCH 07/11] Support accessibleWhenDisabled for button option items --- .../component.tsx | 45 +++++++++++++++---- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx b/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx index c13a57f8736bc8..bdd134f5ede3b5 100644 --- a/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx +++ b/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx @@ -33,6 +33,16 @@ 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 ( @@ -109,14 +119,6 @@ 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, @@ -124,6 +126,30 @@ function ToggleGroupControlOptionBase( 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 ( { children } From 8515e58998c338f74ccae384ef912fecd70c72e5 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 15 Jul 2024 11:42:38 +0200 Subject: [PATCH 08/11] Update snapshots --- .../test/__snapshots__/index.tsx.snap | 108 ++++++++++++++++-- 1 file changed, 96 insertions(+), 12 deletions(-) diff --git a/packages/components/src/toggle-group-control/test/__snapshots__/index.tsx.snap b/packages/components/src/toggle-group-control/test/__snapshots__/index.tsx.snap index cdfa4b1883cc16..cefa4718c6907e 100644 --- a/packages/components/src/toggle-group-control/test/__snapshots__/index.tsx.snap +++ b/packages/components/src/toggle-group-control/test/__snapshots__/index.tsx.snap @@ -132,11 +132,25 @@ exports[`ToggleGroupControl controlled should render correctly with icons 1`] = border: 0; } -.emotion-12[disabled] { - opacity: 0.4; +.emotion-12[disabled], +.emotion-12[aria-disabled='true'] { cursor: default; } +.emotion-12[disabled] .emotion-14, +.emotion-12[aria-disabled='true'] .emotion-14 { + 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)); +} + .emotion-12:active { background: #fff; } @@ -218,11 +232,25 @@ exports[`ToggleGroupControl controlled should render correctly with icons 1`] = border: 0; } -.emotion-18[disabled] { - opacity: 0.4; +.emotion-18[disabled], +.emotion-18[aria-disabled='true'] { cursor: default; } +.emotion-18[disabled] .emotion-14, +.emotion-18[aria-disabled='true'] .emotion-14 { + 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)); +} + .emotion-18:active { background: #fff; } @@ -462,11 +490,25 @@ exports[`ToggleGroupControl controlled should render correctly with text options border: 0; } -.emotion-12[disabled] { - opacity: 0.4; +.emotion-12[disabled], +.emotion-12[aria-disabled='true'] { cursor: default; } +.emotion-12[disabled] .emotion-14, +.emotion-12[aria-disabled='true'] .emotion-14 { + 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)); +} + .emotion-12:active { background: #fff; } @@ -692,11 +734,25 @@ exports[`ToggleGroupControl uncontrolled should render correctly with icons 1`] border: 0; } -.emotion-12[disabled] { - opacity: 0.4; +.emotion-12[disabled], +.emotion-12[aria-disabled='true'] { cursor: default; } +.emotion-12[disabled] .emotion-14, +.emotion-12[aria-disabled='true'] .emotion-14 { + 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)); +} + .emotion-12:active { background: #fff; } @@ -778,11 +834,25 @@ exports[`ToggleGroupControl uncontrolled should render correctly with icons 1`] border: 0; } -.emotion-18[disabled] { - opacity: 0.4; +.emotion-18[disabled], +.emotion-18[aria-disabled='true'] { cursor: default; } +.emotion-18[disabled] .emotion-14, +.emotion-18[aria-disabled='true'] .emotion-14 { + 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)); +} + .emotion-18:active { background: #fff; } @@ -1016,11 +1086,25 @@ exports[`ToggleGroupControl uncontrolled should render correctly with text optio border: 0; } -.emotion-12[disabled] { - opacity: 0.4; +.emotion-12[disabled], +.emotion-12[aria-disabled='true'] { cursor: default; } +.emotion-12[disabled] .emotion-14, +.emotion-12[aria-disabled='true'] .emotion-14 { + 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)); +} + .emotion-12:active { background: #fff; } From 0ee013e027ec366215f1c1bfd9dcf307472daae3 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 15 Jul 2024 12:04:48 +0200 Subject: [PATCH 09/11] Update tests to reflect that options are keyboard focusable while disabled --- .../src/toggle-group-control/test/index.tsx | 82 ++++++++++--------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/packages/components/src/toggle-group-control/test/index.tsx b/packages/components/src/toggle-group-control/test/index.tsx index 098407feef1493..ccbd811d8c20cb 100644 --- a/packages/components/src/toggle-group-control/test/index.tsx +++ b/packages/components/src/toggle-group-control/test/index.tsx @@ -359,7 +359,7 @@ describe.each( [ expect( expectedFocusTarget ).toHaveFocus(); } ); - it( 'should ignore disabled radio options', async () => { + it( 'should keep disabled radio options focusable but not selectable', async () => { const mockOnChange = jest.fn(); render( @@ -375,35 +375,37 @@ describe.each( [ await sleep(); await press.Tab(); - expect( - screen.getByRole( 'radio', { name: 'Pizza' } ) - ).toBeChecked(); - expect( - screen.getByRole( 'radio', { name: 'Rice' } ) - ).toBeDisabled(); + const pizzaOption = screen.getByRole( 'radio', { + name: 'Pizza', + } ); + expect( pizzaOption ).toHaveFocus(); + expect( pizzaOption ).toBeChecked(); - // Arrow navigation skips the disabled option + // Arrow navigation focuses the disabled option await press.ArrowRight(); - expect( - screen.getByRole( 'radio', { name: 'Pasta' } ) - ).toBeChecked(); + const riceOption = screen.getByRole( 'radio', { + name: 'Rice', + } ); + expect( riceOption ).toHaveFocus(); + expect( riceOption ).toHaveAttribute( 'aria-disabled', 'true' ); + expect( riceOption ).not.toBeChecked(); + expect( mockOnChange ).not.toHaveBeenCalled(); + + // Arrow navigation focuses the next enabled option + await press.ArrowRight(); + const pastaOption = screen.getByRole( 'radio', { + name: 'Pasta', + } ); + expect( pastaOption ).toHaveFocus(); + expect( pastaOption ).toBeChecked(); expect( mockOnChange ).toHaveBeenCalledTimes( 1 ); expect( mockOnChange ).toHaveBeenLastCalledWith( 'pasta' ); - // 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 ); + // 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 ); } ); } ); @@ -476,7 +478,7 @@ describe.each( [ ).toHaveFocus(); } ); - it( 'should ignore disabled options', async () => { + it( 'should keep disabled button options focusable but not selectable', async () => { const mockOnChange = jest.fn(); render( @@ -498,15 +500,20 @@ describe.each( [ name: 'Pizza', pressed: true, } ) - ).toBeVisible(); + ).toHaveFocus(); + + // Tab key navigation focuses the disabled option + await press.Tab(); + await press.Space(); expect( screen.getByRole( 'button', { name: 'Rice', pressed: false, } ) - ).toBeDisabled(); + ).toHaveFocus(); + expect( mockOnChange ).not.toHaveBeenCalled(); - // Tab key navigation skips the disabled option + // Tab key navigation focuses the next enabled option await press.Tab(); await press.Space(); expect( @@ -518,21 +525,18 @@ describe.each( [ expect( mockOnChange ).toHaveBeenCalledTimes( 1 ); expect( mockOnChange ).toHaveBeenLastCalledWith( 'pasta' ); - // 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. + // Clicks don't cause the option to be selected nor to be focused. await click( screen.getByRole( 'button', { name: 'Rice', } ) ); + expect( + screen.getByRole( 'button', { + name: 'Pasta', + pressed: true, + } ) + ).toHaveFocus(); expect( mockOnChange ).toHaveBeenCalledTimes( 1 ); } ); } ); From 8172ca656b95a60e88e3a1dd58d65ab8bedb0abd Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 15 Jul 2024 12:05:48 +0200 Subject: [PATCH 10/11] Move CHANGELOG entry to enhancements section --- packages/components/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 01e94dd0ad92fd..3a6ce15c62a7a9 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -10,7 +10,6 @@ - `ComboboxControl`: Fix ComboboxControl reset button when using the keyboard. ([#63410](https://github.com/WordPress/gutenberg/pull/63410)) - `Button`: Never apply `aria-disabled` to anchor ([#63376](https://github.com/WordPress/gutenberg/pull/63376)). -- `ToggleGroupControl`: support disabled options ([#63450](https://github.com/WordPress/gutenberg/pull/63450)). ### Internal @@ -22,6 +21,7 @@ ### Enhancements +- `ToggleGroupControl`: support disabled options ([#63450](https://github.com/WordPress/gutenberg/pull/63450)). - `CustomSelectControl`: Stabilize `__experimentalShowSelectedHint` and `options[]. __experimentalHint` props ([#63248](https://github.com/WordPress/gutenberg/pull/63248)). ## 28.3.0 (2024-07-10) From fe854cf12470c3a795953f197b129d7ff5ee5131 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 16 Jul 2024 13:42:40 +0200 Subject: [PATCH 11/11] Revert "Support accessibleWhenDisabled for ratio option items" This reverts commit 62959f041a499b9c63291b0c790474da0b494a5b. --- .../test/__snapshots__/index.tsx.snap | 108 ++---------------- .../src/toggle-group-control/test/index.tsx | 82 +++++++------ .../component.tsx | 52 ++------- .../styles.ts | 23 +--- 4 files changed, 63 insertions(+), 202 deletions(-) diff --git a/packages/components/src/toggle-group-control/test/__snapshots__/index.tsx.snap b/packages/components/src/toggle-group-control/test/__snapshots__/index.tsx.snap index cefa4718c6907e..cdfa4b1883cc16 100644 --- a/packages/components/src/toggle-group-control/test/__snapshots__/index.tsx.snap +++ b/packages/components/src/toggle-group-control/test/__snapshots__/index.tsx.snap @@ -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 { @@ -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 { @@ -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 { @@ -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 { @@ -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 { @@ -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 { diff --git a/packages/components/src/toggle-group-control/test/index.tsx b/packages/components/src/toggle-group-control/test/index.tsx index ccbd811d8c20cb..098407feef1493 100644 --- a/packages/components/src/toggle-group-control/test/index.tsx +++ b/packages/components/src/toggle-group-control/test/index.tsx @@ -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( @@ -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 ); } ); } ); @@ -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( @@ -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( @@ -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 ); } ); } ); diff --git a/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx b/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx index bdd134f5ede3b5..30d74e68913f22 100644 --- a/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx +++ b/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx @@ -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 ( @@ -119,6 +109,14 @@ 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, @@ -126,30 +124,6 @@ function ToggleGroupControlOptionBase( 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 ( { children } ) : ( } diff --git a/packages/components/src/toggle-group-control/toggle-group-control-option-base/styles.ts b/packages/components/src/toggle-group-control/toggle-group-control-option-base/styles.ts index dde3b71f552308..999a25df8bdd40 100644 --- a/packages/components/src/toggle-group-control/toggle-group-control-option-base/styles.ts +++ b/packages/components/src/toggle-group-control/toggle-group-control-option-base/styles.ts @@ -64,28 +64,9 @@ export const buttonView = ( { border: 0; } - &[disabled], - &[aria-disabled='true'] { + &[disabled] { + opacity: 0.4; cursor: default; - ${ ButtonContentView } { - opacity: 0.4; - } - - /** - * Show an additional focus ring for disabled radio items, since the - * indicator won't follow keyboard focus. This is not an issue for non-radio - * items, since the backdrop is already decoupled from keyboard focus. - */ - &[role='radio']:focus-visible::after { - position: absolute; - content: ''; - - /* y-axis inset matches the border width for good sub-pixel alignment */ - inset: var( --wp-admin-border-width-focus ) 4px; - border-radius: 2px; - outline: var( --wp-admin-border-width-focus ) solid - ${ COLORS.theme.accent }; - } } &:active {