From b24d47c664447605b1e8217dc4316136f57c00da Mon Sep 17 00:00:00 2001 From: Ramon Date: Mon, 24 Jul 2023 16:59:29 +1000 Subject: [PATCH] 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 --- docs/reference-guides/data/data-core.md | 2 +- packages/core-data/README.md | 2 +- packages/core-data/src/selectors.ts | 2 +- .../global-styles/screen-revisions/index.js | 134 ++++++++++-------- .../test/use-global-styles-revisions.js | 15 +- .../use-global-styles-revisions.js | 111 ++++++++------- .../user-global-styles-revisions.spec.js | 21 ++- 7 files changed, 175 insertions(+), 112 deletions(-) diff --git a/docs/reference-guides/data/data-core.md b/docs/reference-guides/data/data-core.md index a66c0991e3d27..f2bc3374f9e72 100644 --- 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 index 63e6e28db08d5..c778b724149ef 100644 --- 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 index 142d24a9d2b8d..377134ab7c9a3 100644 --- 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 index c6920f3d63c24..b3f6c5cf8c500 100644 --- 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 && ( ) } - { ! isLoading && ( - - ) } -
- - { isLoadButtonEnabled && ( - - + + ) } +
+ { isLoadingRevisionWithUnsavedChanges && ( + + restoreRevision( globalStylesRevision ) + } + onCancel={ () => + setIsLoadingRevisionWithUnsavedChanges( false ) } - onClick={ () => { - if ( hasUnsavedChanges ) { - setIsLoadingRevisionWithUnsavedChanges( - true - ); - } else { - restoreRevision( globalStylesRevision ); - } - } } > - { __( 'Apply' ) } - - - ) } - - { isLoadingRevisionWithUnsavedChanges && ( - +

+ { __( + 'Loading this revision will discard all unsaved changes.' + ) } +

+

+ { __( + 'Do you want to replace your unsaved changes in the editor?' + ) } +

+ + ) } - isOpen={ isLoadingRevisionWithUnsavedChanges } - confirmButtonText={ __( ' Discard unsaved changes' ) } - onConfirm={ () => restoreRevision( globalStylesRevision ) } - onCancel={ () => - setIsLoadingRevisionWithUnsavedChanges( false ) + + ) : ( + + { + // 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.' ) } - > - <> -

- { __( - 'Loading this revision will discard all unsaved changes.' - ) } -

-

- { __( - 'Do you want to replace your unsaved changes in the editor?' - ) } -

- - + ) } ); diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/test/use-global-styles-revisions.js b/packages/edit-site/src/components/global-styles/screen-revisions/test/use-global-styles-revisions.js index 0b7d086c1120f..8f618a4897edc 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/test/use-global-styles-revisions.js +++ b/packages/edit-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/packages/edit-site/src/components/global-styles/screen-revisions/use-global-styles-revisions.js b/packages/edit-site/src/components/global-styles/screen-revisions/use-global-styles-revisions.js index ce3123e3fd028..55b6117325a9b 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/use-global-styles-revisions.js +++ b/packages/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,30 +72,32 @@ 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 { @@ -97,5 +105,12 @@ export default function useGlobalStylesRevisions() { 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 index 4ab61111df419..cb90ebe5edf25 100644 --- 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,9 +22,24 @@ 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 ( { @@ -32,6 +47,7 @@ test.describe( 'Global styles revisions', () => { 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