Skip to content

Commit

Permalink
Fix Navigable Toolbar initialIndex (WordPress#51181)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
jeryj authored and sethrubenstein committed Jul 13, 2023
1 parent c68de22 commit e52857c
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 7 additions & 5 deletions packages/block-editor/src/components/navigable-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 );
};
Expand Down

0 comments on commit e52857c

Please sign in to comment.