Fix browser history when synchronising state with urls (#48731)

WordPress · Mar 6, 2023 · 9de4653 · 9de4653
1 parent 637bbcd
commit 9de4653
Show file tree

Hide file tree

Showing 10 changed files with 177 additions and 73 deletions.
diff --git a/packages/edit-site/src/components/list/index.js b/packages/edit-site/src/components/list/index.js
@@ -19,8 +19,10 @@ import useTitle from '../routes/use-title';
 
 export default function List() {
 	const {
-		params: { postType: templateType },
+		params: { path },
 	} = useLocation();
+	const templateType =
+		path === '/wp_template/all' ? 'wp_template' : 'wp_template_part';
 
 	useRegisterShortcuts();
 

diff --git a/packages/edit-site/src/components/routes/link.js b/packages/edit-site/src/components/routes/link.js
@@ -1,7 +1,7 @@
 /**
  * WordPress dependencies
  */
-import { addQueryArgs } from '@wordpress/url';
+import { addQueryArgs, getQueryArgs, removeQueryArgs } from '@wordpress/url';
 
 /**
  * Internal dependencies
@@ -21,8 +21,15 @@ export function useLink( params = {}, state, shouldReplace = false ) {
 		}
 	}
 
+	const currentArgs = getQueryArgs( window.location.href );
+	const currentUrlWithoutArgs = removeQueryArgs(
+		window.location.href,
+		...Object.keys( currentArgs )
+	);
+	const newUrl = addQueryArgs( currentUrlWithoutArgs, params );
+
 	return {
-		href: addQueryArgs( window.location.href, params ),
+		href: newUrl,
 		onClick,
 	};
 }

diff --git a/packages/edit-site/src/components/sidebar-navigation-screen-navigation-item/index.js b/packages/edit-site/src/components/sidebar-navigation-screen-navigation-item/index.js
@@ -2,12 +2,12 @@
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
-import { useDispatch, useSelect } from '@wordpress/data';
+import { useDispatch } from '@wordpress/data';
 import {
 	__experimentalUseNavigator as useNavigator,
 	ExternalLink,
 } from '@wordpress/components';
-import { store as coreStore } from '@wordpress/core-data';
+import { useEntityRecord } from '@wordpress/core-data';
 import { decodeEntities } from '@wordpress/html-entities';
 import { pencil } from '@wordpress/icons';
 
@@ -24,26 +24,11 @@ export default function SidebarNavigationScreenNavigationItem() {
 	const {
 		params: { postType, postId },
 	} = useNavigator();
-
-	const { post } = useSelect(
-		( select ) => {
-			const { getEntityRecord } = select( coreStore );
-
-			// The currently selected entity to display.
-			// Typically template or template part in the site editor.
-			return {
-				post:
-					postId && postType
-						? getEntityRecord( 'postType', postType, postId )
-						: null,
-			};
-		},
-		[ postType, postId ]
-	);
+	const { record } = useEntityRecord( 'postType', postType, postId );
 
 	return (
 		<SidebarNavigationScreen
-			title={ post ? decodeEntities( post?.title?.rendered ) : null }
+			title={ record ? decodeEntities( record?.title?.rendered ) : null }
 			actions={
 				<SidebarButton
 					onClick={ () => setCanvasMode( 'edit' ) }
@@ -53,16 +38,16 @@ export default function SidebarNavigationScreenNavigationItem() {
 			}
 			content={
 				<>
-					{ post?.link ? (
+					{ record?.link ? (
 						<ExternalLink
 							className="edit-site-sidebar-navigation-screen__page-link"
-							href={ post.link }
+							href={ record.link }
 						>
-							{ post.link }
+							{ record.link }
 						</ExternalLink>
 					) : null }
-					{ post
-						? decodeEntities( post?.description?.rendered )
+					{ record
+						? decodeEntities( record?.description?.rendered )
 						: null }
 				</>
 			}

diff --git a/packages/edit-site/src/components/sidebar-navigation-screen-template/index.js b/packages/edit-site/src/components/sidebar-navigation-screen-template/index.js
@@ -4,6 +4,7 @@
 import { __ } from '@wordpress/i18n';
 import { useDispatch } from '@wordpress/data';
 import { pencil } from '@wordpress/icons';
+import { __experimentalUseNavigator as useNavigator } from '@wordpress/components';
 
 /**
  * Internal dependencies
@@ -15,8 +16,13 @@ import { store as editSiteStore } from '../../store';
 import SidebarButton from '../sidebar-button';
 
 export default function SidebarNavigationScreenTemplate() {
+	const { params } = useNavigator();
+	const { postType, postId } = params;
 	const { setCanvasMode } = unlock( useDispatch( editSiteStore ) );
-	const { getDescription, getTitle, record } = useEditedEntityRecord();
+	const { getDescription, getTitle, record } = useEditedEntityRecord(
+		postType,
+		postId
+	);
 	let description = getDescription();
 	if ( ! description && record.is_custom ) {
 		description = __(

diff --git a/packages/edit-site/src/components/sync-state-with-url/use-sync-canvas-mode-with-url.js b/packages/edit-site/src/components/sync-state-with-url/use-sync-canvas-mode-with-url.js
@@ -20,22 +20,52 @@ export default function useSyncCanvasModeWithURL() {
 	);
 	const { setCanvasMode } = unlock( useDispatch( editSiteStore ) );
 	const currentCanvasMode = useRef( canvasMode );
-	const { canvas: canvasInUrl = 'view' } = params;
+	const { canvas: canvasInUrl } = params;
 	const currentCanvasInUrl = useRef( canvasInUrl );
+	const currentUrlParams = useRef( params );
+	useEffect( () => {
+		currentUrlParams.current = params;
+	}, [ params ] );
+
 	useEffect( () => {
 		currentCanvasMode.current = canvasMode;
-		if ( currentCanvasMode !== currentCanvasInUrl ) {
+		if ( canvasMode === 'init' ) {
+			return;
+		}
+
+		if (
+			canvasMode === 'edit' &&
+			currentCanvasInUrl.current !== canvasMode
+		) {
+			history.push( {
+				...currentUrlParams.current,
+				canvas: 'edit',
+			} );
+		}
+
+		if (
+			canvasMode === 'view' &&
+			currentCanvasInUrl.current !== undefined
+		) {
 			history.push( {
-				...params,
-				canvas: canvasMode,
+				...currentUrlParams.current,
+				canvas: undefined,
 			} );
 		}
-	}, [ canvasMode ] );
+	}, [ canvasMode, history ] );
 
 	useEffect( () => {
 		currentCanvasInUrl.current = canvasInUrl;
-		if ( canvasInUrl !== currentCanvasMode.current ) {
-			setCanvasMode( canvasInUrl );
+		if (
+			canvasInUrl === undefined &&
+			currentCanvasMode.current !== 'view'
+		) {
+			setCanvasMode( 'view' );
+		} else if (
+			canvasInUrl === 'edit' &&
+			currentCanvasMode.current !== 'edit'
+		) {
+			setCanvasMode( 'edit' );
 		}
-	}, [ canvasInUrl ] );
+	}, [ canvasInUrl, setCanvasMode ] );
 }
diff --git a/packages/edit-site/src/components/sync-state-with-url/use-sync-path-with-url.js b/packages/edit-site/src/components/sync-state-with-url/use-sync-path-with-url.js
@@ -41,8 +41,16 @@ export default function useSyncPathWithURL() {
 	} = useNavigator();
 	const currentUrlParams = useRef( urlParams );
 	const currentPath = useRef( navigatorLocation.path );
+	const isMounting = useRef( true );
 
 	useEffect( () => {
+		// The navigatorParams are only initially filled properly when the
+		// navigator screens mount. so we ignore the first synchronisation.
+		if ( isMounting.current ) {
+			isMounting.current = false;
+			return;
+		}
+
 		function updateUrlParams( newUrlParams ) {
 			if (
 				Object.entries( newUrlParams ).every( ( [ key, value ] ) => {
@@ -65,17 +73,14 @@ export default function useSyncPathWithURL() {
 				postId: navigatorParams?.postId,
 				path: undefined,
 			} );
-		} else if ( navigatorParams?.postType && ! navigatorParams?.postId ) {
-			updateUrlParams( {
-				postType: navigatorParams?.postType,
-				path: navigatorLocation.path,
-				postId: undefined,
-			} );
 		} else {
 			updateUrlParams( {
 				postType: undefined,
 				postId: undefined,
-				path: navigatorLocation.path,
+				path:
+					navigatorLocation.path === '/'
+						? undefined
+						: navigatorLocation.path,
 			} );
 		}
 	}, [ navigatorLocation?.path, navigatorParams, history ] );

diff --git a/packages/edit-site/src/components/template-details/index.js b/packages/edit-site/src/components/template-details/index.js
@@ -32,9 +32,6 @@ export default function TemplateDetails( { template, onClose } ) {
 
 	// TODO: We should update this to filter by template part's areas as well.
 	const browseAllLinkProps = useLink( {
-		canvas: 'view',
-		postType: template.type,
-		postId: undefined,
 		path: '/' + template.type + '/all',
 	} );
 

diff --git a/packages/edit-site/src/components/use-edited-entity-record/index.js b/packages/edit-site/src/components/use-edited-entity-record/index.js
@@ -11,25 +11,33 @@ import { decodeEntities } from '@wordpress/html-entities';
  */
 import { store as editSiteStore } from '../../store';
 
-export default function useEditedEntityRecord() {
-	const { record, title, description, isLoaded } = useSelect( ( select ) => {
-		const { getEditedPostType, getEditedPostId } = select( editSiteStore );
-		const { getEditedEntityRecord } = select( coreStore );
-		const { __experimentalGetTemplateInfo: getTemplateInfo } =
-			select( editorStore );
-		const postType = getEditedPostType();
-		const postId = getEditedPostId();
-		const _record = getEditedEntityRecord( 'postType', postType, postId );
-		const _isLoaded = !! postId;
-		const templateInfo = getTemplateInfo( _record );
+export default function useEditedEntityRecord( postType, postId ) {
+	const { record, title, description, isLoaded } = useSelect(
+		( select ) => {
+			const { getEditedPostType, getEditedPostId } =
+				select( editSiteStore );
+			const { getEditedEntityRecord } = select( coreStore );
+			const { __experimentalGetTemplateInfo: getTemplateInfo } =
+				select( editorStore );
+			const usedPostType = postType ?? getEditedPostType();
+			const usedPostId = postId ?? getEditedPostId();
+			const _record = getEditedEntityRecord(
+				'postType',
+				usedPostType,
+				usedPostId
+			);
+			const _isLoaded = !! usedPostId;
+			const templateInfo = getTemplateInfo( _record );
 
-		return {
-			record: _record,
-			title: templateInfo.title,
-			description: templateInfo.description,
-			isLoaded: _isLoaded,
-		};
-	}, [] );
+			return {
+				record: _record,
+				title: templateInfo.title,
+				description: templateInfo.description,
+				isLoaded: _isLoaded,
+			};
+		},
+		[ postType, postId ]
+	);
 
 	return {
 		isLoaded,

diff --git a/packages/edit-site/src/utils/history.js b/packages/edit-site/src/utils/history.js
@@ -6,27 +6,31 @@ import { createBrowserHistory } from 'history';
 /**
  * WordPress dependencies
  */
-import { addQueryArgs } from '@wordpress/url';
+import { addQueryArgs, getQueryArgs, removeQueryArgs } from '@wordpress/url';
 
 const history = createBrowserHistory();
 
 const originalHistoryPush = history.push;
 const originalHistoryReplace = history.replace;
 
 function push( params, state ) {
-	return originalHistoryPush.call(
-		history,
-		addQueryArgs( window.location.href, params ),
-		state
+	const currentArgs = getQueryArgs( window.location.href );
+	const currentUrlWithoutArgs = removeQueryArgs(
+		window.location.href,
+		...Object.keys( currentArgs )
 	);
+	const newUrl = addQueryArgs( currentUrlWithoutArgs, params );
+	return originalHistoryPush.call( history, newUrl, state );
 }
 
 function replace( params, state ) {
-	return originalHistoryReplace.call(
-		history,
-		addQueryArgs( window.location.href, params ),
-		state
+	const currentArgs = getQueryArgs( window.location.href );
+	const currentUrlWithoutArgs = removeQueryArgs(
+		window.location.href,
+		...Object.keys( currentArgs )
 	);
+	const newUrl = addQueryArgs( currentUrlWithoutArgs, params );
+	return originalHistoryReplace.call( history, newUrl, state );
 }
 
 history.push = push;

diff --git a/test/e2e/specs/site-editor/browser-history.spec.js b/test/e2e/specs/site-editor/browser-history.spec.js
@@ -0,0 +1,60 @@
+/**
+ * WordPress dependencies
+ */
+const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' );
+
+test.describe( 'Site editor browser history', () => {
+	test.beforeAll( async ( { requestUtils } ) => {
+		await requestUtils.activateTheme( 'emptytheme' );
+	} );
+
+	test.afterAll( async ( { requestUtils } ) => {
+		await requestUtils.activateTheme( 'twentytwentyone' );
+	} );
+
+	test( 'Back button works properly', async ( { admin, page } ) => {
+		await admin.visitAdminPage( 'index.php' );
+		await admin.visitSiteEditor();
+		await expect( page ).toHaveURL( '/wp-admin/site-editor.php' );
+
+		// Navigate to a single template
+		await page.click( 'role=button[name="Templates"]' );
+		await page.click( 'role=button[name="Index"]' );
+		await expect( page ).toHaveURL(
+			'/wp-admin/site-editor.php?postType=wp_template&postId=emptytheme%2F%2Findex'
+		);
+
+		// Navigate back to the template list
+		await page.goBack();
+		await expect( page ).toHaveURL(
+			'/wp-admin/site-editor.php?path=%2Fwp_template'
+		);
+
+		// Navigate back to the dashboard
+		await page.goBack();
+		await page.goBack();
+		await expect( page ).toHaveURL( '/wp-admin/index.php' );
+	} );
+
+	test( 'Opens the template list from the template details view', async ( {
+		admin,
+		page,
+	} ) => {
+		await admin.visitSiteEditor( {
+			postType: 'wp_template',
+			postId: 'emptytheme//index',
+			canvas: 'edit',
+		} );
+
+		// Navigate to the template list
+		await page.click( 'role=button[name="Show template details"]' );
+		await page.click( 'role=link[name="Manage all templates"]' );
+
+		await expect( page ).toHaveURL(
+			'/wp-admin/site-editor.php?path=%2Fwp_template%2Fall'
+		);
+
+		const title = page.getByRole( 'heading', { level: 1 } );
+		await expect( title ).toHaveText( 'Templates' );
+	} );
+} );