Global Styles: Close the revisions screen on the first back click

packages/global-styles-ui/CHANGELOG.md

@@ -4,6 +4,8 @@
 
 ### Bug Fixes
 
+-   Revisions: close the screen on the first click of the back arrow after a revision has been selected, instead of requiring a second click ([#81897](https://github.com/WordPress/gutenberg/pull/81897)).
+-   Revisions: close the screen after applying a revision, which stopped happening when the screen moved into this package ([#81897](https://github.com/WordPress/gutenberg/pull/81897)).
 -   Color palette panel: do not render the theme colors wrapper when the theme provides no colors, which left an empty gap above the Custom section ([#81894](https://github.com/WordPress/gutenberg/pull/81894)).
 
 ### Enhancements

packages/global-styles-ui/src/screen-header.tsx

@@ -5,6 +5,7 @@ import {
 	__experimentalHeading as Heading,
 	__experimentalView as View,
 	__experimentalText as WCText,
+	Button,
 	Navigator,
 } from '@wordpress/components';
 import { isRTL, __ } from '@wordpress/i18n';
@@ -19,6 +20,12 @@ const { StateControl, StateControlBadges } = unlock( blockEditorPrivateApis );
 interface ScreenHeaderProps {
 	title: string;
 	description?: string | React.ReactElement;
+	/**
+	 * Replaces the back button's default navigation. Without it the button
+	 * moves up one path segment, which is wrong for a screen whose path
+	 * carries a selection (for example `/revisions/12`) rather than a
+	 * sub-screen.
+	 */
 	onBack?: () => void;
 	viewportStates?: StateDefinition[];
 	pseudoStates?: StateDefinition[];
@@ -47,12 +54,24 @@ export function ScreenHeader( {
 				<Spacer marginBottom={ 0 } paddingX={ 4 } paddingY={ 3 }>
 					<VStack spacing={ 2 }>
 						<HStack spacing={ 2 } alignment="top">
-							<Navigator.BackButton
-								icon={ isRTL() ? chevronRight : chevronLeft }
-								size="small"
-								label={ __( 'Back' ) }
-								onClick={ onBack }
-							/>
+							{ onBack ? (
+								<Button
+									icon={
+										isRTL() ? chevronRight : chevronLeft
+									}
+									size="small"
+									label={ __( 'Back' ) }
+									onClick={ onBack }
+								/>
+							) : (
+								<Navigator.BackButton
+									icon={
+										isRTL() ? chevronRight : chevronLeft
+									}
+									size="small"
+									label={ __( 'Back' ) }
+								/>
+							) }
 							<Spacer>
 								<HStack justify="space-between" alignment="top">
 									<Heading

packages/global-styles-ui/src/screen-revisions/index.tsx

@@ -14,11 +14,7 @@ import Pagination from '../pagination';
 
 const PAGE_SIZE = 10;
 
-interface ScreenRevisionsProps {
-	onClose?: () => void;
-}
-
-function ScreenRevisions( { onClose }: ScreenRevisionsProps = {} ) {
+function ScreenRevisions() {
 	const { user: currentEditorGlobalStyles, onChange: setUserConfig } =
 		useContext( GlobalStylesContext );
 	const { params, goTo } = useNavigator();
@@ -55,16 +51,18 @@ function ScreenRevisions( { onClose }: ScreenRevisionsProps = {} ) {
 		currentEditorGlobalStyles
 	);
 
-	const onCloseRevisions = () => {
-		if ( onClose ) {
-			onClose();
-		}
+	// Both the back arrow and Apply leave the revisions screen. Selecting a
+	// revision appends its id to the path (`/revisions/12`), so the navigator's
+	// default back action would only strip the id and leave the user on the
+	// same screen. Go straight to the root screen instead.
+	const closeRevisions = () => {
+		goTo( '/', { isBack: true } );
 	};
 
 	const restoreRevision = ( revision: any ) => {
 		setUserConfig( revision );
 		setIsLoadingRevisionWithUnsavedChanges( false );
-		onCloseRevisions();
+		closeRevisions();
 	};
 
 	const handleRevisionSelect = ( revision: any ) => {
@@ -98,7 +96,7 @@ function ScreenRevisions( { onClose }: ScreenRevisionsProps = {} ) {
 				description={ __(
 					'Click on previously saved styles to preview them. To restore a selected version to the editor, hit "Apply." When you\'re ready, use the Save button to save your changes.'
 				) }
-				onBack={ onCloseRevisions }
+				onBack={ closeRevisions }
 			/>
 			{ ! hasRevisions && (
 				<Spinner className="global-styles-ui-screen-revisions__loading" />

packages/global-styles-ui/src/screen-revisions/revisions-buttons.tsx

@@ -240,7 +240,15 @@ function RevisionsButtons( {
 									size="compact"
 									variant="primary"
 									className="global-styles-ui-screen-revisions__apply-button"
-									onClick={ onApplyRevision }
+									onClick={ ( event: React.MouseEvent ) => {
+										// This button sits inside the option,
+										// whose own click handler selects the
+										// revision. Without this the selection
+										// would re-run and navigate back to
+										// the revision just applied.
+										event.stopPropagation();
+										onApplyRevision?.();
+									} }
 									aria-label={ __(
 										'Apply the selected revision to your site.'
 									) }

packages/global-styles-ui/src/test/screen-header.js

@@ -0,0 +1,52 @@
+import { fireEvent, render, screen } from '@testing-library/react';
+import { Navigator, useNavigator } from '@wordpress/components';
+import { ScreenHeader } from '../screen-header';
+
+function CurrentPath() {
+	const { location } = useNavigator();
+	return <span data-testid="path">{ location.path }</span>;
+}
+
+function renderAtRevision( headerProps ) {
+	return render(
+		<Navigator initialPath="/revisions/10">
+			<CurrentPath />
+			<Navigator.Screen path="/">
+				<ScreenHeader title="Styles" />
+			</Navigator.Screen>
+			<Navigator.Screen path="/revisions/:revisionId?">
+				<ScreenHeader title="Revisions (16)" { ...headerProps } />
+			</Navigator.Screen>
+		</Navigator>
+	);
+}
+
+describe( 'ScreenHeader back button', () => {
+	it( 'walks up one path segment by default, which leaves the user on the revisions screen', () => {
+		renderAtRevision();
+
+		fireEvent.click(
+			screen.getAllByRole( 'button', { name: 'Back' } )[ 0 ]
+		);
+
+		expect( screen.getByTestId( 'path' ) ).toHaveTextContent(
+			/^\/revisions$/
+		);
+	} );
+
+	it( 'uses onBack instead of the default navigation when one is given', () => {
+		const onBack = jest.fn();
+		renderAtRevision( { onBack } );
+
+		fireEvent.click(
+			screen.getAllByRole( 'button', { name: 'Back' } )[ 0 ]
+		);
+
+		expect( onBack ).toHaveBeenCalledTimes( 1 );
+		// The default parent walk must not run as well, otherwise the caller's
+		// destination lands on top of an intermediate location.
+		expect( screen.getByTestId( 'path' ) ).toHaveTextContent(
+			/^\/revisions\/10$/
+		);
+	} );
+} );

test/e2e/specs/site-editor/user-global-styles-revisions.spec.js

@@ -204,6 +204,54 @@ test.describe( 'Style Revisions', () => {
 		).toBeHidden();
 	} );
 
+	test( 'should close the revisions panel with a single click of Back after selecting a revision', async ( {
+		page,
+		editor,
+		userGlobalStylesRevisions,
+	} ) => {
+		await editor.canvas.locator( 'body' ).click();
+		await userGlobalStylesRevisions.openStylesPanel();
+		await page.getByRole( 'button', { name: 'Revisions' } ).click();
+
+		// Selecting a revision puts its id in the path (`/revisions/12`).
+		await page
+			.getByRole( 'option', { name: /^Changes saved by / } )
+			.last()
+			.click();
+
+		await page.getByRole( 'button', { name: 'Back', exact: true } ).click();
+
+		await expect(
+			page.getByLabel( 'Global styles revisions list' )
+		).toBeHidden();
+	} );
+
+	test( 'should close the revisions panel after applying a revision', async ( {
+		page,
+		editor,
+		userGlobalStylesRevisions,
+	} ) => {
+		await editor.canvas.locator( 'body' ).click();
+		await userGlobalStylesRevisions.openStylesPanel();
+		await page.getByRole( 'button', { name: 'Revisions' } ).click();
+
+		await page
+			.getByLabel( 'Global styles revisions list' )
+			.getByRole( 'option' )
+			.last()
+			.click();
+
+		await page
+			.getByRole( 'button', {
+				name: 'Apply the selected revision to your site.',
+			} )
+			.click();
+
+		await expect(
+			page.getByLabel( 'Global styles revisions list' )
+		).toBeHidden();
+	} );
+
 	test( 'should close revisions panel and leave style book open if activated', async ( {
 		page,
 		editor,