Tabs: remove custom logic (WordPress#66097)

Co-authored-by: ciampo <[email protected]> Co-authored-by: jeryj <[email protected]> Co-authored-by: tyxla <[email protected]>
karthick-murugan · Nov 13, 2024 · b96bc6f · b96bc6f
1 parent d3606ad
commit b96bc6f
Show file tree

Hide file tree

Showing 5 changed files with 212 additions and 259 deletions.
diff --git a/packages/block-editor/src/components/inserter/category-tabs/index.js b/packages/block-editor/src/components/inserter/category-tabs/index.js
@@ -6,6 +6,7 @@ import {
 	privateApis as componentsPrivateApis,
 	__unstableMotion as motion,
 } from '@wordpress/components';
+import { useState, useEffect } from '@wordpress/element';
 
 /**
  * Internal dependencies
@@ -31,10 +32,22 @@ function CategoryTabs( {
 
 	const previousSelectedCategory = usePrevious( selectedCategory );
 
+	const selectedTabId = selectedCategory ? selectedCategory.name : null;
+	const [ activeTabId, setActiveId ] = useState();
+	const firstTabId = categories?.[ 0 ]?.name;
+	useEffect( () => {
+		// If there is no active tab, make the first tab the active tab, so that
+		// when focus is moved to the tablist, the first tab will be focused
+		// despite not being selected
+		if ( selectedTabId === null && ! activeTabId && firstTabId ) {
+			setActiveId( firstTabId );
+		}
+	}, [ selectedTabId, activeTabId, firstTabId, setActiveId ] );
+
 	return (
 		<Tabs
 			selectOnMove={ false }
-			selectedTabId={ selectedCategory ? selectedCategory.name : null }
+			selectedTabId={ selectedTabId }
 			orientation="vertical"
 			onSelect={ ( categoryId ) => {
 				// Pass the full category object
@@ -44,6 +57,8 @@ function CategoryTabs( {
 					)
 				);
 			} }
+			activeTabId={ activeTabId }
+			onActiveTabIdChange={ setActiveId }
 		>
 			<Tabs.TabList className="block-editor-inserter__category-tablist">
 				{ categories.map( ( category ) => (

diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
@@ -19,6 +19,7 @@
 
 ### Experimental
 
+-   `Tabs`: remove internal custom logic ([#66097](https://github.com/WordPress/gutenberg/pull/66097)).
 -   `Tabs`: add props to control active tab item ([#66223](https://github.com/WordPress/gutenberg/pull/66223)).
 -   `Tabs`: restore vertical alignent for tabs content ([#66215](https://github.com/WordPress/gutenberg/pull/66215)).
 -   `Tabs`: fix indicator animation ([#66198](https://github.com/WordPress/gutenberg/pull/66198)).

diff --git a/packages/components/src/tabs/index.tsx b/packages/components/src/tabs/index.tsx
@@ -2,18 +2,12 @@
  * External dependencies
  */
 import * as Ariakit from '@ariakit/react';
-import { useStoreState } from '@ariakit/react';
 
 /**
  * WordPress dependencies
  */
 import { useInstanceId } from '@wordpress/compose';
-import {
-	useEffect,
-	useLayoutEffect,
-	useMemo,
-	useRef,
-} from '@wordpress/element';
+import { useEffect, useMemo } from '@wordpress/element';
 import { isRTL } from '@wordpress/i18n';
 
 /**
@@ -25,6 +19,22 @@ import { Tab } from './tab';
 import { TabList } from './tablist';
 import { TabPanel } from './tabpanel';
 
+function externalToInternalTabId(
+	externalId: string | undefined | null,
+	instanceId: string
+) {
+	return externalId && `${ instanceId }-${ externalId }`;
+}
+
+function internalToExternalTabId(
+	internalId: string | undefined | null,
+	instanceId: string
+) {
+	return typeof internalId === 'string'
+		? internalId.replace( `${ instanceId }-`, '' )
+		: internalId;
+}
+
 /**
  * Display one panel of content at a time with a tabbed interface, based on the
  * WAI-ARIA Tabs Pattern⁠.
@@ -40,147 +50,41 @@ export const Tabs = Object.assign(
 		onSelect,
 		children,
 		selectedTabId,
+		activeTabId,
+		defaultActiveTabId,
+		onActiveTabIdChange,
 	}: TabsProps ) {
 		const instanceId = useInstanceId( Tabs, 'tabs' );
 		const store = Ariakit.useTabStore( {
 			selectOnMove,
 			orientation,
-			defaultSelectedId:
-				defaultTabId && `${ instanceId }-${ defaultTabId }`,
-			setSelectedId: ( selectedId ) => {
-				const strippedDownId =
-					typeof selectedId === 'string'
-						? selectedId.replace( `${ instanceId }-`, '' )
-						: selectedId;
-				onSelect?.( strippedDownId );
+			defaultSelectedId: externalToInternalTabId(
+				defaultTabId,
+				instanceId
+			),
+			setSelectedId: ( newSelectedId ) => {
+				onSelect?.(
+					internalToExternalTabId( newSelectedId, instanceId )
+				);
+			},
+			selectedId: externalToInternalTabId( selectedTabId, instanceId ),
+			defaultActiveId: externalToInternalTabId(
+				defaultActiveTabId,
+				instanceId
+			),
+			setActiveId: ( newActiveId ) => {
+				onActiveTabIdChange?.(
+					internalToExternalTabId( newActiveId, instanceId )
+				);
 			},
-			selectedId: selectedTabId && `${ instanceId }-${ selectedTabId }`,
+			activeId: externalToInternalTabId( activeTabId, instanceId ),
 			rtl: isRTL(),
 		} );
 
-		const isControlled = selectedTabId !== undefined;
-
-		const { items, selectedId, activeId } = useStoreState( store );
-		const { setSelectedId, setActiveId } = 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
-		// variables are undefined during the initial render.
-		const tabsHavePopulatedRef = useRef( false );
-		if ( items.length > 0 ) {
-			tabsHavePopulatedRef.current = true;
-		}
-
-		const selectedTab = items.find( ( item ) => item.id === selectedId );
-		const firstEnabledTab = items.find( ( item ) => {
-			// Ariakit internally refers to disabled tabs as `dimmed`.
-			return ! item.dimmed;
-		} );
-		const initialTab = items.find(
-			( item ) => item.id === `${ instanceId }-${ defaultTabId }`
-		);
-
-		// Handle selecting the initial tab.
-		useLayoutEffect( () => {
-			if ( isControlled ) {
-				return;
-			}
-
-			// 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, as well as ensuring no tab is
-			// selected if an invalid `defaultTabId` is provided.
-			if ( defaultTabId && ! initialTab ) {
-				return;
-			}
-
-			// If the currently selected tab is missing (i.e. removed from the DOM),
-			// fall back to the initial tab or the first enabled tab if there is
-			// one. Otherwise, no tab should be selected.
-			if ( ! items.find( ( item ) => item.id === selectedId ) ) {
-				if ( initialTab && ! initialTab.dimmed ) {
-					setSelectedId( initialTab?.id );
-					return;
-				}
-
-				if ( firstEnabledTab ) {
-					setSelectedId( firstEnabledTab.id );
-				} else if ( tabsHavePopulatedRef.current ) {
-					setSelectedId( null );
-				}
-			}
-		}, [
-			firstEnabledTab,
-			initialTab,
-			defaultTabId,
-			isControlled,
-			items,
-			selectedId,
-			setSelectedId,
-		] );
-
-		// Handle the currently selected tab becoming disabled.
-		useLayoutEffect( () => {
-			if ( ! selectedTab?.dimmed ) {
-				return;
-			}
-
-			// In controlled mode, we trust that disabling tabs is done
-			// intentionally, and don't select a new tab automatically.
-			if ( isControlled ) {
-				setSelectedId( null );
-				return;
-			}
-
-			// If the currently selected tab becomes disabled, fall back to the
-			// `defaultTabId` if possible. Otherwise select the first
-			// enabled tab (if there is one).
-			if ( initialTab && ! initialTab.dimmed ) {
-				setSelectedId( initialTab.id );
-				return;
-			}
-
-			if ( firstEnabledTab ) {
-				setSelectedId( firstEnabledTab.id );
-			}
-		}, [
-			firstEnabledTab,
-			initialTab,
-			isControlled,
-			selectedTab?.dimmed,
-			setSelectedId,
-		] );
-
-		// Clear `selectedId` if the active tab is removed from the DOM in controlled mode.
-		useLayoutEffect( () => {
-			if ( ! isControlled ) {
-				return;
-			}
-
-			// Once the tabs have populated, if the `selectedTabId` still can't be
-			// found, clear the selection.
-			if (
-				tabsHavePopulatedRef.current &&
-				!! selectedTabId &&
-				! selectedTab
-			) {
-				setSelectedId( null );
-			}
-		}, [ isControlled, selectedTab, selectedTabId, setSelectedId ] );
+		const { items, activeId } = Ariakit.useStoreState( store );
+		const { setActiveId } = store;
 
 		useEffect( () => {
-			// If there is no active tab, fallback to place focus on the first enabled tab
-			// so there is always an active element
-			if ( selectedTabId === null && ! activeId && firstEnabledTab?.id ) {
-				setActiveId( firstEnabledTab.id );
-			}
-		}, [ selectedTabId, activeId, firstEnabledTab?.id, setActiveId ] );
-
-		useEffect( () => {
-			if ( ! isControlled ) {
-				return;
-			}
-
 			requestAnimationFrame( () => {
 				const focusedElement =
 					items?.[ 0 ]?.element?.ownerDocument.activeElement;
@@ -200,7 +104,7 @@ export const Tabs = Object.assign(
 					setActiveId( focusedElement.id );
 				}
 			} );
-		}, [ activeId, isControlled, items, setActiveId ] );
+		}, [ activeId, items, setActiveId ] );
 
 		const contextValue = useMemo(
 			() => ( {

diff --git a/packages/components/src/tabs/tab.tsx b/packages/components/src/tabs/tab.tsx
@@ -1,3 +1,8 @@
+/**
+ * External dependencies
+ */
+import * as Ariakit from '@ariakit/react';
+
 /**
  * WordPress dependencies
  */
@@ -22,20 +27,35 @@ export const Tab = forwardRef<
 	HTMLButtonElement,
 	Omit< WordPressComponentProps< TabProps, 'button', false >, 'id' >
 >( function Tab( { children, tabId, disabled, render, ...otherProps }, ref ) {
-	const context = useTabsContext();
-	if ( ! context ) {
+	const { store, instanceId } = useTabsContext() ?? {};
+
+	// If the active item is not connected, the tablist may end up in a state
+	// where none of the tabs are tabbable. In this case, we force all tabs to
+	// be tabbable, so that as soon as an item received focus, it becomes active
+	// and Tablist goes back to working as expected.
+	// eslint-disable-next-line @wordpress/no-unused-vars-before-return
+	const tabbable = Ariakit.useStoreState( store, ( state ) => {
+		return (
+			state?.activeId !== null &&
+			! store?.item( state?.activeId )?.element?.isConnected
+		);
+	} );
+
+	if ( ! store ) {
 		warning( '`Tabs.Tab` must be wrapped in a `Tabs` component.' );
 		return null;
 	}
-	const { store, instanceId } = context;
+
 	const instancedTabId = `${ instanceId }-${ tabId }`;
+
 	return (
 		<StyledTab
 			ref={ ref }
 			store={ store }
 			id={ instancedTabId }
 			disabled={ disabled }
 			render={ render }
+			tabbable={ tabbable }
 			{ ...otherProps }
 		>
 			<StyledTabChildren>{ children }</StyledTabChildren>