diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 9dfc79d11f757..c39a492f840a2 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -10,6 +10,7 @@ - `ColorPalette`, `BorderControl`: Don't hyphenate hex value in `aria-label` ([#52932](https://github.com/WordPress/gutenberg/pull/52932)). - `MenuItemsChoice`, `MenuItem`: Support a `disabled` prop on a menu item ([#52737](https://github.com/WordPress/gutenberg/pull/52737)). +- `TabPanel`: Introduce a new version of `TabPanel` with updated internals and improved adherence to ARIA guidance on `tabpanel` focus behavior while maintaining the same functionality and API surface.([#52133](https://github.com/WordPress/gutenberg/pull/52133)). ### Bug Fix diff --git a/packages/components/src/tab-panel/index.tsx b/packages/components/src/tab-panel/index.tsx index 20063b2315dd3..4fff7dc306b02 100644 --- a/packages/components/src/tab-panel/index.tsx +++ b/packages/components/src/tab-panel/index.tsx @@ -1,6 +1,7 @@ /** * External dependencies */ +import * as Ariakit from '@ariakit/react'; import classnames from 'classnames'; import type { ForwardedRef } from 'react'; @@ -9,38 +10,30 @@ import type { ForwardedRef } from 'react'; */ import { forwardRef, - useState, useEffect, useLayoutEffect, useCallback, } from '@wordpress/element'; -import { useInstanceId } from '@wordpress/compose'; +import { useInstanceId, usePrevious } from '@wordpress/compose'; /** * Internal dependencies */ -import { NavigableMenu } from '../navigable-container'; + import Button from '../button'; -import type { TabButtonProps, TabPanelProps } from './types'; +import type { TabPanelProps } from './types'; import type { WordPressComponentProps } from '../ui/context'; -const TabButton = ( { - tabId, - children, - selected, - ...rest -}: TabButtonProps ) => ( - -); +// Separate the actual tab name from the instance ID. This is +// necessary because Ariakit internally uses the element ID when +// a new tab is selected, but our implementation looks specifically +// for the tab name to be passed to the `onSelect` callback. +const extractTabName = ( id: string | undefined | null ) => { + if ( typeof id === 'undefined' || id === null ) { + return; + } + return id.match( /^tab-panel-[0-9]*-(.*)/ )?.[ 1 ]; +}; /** * TabPanel is an ARIA-compliant tabpanel. @@ -92,26 +85,65 @@ const UnforwardedTabPanel = ( ref: ForwardedRef< any > ) => { const instanceId = useInstanceId( TabPanel, 'tab-panel' ); - const [ selected, setSelected ] = useState< string >(); - const handleTabSelection = useCallback( - ( tabKey: string ) => { - setSelected( tabKey ); - onSelect?.( tabKey ); + const prependInstanceId = useCallback( + ( tabName: string | undefined ) => { + if ( typeof tabName === 'undefined' ) { + return; + } + return `${ instanceId }-${ tabName }`; + }, + [ instanceId ] + ); + + const tabStore = Ariakit.useTabStore( { + setSelectedId: ( newTabValue ) => { + if ( typeof newTabValue === 'undefined' || newTabValue === null ) { + return; + } + + const newTab = tabs.find( + ( t ) => prependInstanceId( t.name ) === newTabValue + ); + if ( newTab?.disabled || newTab === selectedTab ) { + return; + } + + const simplifiedTabName = extractTabName( newTabValue ); + if ( typeof simplifiedTabName === 'undefined' ) { + return; + } + + onSelect?.( simplifiedTabName ); + }, + orientation, + selectOnMove, + defaultSelectedId: prependInstanceId( initialTabName ), + } ); + + const selectedTabName = extractTabName( tabStore.useState( 'selectedId' ) ); + + const setTabStoreSelectedId = useCallback( + ( tabName: string ) => { + tabStore.setState( 'selectedId', prependInstanceId( tabName ) ); }, - [ onSelect ] + [ prependInstanceId, tabStore ] ); - // Simulate a click on the newly focused tab, which causes the component - // to show the `tab-panel` associated with the clicked tab. - const activateTabAutomatically = ( - _childIndex: number, - child: HTMLElement - ) => { - child.click(); - }; - const selectedTab = tabs.find( ( { name } ) => name === selected ); - const selectedId = `${ instanceId }-${ selectedTab?.name ?? 'none' }`; + const selectedTab = tabs.find( ( { name } ) => name === selectedTabName ); + + const previousSelectedTabName = usePrevious( selectedTabName ); + + // Ensure `onSelect` is called when the initial tab is selected. + useEffect( () => { + if ( + previousSelectedTabName !== selectedTabName && + selectedTabName === initialTabName && + !! selectedTabName + ) { + onSelect?.( selectedTabName ); + } + }, [ selectedTabName, initialTabName, onSelect, previousSelectedTabName ] ); // Handle selecting the initial tab. useLayoutEffect( () => { @@ -119,25 +151,31 @@ const UnforwardedTabPanel = ( if ( selectedTab ) { return; } - const initialTab = tabs.find( ( tab ) => tab.name === initialTabName ); - // Wait for the denoted initial tab to be declared before making a // selection. This ensures that if a tab is declared lazily it can // still receive initial selection. if ( initialTabName && ! initialTab ) { return; } - if ( initialTab && ! initialTab.disabled ) { // Select the initial tab if it's not disabled. - handleTabSelection( initialTab.name ); + setTabStoreSelectedId( initialTab.name ); } else { - // Fallback to the first enabled tab when the initial is disabled. + // Fallback to the first enabled tab when the initial tab is + // disabled or it can't be found. const firstEnabledTab = tabs.find( ( tab ) => ! tab.disabled ); - if ( firstEnabledTab ) handleTabSelection( firstEnabledTab.name ); + if ( firstEnabledTab ) { + setTabStoreSelectedId( firstEnabledTab.name ); + } } - }, [ tabs, selectedTab, initialTabName, handleTabSelection ] ); + }, [ + tabs, + selectedTab, + initialTabName, + instanceId, + setTabStoreSelectedId, + ] ); // Handle the currently selected tab becoming disabled. useEffect( () => { @@ -145,59 +183,58 @@ const UnforwardedTabPanel = ( if ( ! selectedTab?.disabled ) { return; } - const firstEnabledTab = tabs.find( ( tab ) => ! tab.disabled ); - // If the currently selected tab becomes disabled, select the first enabled tab. // (if there is one). if ( firstEnabledTab ) { - handleTabSelection( firstEnabledTab.name ); + setTabStoreSelectedId( firstEnabledTab.name ); } - }, [ tabs, selectedTab?.disabled, handleTabSelection ] ); - + }, [ tabs, selectedTab?.disabled, setTabStoreSelectedId, instanceId ] ); return (
- - { tabs.map( ( tab ) => ( - { + return ( + } - ) } - tabId={ `${ instanceId }-${ tab.name }` } - aria-controls={ `${ instanceId }-${ tab.name }-view` } - selected={ tab.name === selected } - key={ tab.name } - onClick={ () => handleTabSelection( tab.name ) } - disabled={ tab.disabled } - label={ tab.icon && tab.title } - icon={ tab.icon } - showTooltip={ !! tab.icon } - > - { ! tab.icon && tab.title } - - ) ) } - + > + { ! tab.icon && tab.title } + + ); + } ) } + { selectedTab && ( -
{ children( selectedTab ) } -
+ ) }
); diff --git a/packages/components/src/tab-panel/stories/index.tsx b/packages/components/src/tab-panel/stories/index.tsx index 07c076e1ce621..b6a247d99ebdf 100644 --- a/packages/components/src/tab-panel/stories/index.tsx +++ b/packages/components/src/tab-panel/stories/index.tsx @@ -96,3 +96,9 @@ WithTabIconsAndTooltips.args = { }, ], }; + +export const ManualActivation = Template.bind( {} ); +ManualActivation.args = { + ...Default.args, + selectOnMove: false, +}; diff --git a/packages/components/src/tab-panel/test/index.tsx b/packages/components/src/tab-panel/test/index.tsx index 16f88ee8a41e9..c3102fe26833b 100644 --- a/packages/components/src/tab-panel/test/index.tsx +++ b/packages/components/src/tab-panel/test/index.tsx @@ -34,7 +34,8 @@ const TABS = [ }, ]; -const getSelectedTab = () => screen.getByRole( 'tab', { selected: true } ); +const getSelectedTab = async () => + await screen.findByRole( 'tab', { selected: true } ); let originalGetClientRects: () => DOMRectList; @@ -62,7 +63,7 @@ describe.each( [ } ); describe( 'Accessibility and semantics', () => { - test( 'should use the correct aria attributes', () => { + it( 'should use the correct aria attributes', async () => { const panelRenderFunction = jest.fn(); render( @@ -71,7 +72,7 @@ describe.each( [ const tabList = screen.getByRole( 'tablist' ); const allTabs = screen.getAllByRole( 'tab' ); - const selectedTabPanel = screen.getByRole( 'tabpanel' ); + const selectedTabPanel = await screen.findByRole( 'tabpanel' ); expect( tabList ).toBeVisible(); expect( tabList ).toHaveAttribute( @@ -94,7 +95,7 @@ describe.each( [ ); } ); - test( 'should display a tooltip when hovering tabs provided with an icon', async () => { + it( 'should display a tooltip when hovering tabs provided with an icon', async () => { const user = userEvent.setup(); const panelRenderFunction = jest.fn(); @@ -138,7 +139,7 @@ describe.each( [ } } ); - test( 'should display a tooltip when moving the selection via the keyboard on tabs provided with an icon', async () => { + it( 'should display a tooltip when moving the selection via the keyboard on tabs provided with an icon', async () => { const user = userEvent.setup(); const mockOnSelect = jest.fn(); @@ -165,10 +166,10 @@ describe.each( [ ); - expect( getSelectedTab() ).not.toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).not.toHaveTextContent( 'Alpha' ); expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); - await expect( getSelectedTab() ).not.toHaveFocus(); + expect( await getSelectedTab() ).not.toHaveFocus(); // Tab to focus the tablist. Make sure alpha is focused, and that the // corresponding tooltip is shown. @@ -176,7 +177,7 @@ describe.each( [ await user.keyboard( '[Tab]' ); expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); expect( screen.getByText( 'Alpha' ) ).toBeInTheDocument(); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveFocus(); // Move selection with arrow keys. Make sure beta is focused, and that // the corresponding tooltip is shown. @@ -185,7 +186,7 @@ describe.each( [ expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'beta' ); expect( screen.getByText( 'Beta' ) ).toBeInTheDocument(); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveFocus(); // Move selection with arrow keys. Make sure gamma is focused, and that // the corresponding tooltip is shown. @@ -194,7 +195,7 @@ describe.each( [ expect( mockOnSelect ).toHaveBeenCalledTimes( 3 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'gamma' ); expect( screen.getByText( 'Gamma' ) ).toBeInTheDocument(); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveFocus(); // Move selection with arrow keys. Make sure beta is focused, and that // the corresponding tooltip is shown. @@ -203,7 +204,7 @@ describe.each( [ expect( mockOnSelect ).toHaveBeenCalledTimes( 4 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'beta' ); expect( screen.getByText( 'Beta' ) ).toBeInTheDocument(); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveFocus(); } ); } ); @@ -215,11 +216,10 @@ describe.each( [ ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); expect( - screen.getByRole( 'tabpanel', { name: 'Alpha' } ) + await screen.findByRole( 'tabpanel', { name: 'Alpha' } ) ).toBeInTheDocument(); - expect( panelRenderFunction ).toHaveBeenLastCalledWith( TABS[ 0 ] ); } ); it( 'should fall back to first enabled tab if the active tab is removed', async () => { @@ -239,12 +239,12 @@ describe.each( [ onSelect={ mockOnSelect } /> ); - expect( getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); } ); } ); describe( 'With `initialTabName`', () => { - it( 'should render the tab set by initialTabName prop', () => { + it( 'should render the tab set by initialTabName prop', async () => { render( ); - expect( getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); } ); it( 'should not select a tab when `initialTabName` does not match any known tab', () => { @@ -273,8 +273,7 @@ describe.each( [ // No tabpanel should be rendered either expect( screen.queryByRole( 'tabpanel' ) ).not.toBeInTheDocument(); } ); - - it( 'should not change tabs when initialTabName is changed', () => { + it( 'should not change tabs when initialTabName is changed', async () => { const { rerender } = render( ); - expect( getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); } ); it( 'should fall back to the tab associated to `initialTabName` if the currently active tab is removed', async () => { @@ -307,13 +306,11 @@ describe.each( [ /> ); - expect( getSelectedTab() ).toHaveTextContent( 'Gamma' ); - expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'gamma' ); await user.click( screen.getByRole( 'tab', { name: 'Alpha' } ) ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); rerender( @@ -325,12 +322,11 @@ describe.each( [ /> ); - expect( getSelectedTab() ).toHaveTextContent( 'Gamma' ); - expect( mockOnSelect ).toHaveBeenCalledTimes( 3 ); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'gamma' ); } ); - it( 'should have no active tabs when the tab associated to `initialTabName` is removed while being the active tab', () => { + it( 'should have no active tabs when the tab associated to `initialTabName` is removed while being the active tab', async () => { const mockOnSelect = jest.fn(); const { rerender } = render( @@ -342,7 +338,7 @@ describe.each( [ /> ); - expect( getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'gamma' ); @@ -362,7 +358,7 @@ describe.each( [ expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); } ); - it( 'waits for the tab with the `initialTabName` to be present in the `tabs` array before selecting it', () => { + it( 'waits for the tab with the `initialTabName` to be present in the `tabs` array before selecting it', async () => { const mockOnSelect = jest.fn(); const { rerender } = render( ); - expect( getSelectedTab() ).toHaveTextContent( 'Delta' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Delta' ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'delta' ); } ); } ); @@ -433,7 +429,7 @@ describe.each( [ expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); } ); - it( 'should select first enabled tab when the initial tab is disabled', () => { + it( 'should select first enabled tab when the initial tab is disabled', async () => { const mockOnSelect = jest.fn(); const { rerender } = render( @@ -452,7 +448,7 @@ describe.each( [ // As alpha (first tab) is disabled, // the first enabled tab should be gamma. - expect( getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); // Re-enable all tabs rerender( @@ -465,10 +461,10 @@ describe.each( [ // Even if the initial tab becomes enabled again, the selected tab doesn't // change. - expect( getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); } ); - it( 'should select first enabled tab when the tab associated to `initialTabName` is disabled', () => { + it( 'should select first enabled tab when the tab associated to `initialTabName` is disabled', async () => { const mockOnSelect = jest.fn(); const { rerender } = render( @@ -487,7 +483,7 @@ describe.each( [ // As alpha (first tab), and beta (the initial tab), are both // disabled the first enabled tab should be gamma. - expect( getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); // Re-enable all tabs rerender( @@ -501,10 +497,10 @@ describe.each( [ // Even if the initial tab becomes enabled again, the selected tab doesn't // change. - expect( getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); } ); - it( 'should select the first enabled tab when the selected tab becomes disabled', () => { + it( 'should select the first enabled tab when the selected tab becomes disabled', async () => { const mockOnSelect = jest.fn(); const { rerender } = render( ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); @@ -531,7 +527,7 @@ describe.each( [ /> ); - expect( getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'beta' ); @@ -543,12 +539,12 @@ describe.each( [ /> ); - expect( getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'beta' ); } ); - it( 'should select the first enabled tab when the tab associated to `initialTabName` becomes disabled while being the active tab', () => { + it( 'should select the first enabled tab when the tab associated to `initialTabName` becomes disabled while being the active tab', async () => { const mockOnSelect = jest.fn(); const { rerender } = render( @@ -560,7 +556,7 @@ describe.each( [ /> ); - expect( getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'gamma' ); @@ -577,7 +573,7 @@ describe.each( [ /> ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); @@ -590,7 +586,7 @@ describe.each( [ /> ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); } ); } ); @@ -610,31 +606,28 @@ describe.each( [ ); // Alpha is the initially selected tab - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); expect( - screen.getByRole( 'tabpanel', { name: 'Alpha' } ) + await screen.findByRole( 'tabpanel', { name: 'Alpha' } ) ).toBeInTheDocument(); - expect( panelRenderFunction ).toHaveBeenLastCalledWith( TABS[ 0 ] ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); // Click on Beta, make sure beta is the selected tab await user.click( screen.getByRole( 'tab', { name: 'Beta' } ) ); - expect( getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); expect( screen.getByRole( 'tabpanel', { name: 'Beta' } ) ).toBeInTheDocument(); - expect( panelRenderFunction ).toHaveBeenLastCalledWith( TABS[ 1 ] ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'beta' ); // Click on Alpha, make sure beta is the selected tab await user.click( screen.getByRole( 'tab', { name: 'Alpha' } ) ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); expect( screen.getByRole( 'tabpanel', { name: 'Alpha' } ) ).toBeInTheDocument(); - expect( panelRenderFunction ).toHaveBeenLastCalledWith( TABS[ 0 ] ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); } ); @@ -654,24 +647,24 @@ describe.each( [ expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); // Tab to focus the tablist. Make sure alpha is focused. - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - await expect( getSelectedTab() ).not.toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).not.toHaveFocus(); await user.keyboard( '[Tab]' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveFocus(); // Navigate forward with arrow keys and make sure the Beta tab is // selected automatically. await user.keyboard( '[ArrowRight]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Beta' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'beta' ); // Navigate backwards with arrow keys. Make sure alpha is // selected automatically. await user.keyboard( '[ArrowLeft]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 3 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); } ); @@ -692,24 +685,24 @@ describe.each( [ expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); // Tab to focus the tablist. Make sure Alpha is focused. - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - await expect( getSelectedTab() ).not.toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).not.toHaveFocus(); await user.keyboard( '[Tab]' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveFocus(); // Navigate backwards with arrow keys and make sure that the Gamma tab // (the last tab) is selected automatically. await user.keyboard( '[ArrowLeft]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Gamma' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( await getSelectedTab() ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'gamma' ); // Navigate forward with arrow keys. Make sure alpha (the first tab) is // selected automatically. await user.keyboard( '[ArrowRight]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 3 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); } ); @@ -730,22 +723,22 @@ describe.each( [ expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); // Tab to focus the tablist. Make sure alpha is focused. - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - await expect( getSelectedTab() ).not.toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).not.toHaveFocus(); await user.keyboard( '[Tab]' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveFocus(); // Press the arrow up key, nothing happens. await user.keyboard( '[ArrowUp]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); // Press the arrow down key, nothing happens await user.keyboard( '[ArrowDown]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); @@ -766,38 +759,38 @@ describe.each( [ ); // Make sure alpha is still focused. - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveFocus(); // Navigate forward with arrow keys and make sure the Beta tab is // selected automatically. await user.keyboard( '[ArrowDown]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Beta' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'beta' ); // Navigate backwards with arrow keys. Make sure alpha is // selected automatically. await user.keyboard( '[ArrowUp]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 3 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); // Navigate backwards with arrow keys. Make sure alpha is // selected automatically. await user.keyboard( '[ArrowUp]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Gamma' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( await getSelectedTab() ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 4 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'gamma' ); // Navigate backwards with arrow keys. Make sure alpha is // selected automatically. await user.keyboard( '[ArrowDown]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 5 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); } ); @@ -826,10 +819,10 @@ describe.each( [ expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); // Tab to focus the tablist. Make sure Alpha is focused. - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - await expect( getSelectedTab() ).not.toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).not.toHaveFocus(); await user.keyboard( '[Tab]' ); - await expect( getSelectedTab() ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); // Press the right arrow key three times. Since the delta tab is disabled: @@ -838,26 +831,48 @@ describe.each( [ // `mockOnSelect` function gets called only twice (and not three times) // - it will receive focus, when using arrow keys await user.keyboard( '[ArrowRight][ArrowRight][ArrowRight]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Gamma' ); - await expect( + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( screen.getByRole( 'tab', { name: 'Delta' } ) ).toHaveFocus(); expect( mockOnSelect ).toHaveBeenCalledTimes( 3 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'gamma' ); // Navigate backwards with arrow keys. The gamma tab receives focus. + // The `mockOnSelect` callback doesn't fire, since the gamma tab was + // already selected. await user.keyboard( '[ArrowLeft]' ); - expect( getSelectedTab() ).toHaveTextContent( 'Gamma' ); - await expect( getSelectedTab() ).toHaveFocus(); - expect( mockOnSelect ).toHaveBeenCalledTimes( 4 ); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( await getSelectedTab() ).toHaveFocus(); + expect( mockOnSelect ).toHaveBeenCalledTimes( 3 ); - // Click on on the disabled tab. Compared to using arrow keys to move the + // Click on the disabled tab. Compared to using arrow keys to move the // focus, disabled tabs ignore pointer clicks — and therefore, they don't // receive focus, nor they cause the `mockOnSelect` function to fire. await user.click( screen.getByRole( 'tab', { name: 'Delta' } ) ); - expect( getSelectedTab() ).toHaveTextContent( 'Gamma' ); - await expect( getSelectedTab() ).toHaveFocus(); - expect( mockOnSelect ).toHaveBeenCalledTimes( 4 ); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( await getSelectedTab() ).toHaveFocus(); + expect( mockOnSelect ).toHaveBeenCalledTimes( 3 ); + } ); + + it( 'should not focus the next tab when the Tab key is pressed', async () => { + const user = userEvent.setup(); + + render( undefined } /> ); + + // Tab should initially focus the first tab in the tablist, which + // is Alpha. + await user.keyboard( '[Tab]' ); + expect( + await screen.findByRole( 'tab', { name: 'Alpha' } ) + ).toHaveFocus(); + + // Because all other tabs should have `tabindex=-1`, pressing Tab + // should NOT move the focus to the next tab, which is Beta. + await user.keyboard( '[Tab]' ); + expect( + await screen.findByRole( 'tab', { name: 'Beta' } ) + ).not.toHaveFocus(); } ); it( 'switches to manual tab activation when the `selectOnMove` prop is set to `false`', async () => { @@ -873,47 +888,51 @@ describe.each( [ /> ); - // onSelect gets called on the initial render. + // onSelect gets called on the initial render with the default + // selected tab. expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); // Click on Alpha and make sure it is selected. + // onSelect shouldn't fire since the selected tab didn't change. await user.click( screen.getByRole( 'tab', { name: 'Alpha' } ) ); - expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); + expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); // Navigate forward with arrow keys. Make sure Beta is focused, but // that the tab selection happens only when pressing the spacebar // or enter key. await user.keyboard( '[ArrowRight]' ); - expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); - expect( screen.getByRole( 'tab', { name: 'Beta' } ) ).toHaveFocus(); + expect( mockOnSelect ).toHaveBeenCalledTimes( 1 ); + expect( + await screen.findByRole( 'tab', { name: 'Beta' } ) + ).toHaveFocus(); await user.keyboard( '[Enter]' ); - expect( mockOnSelect ).toHaveBeenCalledTimes( 3 ); + expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'beta' ); // Navigate forward with arrow keys. Make sure Gamma (last tab) is // focused, but that tab selection happens only when pressing the // spacebar or enter key. await user.keyboard( '[ArrowRight]' ); - expect( mockOnSelect ).toHaveBeenCalledTimes( 3 ); + expect( mockOnSelect ).toHaveBeenCalledTimes( 2 ); expect( screen.getByRole( 'tab', { name: 'Gamma' } ) ).toHaveFocus(); await user.keyboard( '[Space]' ); - expect( mockOnSelect ).toHaveBeenCalledTimes( 4 ); + expect( mockOnSelect ).toHaveBeenCalledTimes( 3 ); expect( mockOnSelect ).toHaveBeenLastCalledWith( 'gamma' ); } ); } ); describe( 'Tab Attributes', () => { - it( "should apply the tab's `className` to the tab button", () => { + it( "should apply the tab's `className` to the tab button", async () => { render( undefined } /> ); - expect( screen.getByRole( 'tab', { name: 'Alpha' } ) ).toHaveClass( - 'alpha-class' - ); + expect( + await screen.findByRole( 'tab', { name: 'Alpha' } ) + ).toHaveClass( 'alpha-class' ); expect( screen.getByRole( 'tab', { name: 'Beta' } ) ).toHaveClass( 'beta-class' ); @@ -935,8 +954,8 @@ describe.each( [ ); // Make sure that only the selected tab has the active class - expect( getSelectedTab() ).toHaveTextContent( 'Alpha' ); - expect( getSelectedTab() ).toHaveClass( activeClass ); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + expect( await getSelectedTab() ).toHaveClass( activeClass ); screen .getAllByRole( 'tab', { selected: false } ) .forEach( ( unselectedTab ) => { @@ -947,8 +966,8 @@ describe.each( [ await user.click( screen.getByRole( 'tab', { name: 'Beta' } ) ); // Make sure that only the selected tab has the active class - expect( getSelectedTab() ).toHaveTextContent( 'Beta' ); - expect( getSelectedTab() ).toHaveClass( activeClass ); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveClass( activeClass ); screen .getAllByRole( 'tab', { selected: false } ) .forEach( ( unselectedTab ) => { diff --git a/packages/components/src/tab-panel/types.ts b/packages/components/src/tab-panel/types.ts index 4bef866923ebc..1f4dc7c677483 100644 --- a/packages/components/src/tab-panel/types.ts +++ b/packages/components/src/tab-panel/types.ts @@ -1,7 +1,7 @@ /** * External dependencies */ -import type { MouseEvent, ReactNode } from 'react'; +import type { ReactNode } from 'react'; /** * Internal dependencies @@ -31,15 +31,6 @@ type Tab = { disabled?: boolean; } & Record< any, any >; -export type TabButtonProps = { - children: ReactNode; - label?: string; - onClick: ( event: MouseEvent ) => void; - selected: boolean; - showTooltip?: boolean; - tabId: string; -} & Pick< Tab, 'className' | 'icon' | 'disabled' >; - export type TabPanelProps = { /** * The class name to add to the active tab. diff --git a/packages/edit-post/src/components/preferences-modal/test/__snapshots__/index.js.snap b/packages/edit-post/src/components/preferences-modal/test/__snapshots__/index.js.snap index 32254bb91c695..bcaed355d48ad 100644 --- a/packages/edit-post/src/components/preferences-modal/test/__snapshots__/index.js.snap +++ b/packages/edit-post/src/components/preferences-modal/test/__snapshots__/index.js.snap @@ -115,6 +115,8 @@ exports[`EditPostPreferencesModal should match snapshot when the modal is active aria-controls="tab-panel-0-general-view" aria-selected="true" class="components-button components-tab-panel__tabs-item is-active" + data-active-item="" + data-command="" id="tab-panel-0-general" role="tab" type="button" @@ -125,9 +127,9 @@ exports[`EditPostPreferencesModal should match snapshot when the modal is active aria-controls="tab-panel-0-blocks-view" aria-selected="false" class="components-button components-tab-panel__tabs-item" + data-command="" id="tab-panel-0-blocks" role="tab" - tabindex="-1" type="button" > Blocks @@ -136,9 +138,9 @@ exports[`EditPostPreferencesModal should match snapshot when the modal is active aria-controls="tab-panel-0-panels-view" aria-selected="false" class="components-button components-tab-panel__tabs-item" + data-command="" id="tab-panel-0-panels" role="tab" - tabindex="-1" type="button" > Panels @@ -149,6 +151,7 @@ exports[`EditPostPreferencesModal should match snapshot when the modal is active class="components-tab-panel__tab-content" id="tab-panel-0-general-view" role="tabpanel" + tabindex="0" >
jest.fn() ); describe( 'EditPostPreferencesModal', () => { describe( 'should match snapshot when the modal is active', () => { - it( 'large viewports', () => { + it( 'large viewports', async () => { useSelect.mockImplementation( () => [ true, true, false ] ); useViewportMatch.mockImplementation( () => true ); render( ); expect( - screen.getByRole( 'dialog', { name: 'Preferences' } ) + await screen.findByRole( 'dialog', { name: 'Preferences' } ) ).toMatchSnapshot(); } ); - it( 'small viewports', () => { + it( 'small viewports', async () => { useSelect.mockImplementation( () => [ true, true, false ] ); useViewportMatch.mockImplementation( () => false ); render( ); expect( - screen.getByRole( 'dialog', { name: 'Preferences' } ) + await screen.findByRole( 'dialog', { name: 'Preferences' } ) ).toMatchSnapshot(); } ); } );