Tabs: Fix active tab switching from a stale inner-block selection

packages/block-library/src/tab-list/edit.js

@@ -15,7 +15,7 @@ import {
 	__experimentalUseColorProps as useColorProps,
 	__experimentalGetSpacingClassesAndStyles as getSpacingClassesAndStyles,
 } from '@wordpress/block-editor';
-import { useSelect, useDispatch } from '@wordpress/data';
+import { useSelect, useDispatch, useRegistry } from '@wordpress/data';
 import { useEffect, useMemo, useRef } from '@wordpress/element';
 
 /**
@@ -56,10 +56,14 @@ function Edit( {
 			},
 			[ clientId ]
 		);
+	const registry = useRegistry();
 	const { isBlockSelected, hasSelectedInnerBlock } =
 		useSelect( blockEditorStore );
-	const { updateBlockAttributes, __unstableMarkNextChangeAsNotPersistent } =
-		useDispatch( blockEditorStore );
+	const {
+		updateBlockAttributes,
+		selectBlock,
+		__unstableMarkNextChangeAsNotPersistent,
+	} = useDispatch( blockEditorStore );
 	const { insertTab, removeTab } = useTabActions( tabsClientId );
 
 	const effectiveActiveIndex = editorActiveTabIndex ?? activeTabIndex;
@@ -74,9 +78,15 @@ function Edit( {
 
 	function selectTabPanel( tabIndex ) {
 		if ( tabsClientId && tabIndex !== effectiveActiveIndex ) {
-			__unstableMarkNextChangeAsNotPersistent();
-			updateBlockAttributes( tabsClientId, {
-				editorActiveTabIndex: tabIndex,
+			// Batch the selection and index update so the sync effect in
+			// the tab-panel block can't revert the switch from a stale
+			// inner-block selection in the previously active panel.
+			registry.batch( () => {
+				selectBlock( clientId );
+				__unstableMarkNextChangeAsNotPersistent();
+				updateBlockAttributes( tabsClientId, {
+					editorActiveTabIndex: tabIndex,
+				} );
 			} );
 		}
 	}
@@ -89,14 +99,16 @@ function Edit( {
 	}
 
 	const menuRef = useRef();
-	const prevTabCountRef = useRef( tabsList.length );
+	const prevTabIdsRef = useRef( tabsList.map( ( tab ) => tab.clientId ) );
 
 	// When tabs are added or removed, focus the appropriate button.
 	useEffect( () => {
-		const prevCount = prevTabCountRef.current;
-		prevTabCountRef.current = tabsList.length;
+		const prevTabIds = prevTabIdsRef.current;
+		const tabIds = tabsList.map( ( tab ) => tab.clientId );
+		prevTabIdsRef.current = tabIds;
 
-		if ( ! menuRef.current || tabsList.length === prevCount ) {
+		// Only react when a tab was actually added or removed.
+		if ( ! menuRef.current || tabIds.length === prevTabIds.length ) {
 			return;
 		}
 
@@ -118,13 +130,40 @@ function Edit( {
 			} );
 		};
 
-		focusButtonAt( effectiveActiveIndex );
+		// The editor-only active index is a non-persistent attribute, so it
+		// isn't restored on undo/redo. Derive the tab to activate from the
+		// structural change.
+		const addedTabIndex = tabIds.findIndex(
+			( id ) => ! prevTabIds.includes( id )
+		);
+		let targetTabIndex;
+		if ( addedTabIndex !== -1 ) {
+			// A tab appeared (a new tab or undoing a removal): activate it.
+			targetTabIndex = addedTabIndex;
+		} else {
+			// A tab disappeared (undoing an insertion or a removal): activate
+			// the tab adjacent to the removed one.
+			const removedTabIndex = prevTabIds.findIndex(
+				( id ) => ! tabIds.includes( id )
+			);
+			targetTabIndex = Math.min( removedTabIndex, tabIds.length - 1 );
+		}
+
+		if ( targetTabIndex !== effectiveActiveIndex ) {
+			__unstableMarkNextChangeAsNotPersistent();
+			updateBlockAttributes( tabsClientId, {
+				editorActiveTabIndex: targetTabIndex,
+			} );
+		}
+		focusButtonAt( targetTabIndex );
 	}, [
 		effectiveActiveIndex,
 		hasSelectedInnerBlock,
 		isBlockSelected,
 		tabsClientId,
-		tabsList.length,
+		tabsList,
+		updateBlockAttributes,
+		__unstableMarkNextChangeAsNotPersistent,
 	] );
 
 	const blockProps = useBlockProps( {

test/e2e/specs/editor/blocks/tabs.spec.js

@@ -0,0 +1,239 @@
+/**
+ * WordPress dependencies
+ */
+const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' );
+
+test.describe( 'Tabs', () => {
+	test.beforeAll( async ( { requestUtils } ) => {
+		await requestUtils.setGutenbergExperiments( [
+			'gutenberg-block-experiments',
+		] );
+	} );
+
+	test.afterAll( async ( { requestUtils } ) => {
+		await requestUtils.setGutenbergExperiments( [] );
+	} );
+
+	test.describe( 'Editor functionality', () => {
+		test.beforeEach( async ( { admin, editor } ) => {
+			await admin.createNewPost();
+			await editor.insertBlock( {
+				name: 'core/tabs',
+				innerBlocks: [
+					{ name: 'core/tab-list' },
+					{
+						name: 'core/tab-panels',
+						innerBlocks: [
+							{
+								name: 'core/tab-panel',
+								attributes: { label: 'Tab 1' },
+								innerBlocks: [
+									{
+										name: 'core/paragraph',
+										attributes: { content: 'Panel 1' },
+									},
+								],
+							},
+							{
+								name: 'core/tab-panel',
+								attributes: { label: 'Tab 2' },
+								innerBlocks: [
+									{
+										name: 'core/paragraph',
+										attributes: { content: 'Panel 2' },
+									},
+								],
+							},
+						],
+					},
+				],
+			} );
+		} );
+
+		test( 'activates the next tab when the caret moves into its label with the right arrow key', async ( {
+			editor,
+			page,
+			pageUtils,
+		} ) => {
+			const tab1 = editor.canvas.getByRole( 'tab', { name: 'Tab 1' } );
+			const tab2 = editor.canvas.getByRole( 'tab', { name: 'Tab 2' } );
+
+			await tab1.click();
+			await expect( tab1 ).toHaveAttribute( 'aria-selected', 'true' );
+			await expect( tab2 ).toHaveAttribute( 'aria-selected', 'false' );
+			await expect( editor.canvas.getByText( 'Panel 1' ) ).toBeVisible();
+			await expect( editor.canvas.getByText( 'Panel 2' ) ).toBeHidden();
+
+			await page.keyboard.press( 'End' );
+			await page.keyboard.press( 'ArrowRight' );
+
+			await expect( tab1 ).toHaveAttribute( 'aria-selected', 'false' );
+			await expect( tab2 ).toHaveAttribute( 'aria-selected', 'true' );
+			await expect(
+				tab2.locator( '[contenteditable="true"]' )
+			).toBeFocused();
+			await expect( editor.canvas.getByText( 'Panel 1' ) ).toBeHidden();
+			await expect( editor.canvas.getByText( 'Panel 2' ) ).toBeVisible();
+
+			// Switching the active tab is non-persistent, so undo clears the
+			// inserted block instead of only reverting the tab switch.
+			await pageUtils.pressKeys( 'primary+z' );
+			await expect.poll( editor.getBlocks ).toEqual( [] );
+		} );
+
+		test( 'activates the previous tab when the caret moves into its label with the left arrow key', async ( {
+			editor,
+			page,
+			pageUtils,
+		} ) => {
+			const tab1 = editor.canvas.getByRole( 'tab', { name: 'Tab 1' } );
+			const tab2 = editor.canvas.getByRole( 'tab', { name: 'Tab 2' } );
+
+			await tab2.click();
+			await expect( tab1 ).toHaveAttribute( 'aria-selected', 'false' );
+			await expect( tab2 ).toHaveAttribute( 'aria-selected', 'true' );
+			await expect( editor.canvas.getByText( 'Panel 1' ) ).toBeHidden();
+			await expect( editor.canvas.getByText( 'Panel 2' ) ).toBeVisible();
+
+			await page.keyboard.press( 'Home' );
+			await page.keyboard.press( 'ArrowLeft' );
+
+			await expect( tab1 ).toHaveAttribute( 'aria-selected', 'true' );
+			await expect( tab2 ).toHaveAttribute( 'aria-selected', 'false' );
+			await expect(
+				tab1.locator( '[contenteditable="true"]' )
+			).toBeFocused();
+			await expect( editor.canvas.getByText( 'Panel 1' ) ).toBeVisible();
+			await expect( editor.canvas.getByText( 'Panel 2' ) ).toBeHidden();
+
+			// Switching the active tab is non-persistent, so undo clears the
+			// inserted block instead of only reverting the tab switch.
+			await pageUtils.pressKeys( 'primary+z' );
+			await expect.poll( editor.getBlocks ).toEqual( [] );
+		} );
+
+		test( 'switches the active tab and selects the tab list when a tab is clicked while a block in another panel is selected', async ( {
+			editor,
+			pageUtils,
+		} ) => {
+			const tab1 = editor.canvas.getByRole( 'tab', { name: 'Tab 1' } );
+			const tab2 = editor.canvas.getByRole( 'tab', { name: 'Tab 2' } );
+
+			// Select the paragraph inside the first (active) panel.
+			await editor.canvas.getByText( 'Panel 1' ).click();
+			await expect( tab1 ).toHaveAttribute( 'aria-selected', 'true' );
+			await expect( tab2 ).toHaveAttribute( 'aria-selected', 'false' );
+
+			await tab2.click();
+
+			await expect( tab1 ).toHaveAttribute( 'aria-selected', 'false' );
+			await expect( tab2 ).toHaveAttribute( 'aria-selected', 'true' );
+			await expect( editor.canvas.getByText( 'Panel 1' ) ).toBeHidden();
+			await expect( editor.canvas.getByText( 'Panel 2' ) ).toBeVisible();
+
+			await expect(
+				editor.canvas.getByRole( 'document', {
+					name: 'Block: Tab List',
+				} )
+			).toHaveClass( /is-selected/ );
+
+			// Switching the active tab is non-persistent, so undo clears the
+			// inserted block instead of only reverting the tab switch.
+			await pageUtils.pressKeys( 'primary+z' );
+			await expect.poll( editor.getBlocks ).toEqual( [] );
+		} );
+
+		test( 'adds and activates a new tab when pressing Enter at the end of a tab label', async ( {
+			editor,
+			page,
+			pageUtils,
+		} ) => {
+			const tab2 = editor.canvas.getByRole( 'tab', { name: 'Tab 2' } );
+			await tab2.click();
+			await expect( tab2 ).toHaveAttribute( 'aria-selected', 'true' );
+
+			await page.keyboard.press( 'End' );
+			await page.keyboard.press( 'Enter' );
+
+			const tabs = editor.canvas.getByRole( 'tab' );
+			await expect( tabs ).toHaveCount( 3 );
+
+			await expect( tabs.nth( 0 ) ).toHaveAttribute(
+				'aria-selected',
+				'false'
+			);
+			await expect( tabs.nth( 1 ) ).toHaveAttribute(
+				'aria-selected',
+				'false'
+			);
+
+			const newTab = tabs.nth( 2 );
+			await expect( newTab ).toHaveAttribute( 'aria-selected', 'true' );
+			await expect(
+				newTab.locator( '[contenteditable="true"]' )
+			).toBeFocused();
+
+			// The new tab's panel is the active one and is visible.
+			const panels = editor.canvas.getByRole( 'document', {
+				name: 'Block: Tab Panel',
+			} );
+			await expect( panels ).toHaveCount( 3 );
+			await expect( panels.nth( 2 ) ).toBeVisible();
+
+			// The tab insertion is persistent, so undo removes the new tab. The
+			// tab adjacent to the removed one (the second) becomes the active,
+			// focused one.
+			await pageUtils.pressKeys( 'primary+z' );
+			await expect( tabs ).toHaveCount( 2 );
+
+			await expect( tabs.nth( 1 ) ).toHaveAttribute(
+				'aria-selected',
+				'true'
+			);
+			await expect(
+				tabs.nth( 1 ).locator( '[contenteditable="true"]' )
+			).toBeFocused();
+		} );
+
+		test( 'removes the tab and activates the previous one when pressing Delete on an empty tab label', async ( {
+			editor,
+			page,
+			pageUtils,
+		} ) => {
+			const tab2 = editor.canvas.getByRole( 'tab', { name: 'Tab 2' } );
+			await tab2.click();
+			await expect( tab2 ).toHaveAttribute( 'aria-selected', 'true' );
+
+			// Empty the tab label, then delete again to remove the tab.
+			await pageUtils.pressKeys( 'primary+a' );
+			await page.keyboard.press( 'Delete' );
+			await page.keyboard.press( 'Backspace' );
+
+			const tabs = editor.canvas.getByRole( 'tab' );
+			await expect( tabs ).toHaveCount( 1 );
+
+			const tab1 = editor.canvas.getByRole( 'tab', { name: 'Tab 1' } );
+			await expect( tab1 ).toHaveAttribute( 'aria-selected', 'true' );
+			await expect(
+				tab1.locator( '[contenteditable="true"]' )
+			).toBeFocused();
+			await expect( editor.canvas.getByText( 'Panel 1' ) ).toBeVisible();
+
+			// The tab removal is persistent, so undo brings the tab back. The
+			// restored (second) tab becomes the active, focused one.
+			await pageUtils.pressKeys( 'primary+z' );
+			await expect( tabs ).toHaveCount( 2 );
+
+			await expect( tabs.nth( 1 ) ).toHaveAttribute(
+				'aria-selected',
+				'true'
+			);
+			await expect(
+				tabs.nth( 1 ).locator( '[contenteditable="true"]' )
+			).toBeFocused();
+		} );
+	} );
+
+	// TODO: Add a `Frontend functionality` describe block for front-end
+	// interaction tests (e.g. switching tabs on the published post).
+} );