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 style revisions: show change summary on selected item #56577

Merged
merged 21 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
707b75e
Moving date format setting call into the comoponent
ramonjd Nov 20, 2023
cab578a
Testing a message to indicate that the revisions state is the same as…
ramonjd Nov 27, 2023
6e0bfca
Working on change list, adding translations. WIP
ramonjd Nov 27, 2023
b29086c
Adding more translations.
ramonjd Nov 28, 2023
78ee1c5
Revert button-in-item for another day
ramonjd Nov 30, 2023
a31cd2a
Removing shuffle function and fixing up block spacing translation
ramonjd Nov 30, 2023
d27d6c0
Using the revision has the Map key. This allows us to cache the revis…
ramonjd Dec 1, 2023
087084d
Used WeakMap in favour of Map for garbage collection, if it helps at all
ramonjd Dec 3, 2023
dcca429
Remove hasMore var - unneeded because it's only used once
ramonjd Dec 3, 2023
23a2615
Tidying up - remove .map loop
ramonjd Dec 3, 2023
803c4d9
getGlobalStylesChanges was doing nothing! Removed.
ramonjd Dec 3, 2023
0d2d425
Using revision + previousRevision combo for cache key to ensure that …
ramonjd Dec 6, 2023
fe76b98
Moving maxResults decisions to consuming component. getRevisionChange…
ramonjd Dec 6, 2023
c070f9f
Move get blockNames to main component
ramonjd Dec 6, 2023
776094e
Have to use map because WeakMap wants the same reference as the objec…
ramonjd Dec 6, 2023
f53ce53
Remove the trailing comma on truncated results
ramonjd Dec 6, 2023
d5dd7e7
Test commit: listing changes, showing `and n more`
ramonjd Dec 7, 2023
ec31842
Test commit: grouping changes using tuples
ramonjd Dec 7, 2023
fde41c8
Reverting back to comma-separate list of changes
ramonjd Dec 8, 2023
559cd3e
Swapping order of author name and changes block
ramonjd Dec 11, 2023
25b8a15
Don't live in the past, man
ramonjd Dec 11, 2023
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
/**
* WordPress dependencies
*/
import { __, sprintf } from '@wordpress/i18n';

const globalStylesChangesCache = new Map();
const EMPTY_ARRAY = [];

const translationMap = {
caption: __( 'Caption' ),
link: __( 'Link' ),
button: __( 'Button' ),
heading: __( 'Heading' ),
'settings.color': __( 'Color settings' ),
'settings.typography': __( 'Typography settings' ),
'styles.color': __( 'Colors' ),
'styles.spacing': __( 'Spacing' ),
'styles.typography': __( 'Typography' ),
};

const isObject = ( obj ) => obj !== null && typeof obj === 'object';

/**
* Get the translation for a given global styles key.
* @param {string} key A key representing a path to a global style property or setting.
* @param {Record<string,string>} blockNames A key/value pair object of block names and their rendered titles.
* @return {string|undefined} A translated key or undefined if no translation exists.
*/
function getTranslation( key, blockNames ) {
if ( translationMap[ key ] ) {
return translationMap[ key ];
}

const keyArray = key.split( '.' );

if ( keyArray?.[ 0 ] === 'blocks' ) {
const blockName = blockNames[ keyArray[ 1 ] ];
return blockName
? sprintf(
// translators: %s: block name.
__( '%s block' ),
blockName
)
: keyArray[ 1 ];
}

if ( keyArray?.[ 0 ] === 'elements' ) {
return sprintf(
// translators: %s: element name, e.g., heading button, link, caption.
__( '%s element' ),
translationMap[ keyArray[ 1 ] ]
);
}

return undefined;
}

/**
* A deep comparison of two objects, optimized for comparing global styles.
* @param {Object} changedObject The changed object to compare.
* @param {Object} originalObject The original object to compare against.
* @param {string} parentPath A key/value pair object of block names and their rendered titles.
* @return {string[]} An array of paths whose values have changed.
*/
function deepCompare( changedObject, originalObject, parentPath = '' ) {
// We have two non-object values to compare.
if ( ! isObject( changedObject ) && ! isObject( originalObject ) ) {
/*
* Only return a path if the value has changed.
* And then only the path name up to 2 levels deep.
*/
return changedObject !== originalObject
Copy link
Member Author

Choose a reason for hiding this comment

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

So this is very specific to the UI's requirements.

We're just returning the path to a value that has changed between versions.

If we want to reuse this function later, we can return anything, e.g., an object:

{
    path: 'styles.typography.fontWeight',
    oldValue: 600,
    newValue: 300,
}

? parentPath.split( '.' ).slice( 0, 2 ).join( '.' )
: undefined;
}

// Enable comparison when an object doesn't have a corresponding property to compare.
changedObject = isObject( changedObject ) ? changedObject : {};
originalObject = isObject( originalObject ) ? originalObject : {};

const allKeys = new Set( [
...Object.keys( changedObject ),
...Object.keys( originalObject ),
] );

let diffs = [];
for ( const key of allKeys ) {
const path = parentPath ? parentPath + '.' + key : key;
const changedPath = deepCompare(
changedObject[ key ],
originalObject[ key ],
path
);
if ( changedPath ) {
diffs = diffs.concat( changedPath );
}
}
return diffs;
}

