Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/reference-guides/data/data-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ _Parameters_

_Returns_

- `Object | null`: The current global styles.
- `Array< object > | null`: The current global styles.

### getCurrentUser

Expand Down
2 changes: 1 addition & 1 deletion packages/core-data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ _Parameters_

_Returns_

- `Object | null`: The current global styles.
- `Array< object > | null`: The current global styles.

### getCurrentUser

Expand Down
2 changes: 1 addition & 1 deletion packages/core-data/src/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -89,6 +90,7 @@ function ScreenRevisions() {
const isLoadButtonEnabled =
!! globalStylesRevision?.id &&
! areGlobalStyleConfigsEqual( globalStylesRevision, userConfig );
const shouldShowRevisions = ! isLoading && revisions.length;

return (
<>
Expand All @@ -101,68 +103,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>
) }
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ describe( 'useGlobalStylesRevisions', () => {
styles: {},
},
],
isLoadingGlobalStylesRevisions: false,
};

it( 'returns loaded revisions with no unsaved changes', () => {
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -66,37 +72,46 @@ 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 {
revisions: _modifiedRevisions,
hasUnsavedChanges: isDirty,
isLoading: false,
};
}, [ isDirty, revisions, currentUser, authors, userConfig ] );
}, [
isDirty,
revisions,
currentUser,
authors,
userConfig,
isLoadingGlobalStylesRevisions,
] );
}
21 changes: 19 additions & 2 deletions test/e2e/specs/site-editor/user-global-styles-revisions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on failure traces, the command doesn't open the Command Palette. You can use https://trace.playwright.dev/ to check the Playwright - https://trace.playwright.dev/.

The reasons could be that there is no focus on the editor's body or keyboard shortcuts must be corrected.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, interesting. Thanks for the tips. It's passing locally, so I'll try a few things and push them 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it helps, but it looks like the Open the command palette and navigate to the page create page test moves focus to the command palette button before pressing Meta+k, so something like that might make it more reliable? E.g.

await page
.getByRole( 'button', { name: 'Open command palette' } )
.focus();

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but it looks like the Open the command palette and navigate to the page create page test moves focus to the command palette button before pressing Meta+k, so something like that might make it more reliable?

Thanks @andrewserong

That was actually the first thing I tried (copy paste 😄 ) but it wasn't passing locally with that line.

Maybe I'll add it and YOLO it up to CI

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because that button only exists in view mode. I'm opening the canvas in edit mode. 👍🏻

Copy link
Member

@Mamaduka Mamaduka Jul 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just click the button if we're already locating it 😄 But as long as it works, I'm fine with it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just click the button if we're already locating it 😄 But as long as it works, I'm fine with it.

I ❤️ copy 🍝

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍝 goes well with e2e tests 😁

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();
Expand Down Expand Up @@ -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
Expand Down