From e9d22773145575ed806d9a8eaa0346569d81b27d Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 24 Jul 2023 15:13:41 +1000 Subject: [PATCH 1/4] 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! --- 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 | 132 ++++++++++-------- .../use-global-styles-revisions.js | 113 ++++++++------- .../user-global-styles-revisions.spec.js | 13 ++ 6 files changed, 153 insertions(+), 111 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 b21c14418cbee..d58415c8cd836 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'; @@ -89,6 +90,7 @@ function ScreenRevisions() { const isLoadButtonEnabled = !! globalStylesRevision?.id && ! areGlobalStyleConfigsEqual( globalStylesRevision, userConfig ); + const shouldShowRevisions = ! isLoading && revisions.length; return ( <> @@ -101,68 +103,80 @@ function ScreenRevisions() { { isLoading && ( ) } - { ! isLoading && ( - - ) } -
- - { isLoadButtonEnabled && ( - - - - ) } -
- { isLoadingRevisionWithUnsavedChanges && ( - restoreRevision( globalStylesRevision ) } - onCancel={ () => - setIsLoadingRevisionWithUnsavedChanges( false ) - } - > - <> -

- { __( + { shouldShowRevisions ? ( + <> + +
+ + { isLoadButtonEnabled && ( + + + + ) } +
+ { isLoadingRevisionWithUnsavedChanges && ( + -

- { __( - 'Do you want to replace your unsaved changes in the editor?' + isOpen={ isLoadingRevisionWithUnsavedChanges } + confirmButtonText={ __( + ' Discard unsaved changes' ) } -

- - + onConfirm={ () => + restoreRevision( globalStylesRevision ) + } + onCancel={ () => + setIsLoadingRevisionWithUnsavedChanges( false ) + } + > + <> +

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

+

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

+ + + ) } + + ) : ( + + { __( 'There are currently no style revisions.' ) } + ) } ); 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 5aee31f1ff99a..bc82a5f2b1293 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,31 +72,33 @@ 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, - behaviors: userConfig?.behaviors, - 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, + behaviors: userConfig?.behaviors, + author: { + name: currentUser?.name, + avatar_urls: currentUser?.avatar_urls, + }, + modified: new Date(), + }; - _modifiedRevisions.unshift( unsavedRevision ); + _modifiedRevisions.unshift( unsavedRevision ); + } } return { @@ -98,5 +106,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..41c6a0141cb89 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 @@ -27,6 +27,19 @@ test.describe( 'Global styles revisions', () => { await editor.canvas.click( 'body' ); } ); + test( 'should display no revisions message if landing via command center', async ( { + page, + } ) => { + 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( 'There are currently no style revisions.' ); + } ); + test( 'should display revisions UI when there is more than 1 revision', async ( { page, editor, From 104b83c7404b58e8fbbf467c35d721a024229e94 Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 24 Jul 2023 16:06:21 +1000 Subject: [PATCH 2/4] Updated unit tests to reflect resolver logic changes --- .../test/use-global-styles-revisions.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) 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, From 2ddedb2a80bf8e33366b0626846f1435e95af8ef Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 24 Jul 2023 16:12:02 +1000 Subject: [PATCH 3/4] Use existing string --- .../src/components/global-styles/screen-revisions/index.js | 6 +++++- .../specs/site-editor/user-global-styles-revisions.spec.js | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) 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 d58415c8cd836..aa8bb1f377f29 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 @@ -175,7 +175,11 @@ function ScreenRevisions() { ) : ( - { __( 'There are currently no style 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.' ) + } ) } 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 41c6a0141cb89..45a51d1f8203c 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 @@ -37,7 +37,7 @@ test.describe( 'Global styles revisions', () => { .click(); await expect( page.getByTestId( 'global-styles-no-revisions' ) - ).toHaveText( 'There are currently no style revisions.' ); + ).toHaveText( 'No results found.' ); } ); test( 'should display revisions UI when there is more than 1 revision', async ( { From b16b0343d10b5bf74feee9aeea4c80754ff34a8e Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 24 Jul 2023 16:26:16 +1000 Subject: [PATCH 4/4] Only open edit view when testing the revisions panel itself --- .../site-editor/user-global-styles-revisions.spec.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 45a51d1f8203c..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,14 +22,16 @@ 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 @@ -45,6 +47,7 @@ test.describe( 'Global styles revisions', () => { editor, userGlobalStylesRevisions, } ) => { + await editor.canvas.click( 'body' ); const currentRevisions = await userGlobalStylesRevisions.getGlobalStylesRevisions(); await userGlobalStylesRevisions.openStylesPanel(); @@ -91,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