/**
* Get an array of translated summarized global styles changes.
* Results are cached using a Map() key of `JSON.stringify( { revision, previousRevision } )`.
*
* @param {Object} revision The changed object to compare.
* @param {Object} previousRevision The original object to compare against.
* @param {Record<string,string>} blockNames A key/value pair object of block names and their rendered titles.
* @return {string[]} An array of translated changes.
*/
export default function getRevisionChanges(
revision,
previousRevision,
blockNames
) {
const cacheKey = JSON.stringify( { revision, previousRevision } );
Copy link
Member Author

Choose a reason for hiding this comment

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

This might be an overkill, but it's the only way I think of to ensure that we can return the right cached results for a matching set of revision and previousRevision.


if ( globalStylesChangesCache.has( cacheKey ) ) {
return globalStylesChangesCache.get( cacheKey );
}

/*
* Compare the two revisions with normalized keys.
* The order of these keys determines the order in which
* they'll appear in the results.
*/
const changedValueTree = deepCompare(
{
styles: {
color: revision?.styles?.color,
typography: revision?.styles?.typography,
spacing: revision?.styles?.spacing,
},
blocks: revision?.styles?.blocks,
elements: revision?.styles?.elements,
settings: revision?.settings,
},
{
styles: {
color: previousRevision?.styles?.color,
typography: previousRevision?.styles?.typography,
spacing: previousRevision?.styles?.spacing,
},
blocks: previousRevision?.styles?.blocks,
elements: previousRevision?.styles?.elements,
settings: previousRevision?.settings,
}
);

if ( ! changedValueTree.length ) {
globalStylesChangesCache.set( cacheKey, EMPTY_ARRAY );
return EMPTY_ARRAY;
}

// Remove duplicate results.
const result = [ ...new Set( changedValueTree ) ]
/*
* Translate the keys.
* Remove duplicate or empty translations.
*/
.reduce( ( acc, curr ) => {
const translation = getTranslation( curr, blockNames );
if ( translation && ! acc.includes( translation ) ) {
acc.push( translation );
}
return acc;
}, [] );

globalStylesChangesCache.set( cacheKey, result );

return result;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
__experimentalUseNavigator as useNavigator,
__experimentalConfirmDialog as ConfirmDialog,
Spinner,
__experimentalSpacer as Spacer,
} from '@wordpress/components';
import { useSelect, useDispatch } from '@wordpress/data';
import { store as coreStore } from '@wordpress/core-data';
Expand Down Expand Up @@ -135,7 +134,8 @@ function ScreenRevisions() {
}
}, [ shouldSelectFirstItem, firstRevision ] );

// Only display load button if there is a revision to load and it is different from the current editor styles.
// Only display load button if there is a revision to load,
// and it is different from the current editor styles.
const isLoadButtonEnabled =
!! currentlySelectedRevisionId && ! selectedRevisionMatchesEditorStyles;
const shouldShowRevisions = ! isLoading && revisions.length;
Expand All @@ -156,7 +156,7 @@ function ScreenRevisions() {
{ isLoading && (
<Spinner className="edit-site-global-styles-screen-revisions__loading" />
) }
{ shouldShowRevisions ? (
{ shouldShowRevisions && (
<>
<Revisions
blocks={ blocks }
Expand All @@ -168,6 +168,7 @@ function ScreenRevisions() {
onChange={ selectRevision }
selectedRevisionId={ currentlySelectedRevisionId }
userRevisions={ revisions }
canApplyRevision={ isLoadButtonEnabled }
/>
{ isLoadButtonEnabled && (
<SidebarFixedBottom>
Expand Down Expand Up @@ -215,14 +216,6 @@ function ScreenRevisions() {
</ConfirmDialog>
) }
</>
) : (
<Spacer marginX={ 4 } data-testid="global-styles-no-revisions">
Copy link
Member Author

Choose a reason for hiding this comment

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

This was dead code because we never show the panel where there are no revision.

{
// 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.' )
}
</Spacer>
) }
</>
);
Expand Down
Loading
Loading