Skip to content

Commit

Permalink
Tabs: sync browser focus to selected tab in controlled mode (#56658)
Browse files Browse the repository at this point in the history
* link browser focus to selected tab changes in controlled mode

* remove unnecessary memoization

* remove duplicate condition

* dont apply when selectOnMove is false

* use `items` to find active element, instead of a ref

* CHANGELOG

* add `selectOnMove` unit tests

* convert new tests to ariakit/test

* remove redundant tests
  • Loading branch information
chad1008 authored Dec 8, 2023
1 parent 91cb8fe commit 2e95c20
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 2 deletions.
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@

- `Tabs`: implement new `tabId` prop ([#56883](https://github.com/WordPress/gutenberg/pull/56883)).

### Experimental

- `Tabs`: improve focus handling in controlled mode ([#56658](https://github.com/WordPress/gutenberg/pull/56658)).

## 25.13.0 (2023-11-29)

### Enhancements
Expand Down
23 changes: 22 additions & 1 deletion packages/components/src/tabs/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function Tabs( {
const isControlled = selectedTabId !== undefined;

const { items, selectedId } = store.useState();
const { setSelectedId } = store;
const { setSelectedId, move } = store;

// Keep track of whether tabs have been populated. This is used to prevent
// certain effects from firing too early while tab data and relevant
Expand Down Expand Up @@ -154,6 +154,27 @@ function Tabs( {
setSelectedId,
] );

// In controlled mode, make sure browser focus follows the selected tab if
// the selection is changed while a tab is already being focused.
useLayoutEffect( () => {
if ( ! isControlled || ! selectOnMove ) {
return;
}
const currentItem = items.find( ( item ) => item.id === selectedId );
const activeElement = currentItem?.element?.ownerDocument.activeElement;
const tabsHasFocus = items.some( ( item ) => {
return activeElement && activeElement === item.element;
} );

if (
activeElement &&
tabsHasFocus &&
selectedId !== activeElement.id
) {
move( selectedId );
}
}, [ isControlled, items, move, selectOnMove, selectedId ] );

const contextValue = useMemo(
() => ( {
store,
Expand Down
111 changes: 110 additions & 1 deletion packages/components/src/tabs/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { press, click } from '@ariakit/test';
/**
* WordPress dependencies
*/
import { useState } from '@wordpress/element';
import { useEffect, useState } from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -102,6 +102,10 @@ const ControlledTabs = ( {
string | undefined | null
>( props.selectedTabId );

useEffect( () => {
setSelectedTabId( props.selectedTabId );
}, [ props.selectedTabId ] );

return (
<Tabs
{ ...props }
Expand Down Expand Up @@ -1168,5 +1172,110 @@ describe( 'Tabs', () => {
).not.toBeInTheDocument();
} );
} );

describe( 'When `selectOnMove` is `true`', () => {
it( 'should automatically select a newly focused tab', async () => {
render( <ControlledTabs tabs={ TABS } selectedTabId="beta" /> );

await press.Tab();

// Tab key should focus the currently selected tab, which is Beta.
expect( await getSelectedTab() ).toHaveTextContent( 'Beta' );
expect( await getSelectedTab() ).toHaveFocus();

// Arrow keys should select and move focus to the next tab.
await press.ArrowRight();
expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' );
expect( await getSelectedTab() ).toHaveFocus();
} );
it( 'should automatically update focus when the selected tab is changed by the controlling component', async () => {
const { rerender } = render(
<ControlledTabs tabs={ TABS } selectedTabId="beta" />
);

// Tab key should focus the currently selected tab, which is Beta.
await press.Tab();
expect( await getSelectedTab() ).toHaveTextContent( 'Beta' );
expect( await getSelectedTab() ).toHaveFocus();

rerender(
<ControlledTabs tabs={ TABS } selectedTabId="gamma" />
);

// When the selected tab is changed, it should automatically receive focus.
expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' );
expect( await getSelectedTab() ).toHaveFocus();
} );
} );
describe( 'When `selectOnMove` is `false`', () => {
it( 'should apply focus without automatically changing the selected tab', async () => {
render(
<ControlledTabs
tabs={ TABS }
selectedTabId="beta"
selectOnMove={ false }
/>
);

expect( await getSelectedTab() ).toHaveTextContent( 'Beta' );

// Tab key should focus the currently selected tab, which is Beta.
await press.Tab();
expect(
await screen.findByRole( 'tab', { name: 'Beta' } )
).toHaveFocus();

// Arrow key should move focus but not automatically change the selected tab.
await press.ArrowRight();
expect(
screen.getByRole( 'tab', { name: 'Gamma' } )
).toHaveFocus();
expect( await getSelectedTab() ).toHaveTextContent( 'Beta' );

// Pressing the spacebar should select the focused tab.
await press.Space();
expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' );

// Arrow key should move focus but not automatically change the selected tab.
await press.ArrowRight();
expect(
screen.getByRole( 'tab', { name: 'Alpha' } )
).toHaveFocus();
expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' );

// Pressing the enter/return should select the focused tab.
await press.Enter();
expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' );
} );
it( 'should not automatically update focus when the selected tab is changed by the controlling component', async () => {
const { rerender } = render(
<ControlledTabs
tabs={ TABS }
selectedTabId="beta"
selectOnMove={ false }
/>
);

expect( await getSelectedTab() ).toHaveTextContent( 'Beta' );

// Tab key should focus the currently selected tab, which is Beta.
await press.Tab();
expect( await getSelectedTab() ).toHaveFocus();

rerender(
<ControlledTabs
tabs={ TABS }
selectedTabId="gamma"
selectOnMove={ false }
/>
);

// When the selected tab is changed, it should not automatically receive focus.
expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' );
expect(
screen.getByRole( 'tab', { name: 'Beta' } )
).toHaveFocus();
} );
} );
} );
} );

1 comment on commit 2e95c20

@github-actions
Copy link

Choose a reason for hiding this comment

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

Flaky tests detected in 2e95c20.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7144846982
📝 Reported issues:

Please sign in to comment.