From e52857c6533656368a67c77aee6dcf5044d4df1a Mon Sep 17 00:00:00 2001
From: Jerry Jones <jones.jeremydavid@gmail.com>
Date: Wed, 7 Jun 2023 10:59:04 -0500
Subject: [PATCH] Fix Navigable Toolbar initialIndex (#51181)

* Store navigableToolbarRef so we have access on useEffect cleanup

* Always set initialIndex to 0 if undefined

If the initialIndex was undefined, the toolbar would set the tabindex=0 to the last item in the toolbar, which was an unexpected behavior.

* Reset toolbar focus index on clientId change
---
 .../components/block-tools/selected-block-popover.js |  6 ++++++
 .../src/components/navigable-toolbar/index.js        | 12 +++++++-----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/packages/block-editor/src/components/block-tools/selected-block-popover.js b/packages/block-editor/src/components/block-tools/selected-block-popover.js
index d6268567b13a3c..8c87d8ea3a4739 100644
--- a/packages/block-editor/src/components/block-tools/selected-block-popover.js
+++ b/packages/block-editor/src/components/block-tools/selected-block-popover.js
@@ -102,6 +102,12 @@ function SelectedBlockPopover( {
 	// to it when re-mounting.
 	const initialToolbarItemIndexRef = useRef();
 
+	useEffect( () => {
+		// Resets the index whenever the active block changes so this is not
+		// persisted. See https://github.com/WordPress/gutenberg/pull/25760#issuecomment-717906169
+		initialToolbarItemIndexRef.current = undefined;
+	}, [ clientId ] );
+
 	const popoverProps = useBlockToolbarPopoverProps( {
 		contentElement: __unstableContentRef?.current,
 		clientId,
diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js
index cc2b6e67b57825..3e531c93c11989 100644
--- a/packages/block-editor/src/components/navigable-toolbar/index.js
+++ b/packages/block-editor/src/components/navigable-toolbar/index.js
@@ -120,16 +120,18 @@ function useToolbarFocus(
 	}, [ isAccessibleToolbar, initialFocusOnMount, focusToolbar ] );
 
 	useEffect( () => {
+		// Store ref so we have access on useEffect cleanup: https://legacy.reactjs.org/blog/2020/08/10/react-v17-rc.html#effect-cleanup-timing
+		const navigableToolbarRef = ref.current;
 		// If initialIndex is passed, we focus on that toolbar item when the
 		// toolbar gets mounted and initial focus is not forced.
 		// We have to wait for the next browser paint because block controls aren't
 		// rendered right away when the toolbar gets mounted.
 		let raf = 0;
-		if ( initialIndex && ! initialFocusOnMount ) {
+		if ( ! initialFocusOnMount ) {
 			raf = window.requestAnimationFrame( () => {
-				const items = getAllToolbarItemsIn( ref.current );
+				const items = getAllToolbarItemsIn( navigableToolbarRef );
 				const index = initialIndex || 0;
-				if ( items[ index ] && hasFocusWithin( ref.current ) ) {
+				if ( items[ index ] && hasFocusWithin( navigableToolbarRef ) ) {
 					items[ index ].focus( {
 						// When focusing newly mounted toolbars,
 						// the position of the popover is often not right on the first render
@@ -141,10 +143,10 @@ function useToolbarFocus(
 		}
 		return () => {
 			window.cancelAnimationFrame( raf );
-			if ( ! onIndexChange || ! ref.current ) return;
+			if ( ! onIndexChange || ! navigableToolbarRef ) return;
 			// When the toolbar element is unmounted and onIndexChange is passed, we
 			// pass the focused toolbar item index so it can be hydrated later.
-			const items = getAllToolbarItemsIn( ref.current );
+			const items = getAllToolbarItemsIn( navigableToolbarRef );
 			const index = items.findIndex( ( item ) => item.tabIndex === 0 );
 			onIndexChange( index );
 		};