Fix region navigation in the site editor (#46525)

WordPress · Dec 15, 2022 · 04158bc · 04158bc
1 parent b4bfe4f
commit 04158bc
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 102 deletions.
diff --git a/packages/base-styles/_z-index.scss b/packages/base-styles/_z-index.scss
@@ -142,6 +142,9 @@ $z-layers: (
 	".skip-to-selected-block": 100000,
 	".interface-interface-skeleton__actions": 100000,
 
+	// The focus styles of the region navigation containers should be above their content.
+	".is-focusing-regions {region} :focus::after": 1000000,
+
 	// Show NUX tips above popovers, wp-admin menus, submenus, and sidebar:
 	".nux-dot-tip": 1000001,
 
@@ -177,8 +180,6 @@ $z-layers: (
 
 	// Appear under the topbar.
 	".customize-widgets__block-toolbar": 7,
-
-	".is-focusing-regions [role='region']:focus .interface-navigable-region__stacker": -1,
 );
 
 @function z-index( $key ) {

diff --git a/packages/components/src/higher-order/navigate-regions/style.scss b/packages/components/src/higher-order/navigate-regions/style.scss
@@ -4,14 +4,17 @@
 }
 
 .is-focusing-regions {
-	[role="region"]:focus {
+	[role="region"]:focus::after {
+		position: absolute;
+		top: 0;
+		left: 0;
+		right: 0;
+		bottom: 0;
+		content: "";
+		pointer-events: none;
 		outline: 4px solid $components-color-accent;
 		outline-offset: -4px;
-
-		.interface-navigable-region__stacker {
-			position: relative;
-			z-index: z-index(".is-focusing-regions [role='region']:focus .interface-navigable-region__stacker");
-		}
+		z-index: z-index(".is-focusing-regions {region} :focus::after");
 	}
 
 	// Fixes for edge cases.
@@ -25,40 +28,11 @@
 	// regardles of the CSS used on other components.
 
 	// Header top bar when Distraction free mode is on.
-	&.is-distraction-free .interface-interface-skeleton__header {
-		.interface-navigable-region__stacker,
-		.edit-post-header {
-			outline: inherit;
-			outline-offset: inherit;
-		}
-	}
-
-	// Sidebar toggle button shown when navigating regions.
-	.interface-interface-skeleton__sidebar {
-		.interface-navigable-region__stacker,
-		.edit-post-layout__toggle-sidebar-panel {
-			outline: inherit;
-			outline-offset: inherit;
-		}
-	}
-
-	// Publish sidebar toggle button shown when navigating regions.
-	.interface-interface-skeleton__actions {
-		.interface-navigable-region__stacker,
-		.edit-post-layout__toggle-publish-panel {
-			outline: inherit;
-			outline-offset: inherit;
-		}
-	}
-
-	// Publish sidebar.
-	[role="region"].interface-interface-skeleton__actions:focus  .editor-post-publish-panel {
+	&.is-distraction-free .interface-interface-skeleton__header .edit-post-header,
+	.interface-interface-skeleton__sidebar .edit-post-layout__toggle-sidebar-panel,
+	.interface-interface-skeleton__actions .edit-post-layout__toggle-publish-panel,
+	.editor-post-publish-panel {
 		outline: 4px solid $components-color-accent;
 		outline-offset: -4px;
 	}
 }
-
-.interface-navigable-region__stacker {
-	height: 100%;
-	width: 100%;
-}
diff --git a/packages/edit-site/src/components/editor/index.js b/packages/edit-site/src/components/editor/index.js
@@ -22,7 +22,6 @@ import {
 	EntitiesSavedStates,
 } from '@wordpress/editor';
 import { __ } from '@wordpress/i18n';
-import { store as keyboardShortcutsStore } from '@wordpress/keyboard-shortcuts';
 
 /**
  * Internal dependencies
@@ -64,8 +63,6 @@ export default function Editor() {
 		isInserterOpen,
 		isListViewOpen,
 		isSaveViewOpen,
-		previousShortcut,
-		nextShortcut,
 		showIconLabels,
 	} = useSelect( ( select ) => {
 		const {
@@ -80,9 +77,6 @@ export default function Editor() {
 		} = select( editSiteStore );
 		const { hasFinishedResolution, getEntityRecord } = select( coreStore );
 		const { __unstableGetEditorMode } = select( blockEditorStore );
-		const { getAllShortcutKeyCombinations } = select(
-			keyboardShortcutsStore
-		);
 		const { getActiveComplementaryArea } = select( interfaceStore );
 		const postType = getEditedPostType();
 		const postId = getEditedPostId();
@@ -112,12 +106,6 @@ export default function Editor() {
 			isRightSidebarOpen: getActiveComplementaryArea(
 				editSiteStore.name
 			),
-			previousShortcut: getAllShortcutKeyCombinations(
-				'core/edit-site/previous-region'
-			),
-			nextShortcut: getAllShortcutKeyCombinations(
-				'core/edit-site/next-region'
-			),
 			showIconLabels: select( preferencesStore ).get(
 				'core/edit-site',
 				'showIconLabels'
@@ -178,6 +166,7 @@ export default function Editor() {
 						<BlockContextProvider value={ blockContext }>
 							<SidebarComplementaryAreaFills />
 							<InterfaceSkeleton
+								enableRegionNavigation={ false }
 								className={
 									showIconLabels && 'show-icon-labels'
 								}
@@ -258,10 +247,6 @@ export default function Editor() {
 										/>
 									)
 								}
-								shortcuts={ {
-									previous: previousShortcut,
-									next: nextShortcut,
-								} }
 								labels={ {
 									...interfaceLabels,
 									secondarySidebar: secondarySidebarLabel,

diff --git a/packages/edit-site/src/components/layout/index.js b/packages/edit-site/src/components/layout/index.js
@@ -12,6 +12,7 @@ import {
 	__experimentalHStack as HStack,
 	__unstableMotion as motion,
 	__unstableAnimatePresence as AnimatePresence,
+	__unstableUseNavigateRegions as useNavigateRegions,
 } from '@wordpress/components';
 import {
 	useReducedMotion,
@@ -22,6 +23,7 @@ import { __ } from '@wordpress/i18n';
 import { store as blockEditorStore } from '@wordpress/block-editor';
 import { useState, useEffect } from '@wordpress/element';
 import { NavigableRegion } from '@wordpress/interface';
+import { store as keyboardShortcutsStore } from '@wordpress/keyboard-shortcuts';
 
 /**
  * Internal dependencies
@@ -47,15 +49,28 @@ export default function Layout( { onError } ) {
 	const { params } = useLocation();
 	const isListPage = getIsListPage( params );
 	const isEditorPage = ! isListPage;
-	const { canvasMode, dashboardLink } = useSelect(
-		( select ) => ( {
-			canvasMode: select( editSiteStore ).__unstableGetCanvasMode(),
-			dashboardLink:
-				select( editSiteStore ).getSettings()
-					.__experimentalDashboardLink,
-		} ),
-		[]
-	);
+	const { canvasMode, dashboardLink, previousShortcut, nextShortcut } =
+		useSelect( ( select ) => {
+			const { getAllShortcutKeyCombinations } = select(
+				keyboardShortcutsStore
+			);
+			const { __unstableGetCanvasMode, getSettings } =
+				select( editSiteStore );
+			return {
+				canvasMode: __unstableGetCanvasMode(),
+				dashboardLink: getSettings().__experimentalDashboardLink,
+				previousShortcut: getAllShortcutKeyCombinations(
+					'core/edit-site/previous-region'
+				),
+				nextShortcut: getAllShortcutKeyCombinations(
+					'core/edit-site/next-region'
+				),
+			};
+		}, [] );
+	const navigateRegionsProps = useNavigateRegions( {
+		previous: previousShortcut,
+		next: nextShortcut,
+	} );
 	const { __unstableSetCanvasMode } = useDispatch( editSiteStore );
 	const { clearSelectedBlock } = useDispatch( blockEditorStore );
 	const disableMotion = useReducedMotion();
@@ -108,13 +123,19 @@ export default function Layout( { onError } ) {
 		<>
 			{ fullResizer }
 			<div
-				className={ classnames( 'edit-site-layout', {
-					'is-full-canvas':
-						( isEditorPage &&
-							canvasMode === 'edit' &&
-							! isMobileViewport ) ||
-						isMobileCanvasVisible,
-				} ) }
+				{ ...navigateRegionsProps }
+				ref={ navigateRegionsProps.ref }
+				className={ classnames(
+					'edit-site-layout',
+					navigateRegionsProps.className,
+					{
+						'is-full-canvas':
+							( isEditorPage &&
+								canvasMode === 'edit' &&
+								! isMobileViewport ) ||
+							isMobileCanvasVisible,
+					}
+				) }
 			>
 				<div className="edit-site-layout__header">
 					<div className="edit-site-layout__logo">
@@ -196,7 +217,8 @@ export default function Layout( { onError } ) {
 
 				<AnimatePresence initial={ false }>
 					{ showSidebar && (
-						<motion.div
+						<NavigableRegion
+							as={ motion.div }
 							initial={ {
 								opacity: 0,
 							} }
@@ -214,9 +236,10 @@ export default function Layout( { onError } ) {
 								ease: 'easeOut',
 							} }
 							className="edit-site-layout__sidebar"
+							ariaLabel={ __( 'Navigation sidebar' ) }
 						>
 							<Sidebar />
-						</motion.div>
+						</NavigableRegion>
 					) }
 				</AnimatePresence>
 

diff --git a/packages/edit-site/src/components/list/style.scss b/packages/edit-site/src/components/list/style.scss
@@ -44,18 +44,11 @@
 		.interface-interface-skeleton__content {
 			background: $white;
 			padding: $grid-unit-20;
-
-			.interface-navigable-region__stacker {
-				align-items: center;
-			}
+			align-items: center;
 
 			@include break-medium() {
 				padding: $grid-unit * 9;
 			}
-
-			& > .interface-navigable-region__stacker {
-				height: auto;
-			}
 		}
 	}
 }

diff --git a/packages/interface/src/components/interface-skeleton/index.js b/packages/interface/src/components/interface-skeleton/index.js
@@ -46,6 +46,7 @@ function InterfaceSkeleton(
 		actions,
 		labels,
 		className,
+		enableRegionNavigation = true,
 		// Todo: does this need to be a prop.
 		// Can we use a dependency to keyboard-shortcuts directly?
 		shortcuts,
@@ -83,8 +84,11 @@ function InterfaceSkeleton(
 
 	return (
 		<div
-			{ ...navigateRegionsProps }
-			ref={ useMergeRefs( [ ref, navigateRegionsProps.ref ] ) }
+			{ ...( enableRegionNavigation ? navigateRegionsProps : {} ) }
+			ref={ useMergeRefs( [
+				ref,
+				enableRegionNavigation ? navigateRegionsProps.ref : undefined,
+			] ) }
 			className={ classnames(
 				className,
 				'interface-interface-skeleton',

diff --git a/packages/interface/src/components/interface-skeleton/style.scss b/packages/interface/src/components/interface-skeleton/style.scss
@@ -72,16 +72,6 @@ html.interface-interface-skeleton__html-container {
 }
 
 .interface-interface-skeleton__content {
-	.interface-navigable-region__stacker {
-		// It's a flex item within a flex container with flex-direction column.
-		// Make sure it takes all the available height.
-		flex-grow: 1;
-		// Allow its flex items (the visual editor area) to take all the available
-		// height by making it also a flex container.
-		display: flex;
-		flex-direction: column;
-	}
-
 	flex-grow: 1;
 
 	// Treat as flex container to allow children to grow to occupy full
@@ -135,10 +125,6 @@ html.interface-interface-skeleton__html-container {
 }
 
 .interface-interface-skeleton__secondary-sidebar {
-	.interface-navigable-region__stacker {
-		height: 100%;
-	}
-
 	@include break-medium() {
 		border-right: $border-width solid $gray-200;
 	}

diff --git a/packages/interface/src/components/navigable-region/index.js b/packages/interface/src/components/navigable-region/index.js
@@ -18,9 +18,7 @@ export default function NavigableRegion( {
 			tabIndex="-1"
 			{ ...props }
 		>
-			<div className="interface-navigable-region__stacker">
-				{ children }
-			</div>
+			{ children }
 		</Tag>
 	);
 }