Global styles revisions: display text if no revisions are found (#52865)

* If somehow a user lands on the revisions panel when there are no revisions, show some helpful text rather than a loading spinner. Also, add an E2E test! * Updated unit tests to reflect resolver logic changes * Use existing string * Only open edit view when testing the revisions panel itself
WordPress · Jul 25, 2023 · b24d47c · b24d47c
1 parent 1a67dbb
commit b24d47c
Show file tree

Hide file tree

Showing 7 changed files with 175 additions and 112 deletions.
diff --git a/docs/reference-guides/data/data-core.md b/docs/reference-guides/data/data-core.md
@@ -136,7 +136,7 @@ _Parameters_
 
 _Returns_
 
--   `Object | null`: The current global styles.
+-   `Array< object > | null`: The current global styles.
 
 ### getCurrentUser
 

diff --git a/packages/core-data/README.md b/packages/core-data/README.md
@@ -313,7 +313,7 @@ _Parameters_
 
 _Returns_
 
--   `Object | null`: The current global styles.
+-   `Array< object > | null`: The current global styles.
 
 ### getCurrentUser
 

diff --git a/packages/core-data/src/selectors.ts b/packages/core-data/src/selectors.ts
@@ -1266,7 +1266,7 @@ export function getBlockPatternCategories( state: State ): Array< any > {
  */
 export function getCurrentThemeGlobalStylesRevisions(
 	state: State
-): Object | null {
+): Array< object > | null {
 	const currentGlobalStylesId =
 		__experimentalGetCurrentGlobalStylesId( state );
 

diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/index.js b/packages/edit-site/src/components/global-styles/screen-revisions/index.js
@@ -7,6 +7,7 @@ import {
 	__experimentalUseNavigator as useNavigator,
 	__experimentalConfirmDialog as ConfirmDialog,
 	Spinner,
+	__experimentalSpacer as Spacer,
 } from '@wordpress/components';
 import { useSelect, useDispatch } from '@wordpress/data';
 import { useContext, useState, useEffect } from '@wordpress/element';
@@ -87,6 +88,7 @@ function ScreenRevisions() {
 	const isLoadButtonEnabled =
 		!! globalStylesRevision?.id &&
 		! areGlobalStyleConfigsEqual( globalStylesRevision, userConfig );
+	const shouldShowRevisions = ! isLoading && revisions.length;
 
 	return (
 		<>
@@ -99,68 +101,84 @@ function ScreenRevisions() {
 			{ isLoading && (
 				<Spinner className="edit-site-global-styles-screen-revisions__loading" />
 			) }
-			{ ! isLoading && (
-				<Revisions
-					blocks={ blocks }
-					userConfig={ globalStylesRevision }
-					onClose={ onCloseRevisions }
-				/>
-			) }
-			<div className="edit-site-global-styles-screen-revisions">
-				<RevisionsButtons
-					onChange={ selectRevision }
-					selectedRevisionId={ selectedRevisionId }
-					userRevisions={ revisions }
-				/>
-				{ isLoadButtonEnabled && (
-					<SidebarFixedBottom>
-						<Button
-							variant="primary"
-							className="edit-site-global-styles-screen-revisions__button"
-							disabled={
-								! globalStylesRevision?.id ||
-								globalStylesRevision?.id === 'unsaved'
+			{ shouldShowRevisions ? (
+				<>
+					<Revisions
+						blocks={ blocks }
+						userConfig={ globalStylesRevision }
+						onClose={ onCloseRevisions }
+					/>
+					<div className="edit-site-global-styles-screen-revisions">
+						<RevisionsButtons
+							onChange={ selectRevision }
+							selectedRevisionId={ selectedRevisionId }
+							userRevisions={ revisions }
+						/>
+						{ isLoadButtonEnabled && (
+							<SidebarFixedBottom>
+								<Button
+									variant="primary"
+									className="edit-site-global-styles-screen-revisions__button"
+									disabled={
+										! globalStylesRevision?.id ||
+										globalStylesRevision?.id === 'unsaved'
+									}
+									onClick={ () => {
+										if ( hasUnsavedChanges ) {
+											setIsLoadingRevisionWithUnsavedChanges(
+												true
+											);
+										} else {
+											restoreRevision(
+												globalStylesRevision
+											);
+										}
+									} }
+								>
+									{ __( 'Apply' ) }
+								</Button>
+							</SidebarFixedBottom>
+						) }
+					</div>
+					{ isLoadingRevisionWithUnsavedChanges && (
+						<ConfirmDialog
+							title={ __(
+								'Loading this revision will discard all unsaved changes.'
+							) }
+							isOpen={ isLoadingRevisionWithUnsavedChanges }
+							confirmButtonText={ __(
+								' Discard unsaved changes'
+							) }
+							onConfirm={ () =>
+								restoreRevision( globalStylesRevision )
+							}
+							onCancel={ () =>
+								setIsLoadingRevisionWithUnsavedChanges( false )
 							}
-							onClick={ () => {
-								if ( hasUnsavedChanges ) {
-									setIsLoadingRevisionWithUnsavedChanges(
-										true
-									);
-								} else {
-									restoreRevision( globalStylesRevision );
-								}
-							} }
 						>
-							{ __( 'Apply' ) }
-						</Button>
-					</SidebarFixedBottom>
-				) }
-			</div>
-			{ isLoadingRevisionWithUnsavedChanges && (
-				<ConfirmDialog
-					title={ __(
-						'Loading this revision will discard all unsaved changes.'
+							<>
+								<h2>
+									{ __(
+										'Loading this revision will discard all unsaved changes.'
+									) }
+								</h2>
+								<p>
+									{ __(
+										'Do you want to replace your unsaved changes in the editor?'
+									) }
+								</p>
+							</>
+						</ConfirmDialog>
 					) }
-					isOpen={ isLoadingRevisionWithUnsavedChanges }
-					confirmButtonText={ __( ' Discard unsaved changes' ) }
-					onConfirm={ () => restoreRevision( globalStylesRevision ) }
-					onCancel={ () =>
-						setIsLoadingRevisionWithUnsavedChanges( false )
+				</>
+			) : (
+				<Spacer marginX={ 4 } data-testid="global-styles-no-revisions">
+					{
+						// Adding an existing translation here in case these changes are shipped to WordPress 6.3.
+						// Later we could update to something better, e.g., "There are currently no style revisions.".
+						__( 'No results found.' )
 					}
-				>
-					<>
-						<h2>
-							{ __(
-								'Loading this revision will discard all unsaved changes.'
-							) }
-						</h2>
-						<p>
-							{ __(
-								'Do you want to replace your unsaved changes in the editor?'
-							) }
-						</p>
-					</>
-				</ConfirmDialog>
+				</Spacer>
 			) }
 		</>
 	);

diff --git a/...it-site/src/components/global-styles/screen-revisions/test/use-global-styles-revisions.js b/...it-site/src/components/global-styles/screen-revisions/test/use-global-styles-revisions.js
@@ -49,6 +49,7 @@ describe( 'useGlobalStylesRevisions', () => {
 				styles: {},
 			},
 		],
+		isLoadingGlobalStylesRevisions: false,
 	};
 
 	it( 'returns loaded revisions with no unsaved changes', () => {
@@ -117,11 +118,23 @@ describe( 'useGlobalStylesRevisions', () => {
 		const { result } = renderHook( () => useGlobalStylesRevisions() );
 		const { revisions, isLoading, hasUnsavedChanges } = result.current;
 
-		expect( isLoading ).toBe( true );
+		expect( isLoading ).toBe( false );
 		expect( hasUnsavedChanges ).toBe( false );
 		expect( revisions ).toEqual( [] );
 	} );
 
+	it( 'returns loading status when resolving global revisions', () => {
+		useSelect.mockImplementation( () => ( {
+			...selectValue,
+			isLoadingGlobalStylesRevisions: true,
+		} ) );
+
+		const { result } = renderHook( () => useGlobalStylesRevisions() );
+		const { isLoading } = result.current;
+
+		expect( isLoading ).toBe( true );
+	} );
+
 	it( 'returns empty revisions when authors are not yet available', () => {
 		useSelect.mockImplementation( () => ( {
 			...selectValue,

diff --git a/...es/edit-site/src/components/global-styles/screen-revisions/use-global-styles-revisions.js b/...es/edit-site/src/components/global-styles/screen-revisions/use-global-styles-revisions.js
@@ -21,34 +21,40 @@ const EMPTY_ARRAY = [];
 const { GlobalStylesContext } = unlock( blockEditorPrivateApis );
 export default function useGlobalStylesRevisions() {
 	const { user: userConfig } = useContext( GlobalStylesContext );
-	const { authors, currentUser, isDirty, revisions } = useSelect(
-		( select ) => {
-			const {
-				__experimentalGetDirtyEntityRecords,
-				getCurrentUser,
-				getUsers,
-				getCurrentThemeGlobalStylesRevisions,
-			} = select( coreStore );
-			const dirtyEntityRecords = __experimentalGetDirtyEntityRecords();
-			const _currentUser = getCurrentUser();
-			const _isDirty = dirtyEntityRecords.length > 0;
-			const globalStylesRevisions =
-				getCurrentThemeGlobalStylesRevisions() || EMPTY_ARRAY;
-			const _authors =
-				getUsers( SITE_EDITOR_AUTHORS_QUERY ) || EMPTY_ARRAY;
+	const {
+		authors,
+		currentUser,
+		isDirty,
+		revisions,
+		isLoadingGlobalStylesRevisions,
+	} = useSelect( ( select ) => {
+		const {
+			__experimentalGetDirtyEntityRecords,
+			getCurrentUser,
+			getUsers,
+			getCurrentThemeGlobalStylesRevisions,
+			isResolving,
+		} = select( coreStore );
+		const dirtyEntityRecords = __experimentalGetDirtyEntityRecords();
+		const _currentUser = getCurrentUser();
+		const _isDirty = dirtyEntityRecords.length > 0;
+		const globalStylesRevisions =
+			getCurrentThemeGlobalStylesRevisions() || EMPTY_ARRAY;
+		const _authors = getUsers( SITE_EDITOR_AUTHORS_QUERY ) || EMPTY_ARRAY;
 
-			return {
-				authors: _authors,
-				currentUser: _currentUser,
-				isDirty: _isDirty,
-				revisions: globalStylesRevisions,
-			};
-		},
-		[]
-	);
+		return {
+			authors: _authors,
+			currentUser: _currentUser,
+			isDirty: _isDirty,
+			revisions: globalStylesRevisions,
+			isLoadingGlobalStylesRevisions: isResolving(
+				'getCurrentThemeGlobalStylesRevisions'
+			),
+		};
+	}, [] );
 	return useMemo( () => {
 		let _modifiedRevisions = [];
-		if ( ! authors.length || ! revisions.length ) {
+		if ( ! authors.length || isLoadingGlobalStylesRevisions ) {
 			return {
 				revisions: _modifiedRevisions,
 				hasUnsavedChanges: isDirty,
@@ -66,36 +72,45 @@ export default function useGlobalStylesRevisions() {
 			};
 		} );
 
-		// Flags the most current saved revision.
-		if ( _modifiedRevisions[ 0 ].id !== 'unsaved' ) {
-			_modifiedRevisions[ 0 ].isLatest = true;
-		}
+		if ( _modifiedRevisions.length ) {
+			// Flags the most current saved revision.
+			if ( _modifiedRevisions[ 0 ].id !== 'unsaved' ) {
+				_modifiedRevisions[ 0 ].isLatest = true;
+			}
 
-		// Adds an item for unsaved changes.
-		if (
-			isDirty &&
-			userConfig &&
-			Object.keys( userConfig ).length > 0 &&
-			currentUser
-		) {
-			const unsavedRevision = {
-				id: 'unsaved',
-				styles: userConfig?.styles,
-				settings: userConfig?.settings,
-				author: {
-					name: currentUser?.name,
-					avatar_urls: currentUser?.avatar_urls,
-				},
-				modified: new Date(),
-			};
+			// Adds an item for unsaved changes.
+			if (
+				isDirty &&
+				userConfig &&
+				Object.keys( userConfig ).length > 0 &&
+				currentUser
+			) {
+				const unsavedRevision = {
+					id: 'unsaved',
+					styles: userConfig?.styles,
+					settings: userConfig?.settings,
+					author: {
+						name: currentUser?.name,
+						avatar_urls: currentUser?.avatar_urls,
+					},
+					modified: new Date(),
+				};
 
-			_modifiedRevisions.unshift( unsavedRevision );
+				_modifiedRevisions.unshift( unsavedRevision );
+			}
 		}
 
 		return {
 			revisions: _modifiedRevisions,
 			hasUnsavedChanges: isDirty,
 			isLoading: false,
 		};
-	}, [ isDirty, revisions, currentUser, authors, userConfig ] );
+	}, [
+		isDirty,
+		revisions,
+		currentUser,
+		authors,
+		userConfig,
+		isLoadingGlobalStylesRevisions,
+	] );
 }
diff --git a/test/e2e/specs/site-editor/user-global-styles-revisions.spec.js b/test/e2e/specs/site-editor/user-global-styles-revisions.spec.js
@@ -22,16 +22,32 @@ test.describe( 'Global styles revisions', () => {
 		await requestUtils.activateTheme( 'twentytwentyone' );
 	} );
 
-	test.beforeEach( async ( { admin, editor } ) => {
+	test.beforeEach( async ( { admin } ) => {
 		await admin.visitSiteEditor();
-		await editor.canvas.click( 'body' );
+	} );
+
+	test( 'should display no revisions message if landing via command center', async ( {
+		page,
+	} ) => {
+		await page
+			.getByRole( 'button', { name: 'Open command palette' } )
+			.focus();
+		await page.keyboard.press( 'Meta+k' );
+		await page.keyboard.type( 'styles revisions' );
+		await page
+			.getByRole( 'option', { name: 'Open styles revisions' } )
+			.click();
+		await expect(
+			page.getByTestId( 'global-styles-no-revisions' )
+		).toHaveText( 'No results found.' );
 	} );
 
 	test( 'should display revisions UI when there is more than 1 revision', async ( {
 		page,
 		editor,
 		userGlobalStylesRevisions,
 	} ) => {
+		await editor.canvas.click( 'body' );
 		const currentRevisions =
 			await userGlobalStylesRevisions.getGlobalStylesRevisions();
 		await userGlobalStylesRevisions.openStylesPanel();
@@ -78,6 +94,7 @@ test.describe( 'Global styles revisions', () => {
 		editor,
 		userGlobalStylesRevisions,
 	} ) => {
+		await editor.canvas.click( 'body' );
 		await userGlobalStylesRevisions.openStylesPanel();
 		await page.getByRole( 'button', { name: 'Colors styles' } ).click();
 		await page