Skip to content

Commit

Permalink
Set post editor sidebar tabs to manual activation (#58041)
Browse files Browse the repository at this point in the history
* selectOnMove = false

* address tab flow race condition

* Update packages/edit-post/src/components/sidebar/settings-sidebar/index.js

Co-authored-by: chad1008 <shireling@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
  • Loading branch information
3 people authored Feb 2, 2024
1 parent 22bbacb commit 486b2db
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 8 deletions.
21 changes: 16 additions & 5 deletions packages/edit-post/src/components/sidebar/settings-header/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { privateApis as componentsPrivateApis } from '@wordpress/components';
import { __, _x } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';
import { forwardRef } from '@wordpress/element';
import { store as editorStore } from '@wordpress/editor';

/**
Expand All @@ -14,7 +15,7 @@ import { sidebars } from '../settings-sidebar';

const { Tabs } = unlock( componentsPrivateApis );

const SettingsHeader = () => {
const SettingsHeader = ( _, ref ) => {
const { documentLabel } = useSelect( ( select ) => {
const { getPostTypeLabel } = select( editorStore );

Expand All @@ -25,14 +26,24 @@ const SettingsHeader = () => {
}, [] );

return (
<Tabs.TabList>
<Tabs.Tab tabId={ sidebars.document }>{ documentLabel }</Tabs.Tab>
<Tabs.Tab tabId={ sidebars.block }>
<Tabs.TabList ref={ ref }>
<Tabs.Tab
tabId={ sidebars.document }
// Used for focus management in the SettingsSidebar component.
data-tab-id={ sidebars.document }
>
{ documentLabel }
</Tabs.Tab>
<Tabs.Tab
tabId={ sidebars.block }
// Used for focus management in the SettingsSidebar component.
data-tab-id={ sidebars.block }
>
{ /* translators: Text label for the Block Settings Sidebar tab. */ }
{ __( 'Block' ) }
</Tabs.Tab>
</Tabs.TabList>
);
};

export default SettingsHeader;
export default forwardRef( SettingsHeader );
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ import {
store as blockEditorStore,
} from '@wordpress/block-editor';
import { useSelect, useDispatch } from '@wordpress/data';
import { Platform, useCallback, useContext } from '@wordpress/element';
import {
Platform,
useCallback,
useContext,
useEffect,
useRef,
} from '@wordpress/element';
import { isRTL, __ } from '@wordpress/i18n';
import { drawerLeft, drawerRight } from '@wordpress/icons';
import { store as interfaceStore } from '@wordpress/interface';
Expand Down Expand Up @@ -50,17 +56,45 @@ const SidebarContent = ( {
keyboardShortcut,
isEditingTemplate,
} ) => {
const tabListRef = useRef( null );
// Because `PluginSidebarEditPost` renders a `ComplementaryArea`, we
// need to forward the `Tabs` context so it can be passed through the
// underlying slot/fill.
const tabsContextValue = useContext( Tabs.Context );

// This effect addresses a race condition caused by tabbing from the last
// block in the editor into the settings sidebar. Without this effect, the
// selected tab and browser focus can become separated in an unexpected way
// (e.g the "block" tab is focused, but the "post" tab is selected).
useEffect( () => {
const tabsElements = Array.from(
tabListRef.current?.querySelectorAll( '[role="tab"]' ) || []
);
const selectedTabElement = tabsElements.find(
// We are purposefully using a custom `data-tab-id` attribute here
// because we don't want rely on any assumptions about `Tabs`
// component internals.
( element ) => element.getAttribute( 'data-tab-id' ) === sidebarName
);
const activeElement = selectedTabElement?.ownerDocument.activeElement;
const tabsHasFocus = tabsElements.some( ( element ) => {
return activeElement && activeElement.id === element.id;
} );
if (
tabsHasFocus &&
selectedTabElement &&
selectedTabElement.id !== activeElement?.id
) {
selectedTabElement?.focus();
}
}, [ sidebarName ] );

return (
<PluginSidebarEditPost
identifier={ sidebarName }
header={
<Tabs.Context.Provider value={ tabsContextValue }>
<SettingsHeader />
<SettingsHeader ref={ tabListRef } />
</Tabs.Context.Provider>
}
closeLabel={ __( 'Close Settings' ) }
Expand Down Expand Up @@ -157,6 +191,7 @@ const SettingsSidebar = () => {
// the selected tab to `null` avoids that.
selectedTabId={ isSettingsSidebarActive ? sidebarName : null }
onSelect={ onTabSelect }
selectOnMove={ false }
>
<SidebarContent
sidebarName={ sidebarName }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ test.describe( 'Order of block keyboard navigation', () => {
);

await page.keyboard.press( 'Tab' );
await KeyboardNavigableBlocks.expectLabelToHaveFocus( 'Block' );
await KeyboardNavigableBlocks.expectLabelToHaveFocus( 'Post' );
} );

test( 'allows tabbing in navigation mode if no block is selected (reverse)', async ( {
Expand Down

0 comments on commit 486b2db

Please sign in to comment.