From 06acccc3aeb629bf74f4a364a0b9f81120e37cbc Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Mon, 6 May 2024 09:54:56 +0200 Subject: [PATCH 1/2] Trigger sidebar animations only on cross-route navigations --- .../sidebar-navigation-item/index.js | 2 +- .../sidebar-navigation-screen/index.js | 2 +- .../edit-site/src/components/sidebar/index.js | 90 ++++++++++++------- 3 files changed, 59 insertions(+), 35 deletions(-) diff --git a/packages/edit-site/src/components/sidebar-navigation-item/index.js b/packages/edit-site/src/components/sidebar-navigation-item/index.js index 69a2af4a45de4..8e979e7ab8bde 100644 --- a/packages/edit-site/src/components/sidebar-navigation-item/index.js +++ b/packages/edit-site/src/components/sidebar-navigation-item/index.js @@ -35,7 +35,7 @@ export default function SidebarNavigationItem( { ...props } ) { const history = useHistory(); - const navigate = useContext( SidebarNavigationContext ); + const { navigate } = useContext( SidebarNavigationContext ); // If there is no custom click handler, create one that navigates to `path`. function handleClick( e ) { diff --git a/packages/edit-site/src/components/sidebar-navigation-screen/index.js b/packages/edit-site/src/components/sidebar-navigation-screen/index.js index e9c11573ab9bf..19e3335f9ff06 100644 --- a/packages/edit-site/src/components/sidebar-navigation-screen/index.js +++ b/packages/edit-site/src/components/sidebar-navigation-screen/index.js @@ -62,7 +62,7 @@ export default function SidebarNavigationScreen( { ); const location = useLocation(); const history = useHistory(); - const navigate = useContext( SidebarNavigationContext ); + const { navigate } = useContext( SidebarNavigationContext ); const backPath = backPathProp ?? location.state?.backPath; const icon = isRTL() ? chevronRight : chevronLeft; diff --git a/packages/edit-site/src/components/sidebar/index.js b/packages/edit-site/src/components/sidebar/index.js index a246acaa46ce7..42af483a37cc4 100644 --- a/packages/edit-site/src/components/sidebar/index.js +++ b/packages/edit-site/src/components/sidebar/index.js @@ -7,8 +7,8 @@ import clsx from 'clsx'; * WordPress dependencies */ import { - useCallback, createContext, + useContext, useState, useRef, useEffect, @@ -16,53 +16,77 @@ import { import { focus } from '@wordpress/dom'; export const SidebarNavigationContext = createContext( () => {} ); +// Focus a sidebar element after a navigation. The element to focus is either +// specified by `focusSelector` (when navigating back) or it is the first +// tabbable element (usually the "Back" button). +function focusSidebarElement( el, direction, focusSelector ) { + let elementToFocus; + if ( direction === 'back' && focusSelector ) { + elementToFocus = el.querySelector( focusSelector ); + } + if ( direction !== null && ! elementToFocus ) { + const [ firstTabbable ] = focus.tabbable.find( el ); + elementToFocus = firstTabbable ?? el; + } + elementToFocus?.focus(); +} -export default function SidebarContent( { routeKey, children } ) { - const [ navState, setNavState ] = useState( { +// Navigation state that is updated when navigating back or forward. Helps us +// manage the animations and also focus. +function createNavState() { + let state = { direction: null, focusSelector: null, - } ); + }; - const navigate = useCallback( ( direction, focusSelector = null ) => { - setNavState( ( prevState ) => ( { - direction, - focusSelector: - direction === 'forward' && focusSelector - ? focusSelector - : prevState.focusSelector, - } ) ); - }, [] ); + return { + get() { + return state; + }, + navigate( direction, focusSelector = null ) { + state = { + direction, + focusSelector: + direction === 'forward' && focusSelector + ? focusSelector + : state.focusSelector, + }; + }, + }; +} +function SidebarContentWrapper( { children } ) { + const navState = useContext( SidebarNavigationContext ); const wrapperRef = useRef(); + const [ navAnimation, setNavAnimation ] = useState( null ); + useEffect( () => { - let elementToFocus; - if ( navState.direction === 'back' && navState.focusSelector ) { - elementToFocus = wrapperRef.current.querySelector( - navState.focusSelector - ); - } - if ( navState.direction !== null && ! elementToFocus ) { - const [ firstTabbable ] = focus.tabbable.find( wrapperRef.current ); - elementToFocus = firstTabbable ?? wrapperRef.current; - } - elementToFocus?.focus(); + const { direction, focusSelector } = navState.get(); + focusSidebarElement( wrapperRef.current, direction, focusSelector ); + setNavAnimation( direction ); }, [ navState ] ); const wrapperCls = clsx( 'edit-site-sidebar__screen-wrapper', { - 'slide-from-left': navState.direction === 'back', - 'slide-from-right': navState.direction === 'forward', + 'slide-from-left': navAnimation === 'back', + 'slide-from-right': navAnimation === 'forward', } ); return ( - +
+ { children } +
+ ); +} + +export default function SidebarContent( { routeKey, children } ) { + const [ navState ] = useState( createNavState ); + + return ( +
-
+ { children } -
+
); From 7c861dff9fe19c755bbc839e503a2842a038ad2c Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Tue, 7 May 2024 10:46:04 +0200 Subject: [PATCH 2/2] Use layout effect when triggering animation --- packages/edit-site/src/components/sidebar/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/edit-site/src/components/sidebar/index.js b/packages/edit-site/src/components/sidebar/index.js index 42af483a37cc4..84820952e1b62 100644 --- a/packages/edit-site/src/components/sidebar/index.js +++ b/packages/edit-site/src/components/sidebar/index.js @@ -11,7 +11,7 @@ import { useContext, useState, useRef, - useEffect, + useLayoutEffect, } from '@wordpress/element'; import { focus } from '@wordpress/dom'; @@ -60,7 +60,7 @@ function SidebarContentWrapper( { children } ) { const wrapperRef = useRef(); const [ navAnimation, setNavAnimation ] = useState( null ); - useEffect( () => { + useLayoutEffect( () => { const { direction, focusSelector } = navState.get(); focusSidebarElement( wrapperRef.current, direction, focusSelector ); setNavAnimation( direction );