From 707b75edf24236bb12c2e6ef2da4eb8662eeb99a Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 20 Nov 2023 13:39:51 +1100 Subject: [PATCH 01/21] Moving date format setting call into the comoponent rejigging getLabel function adding css var --- .../screen-revisions/get-revision-changes.js | 137 ++++++++++++++++++ .../global-styles/screen-revisions/index.js | 45 ++---- .../screen-revisions/revisions-buttons.js | 38 ++++- .../global-styles/screen-revisions/style.scss | 7 + 4 files changed, 193 insertions(+), 34 deletions(-) create mode 100644 packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js new file mode 100644 index 00000000000000..071894779478d1 --- /dev/null +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -0,0 +1,137 @@ +/** + * WordPress dependencies + */ +import { __ } from '@wordpress/i18n'; + +const translationMap = { + blocks: __( 'Block styles' ), + border: __( 'Border' ), + color: __( 'Colors' ), + elements: __( 'Elements' ), + link: __( 'Links' ), + layout: __( 'Layout' ), + spacing: __( 'Spacing' ), + typography: __( 'Typography' ), +}; + +/** + * Returns an array of translated strings describing high level global styles and settings. + * + * @param {Object} revision + * @param {Object} revision.settings Global styles settings. + * @param {Object} revision.styles Global styles. + * @return {string[] | []} An array of translated labels. + */ +export function getGlobalStylesChanges( { settings = {}, styles = {} } ) { + const changes = []; + if ( Object.keys( settings ).length > 0 ) { + changes.push( __( 'Global settings' ) ); + } + Object.keys( styles ).forEach( ( key ) => { + if ( translationMap[ key ] ) { + changes.push( translationMap[ key ] ); + } + } ); + + return changes; +} + +const shuffle = ( array ) => { + for ( let i = array.length - 1; i > 0; i-- ) { + // eslint-disable-next-line no-restricted-syntax + const j = Math.floor( Math.random() * ( i + 1 ) ); + [ array[ i ], array[ j ] ] = [ array[ j ], array[ i ] ]; + } + return array; +}; + +const cache = {}; // Should be a Set? + +export function getRevisionChanges( + revision, + previousRevision, + maxResults = 10 +) { + if ( cache[ revision.id ] ) { + return cache[ revision.id ]; + } +console.log( 'revision, previousRevision', revision, previousRevision ); + const changedValueTree = deepCompare( + { + blocks: revision?.styles?.blocks, + elements: revision?.styles?.elements, + color: revision?.styles?.color, + typography: revision?.styles?.typography, + dimensions: revision?.styles?.dimensions, + spacing: revision?.styles?.spacing, + border: revision?.styles?.border, + settingsColor: revision?.settings?.color, + }, + { + blocks: previousRevision?.styles?.blocks, + elements: previousRevision?.styles?.elements, + color: previousRevision?.styles?.color, + typography: previousRevision?.styles?.typography, + dimensions: previousRevision?.styles?.dimensions, + spacing: previousRevision?.styles?.spacing, + border: previousRevision?.styles?.border, + settingsColor: previousRevision?.settings?.color, + } + ); + + + console.log( 'flattenedChanges', changedValueTree); + const shuffled = shuffle( + changedValueTree.filter( ( { hasChanged, path } ) => hasChanged ) + ); + console.log( 'shuffled', shuffled ); + /* cache[ revision.id ] = shuffled + .slice( 0, maxResults ) + .map( ( { path } ) => path ) + .join( ', ' ); + + return cache[ revision.id ];*/ +} + +function isObject( obj ) { + return obj !== null && typeof obj === 'object'; +} + +function deepCompare( revisionValue, configValue, depth = 0, parentPath = '' ) { + if ( ! isObject( revisionValue ) && ! isObject( configValue ) ) { + return [ + { + path: parentPath, + revisionValue, + configValue, + hasChanged: revisionValue !== configValue, + }, + ]; + } + + revisionValue = isObject( revisionValue ) ? revisionValue : {}; + configValue = isObject( configValue ) ? configValue : {}; + + const keys1 = Object.keys( revisionValue ); + const keys2 = Object.keys( configValue ); + const allKeys = new Set( [ ...keys1, ...keys2 ] ); + + let diffs = []; + for ( const key of allKeys ) { + const path = parentPath ? parentPath + '.' + key : key; + const subDiffs = deepCompare( + revisionValue[ key ], + configValue[ key ], + depth + 1, + path + ); + diffs = diffs.concat( subDiffs ); + } +/* diffs = diffs.filter( + ( diff ) => + diff.path.includes( 'behaviors' ) || + diff.path.includes( 'settings' ) || + diff.path.includes( 'styles' ) + );*/ + return diffs; +} 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 90bf68e579cb7c..a56a5ea28dee1f 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 @@ -3,7 +3,6 @@ */ import { __, sprintf } from '@wordpress/i18n'; import { - Button, __experimentalUseNavigator as useNavigator, __experimentalConfirmDialog as ConfirmDialog, Spinner, @@ -23,7 +22,6 @@ import { import ScreenHeader from '../header'; import { unlock } from '../../../lock-unlock'; import Revisions from '../../revisions'; -import SidebarFixedBottom from '../../sidebar-edit-mode/sidebar-fixed-bottom'; import { store as editSiteStore } from '../../../store'; import useGlobalStylesRevisions from './use-global-styles-revisions'; import RevisionsButtons from './revisions-buttons'; @@ -135,7 +133,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; @@ -168,35 +167,19 @@ function ScreenRevisions() { onChange={ selectRevision } selectedRevisionId={ currentlySelectedRevisionId } userRevisions={ revisions } + canApplyRevision={ isLoadButtonEnabled } + onSelect={ () => { + if ( hasUnsavedChanges ) { + setIsLoadingRevisionWithUnsavedChanges( + true + ); + } else { + restoreRevision( + currentlySelectedRevision + ); + } + } } /> - { isLoadButtonEnabled && ( - - - - ) } { isLoadingRevisionWithUnsavedChanges && ( { const { getCurrentTheme, getCurrentUser } = select( coreStore ); const currentTheme = getCurrentTheme(); @@ -78,7 +89,7 @@ function RevisionsButtons( { userRevisions, selectedRevisionId, onChange } ) { }; }, [] ); const dateNowInMs = getDate().getTime(); - const { date: dateFormat, datetimeAbbreviated } = getSettings().formats; + const { datetimeAbbreviated } = getSettings().formats; return (
    DAY_IN_MILLISECONDS - ? dateI18n( dateFormat, modifiedDate ) + ? dateI18n( datetimeAbbreviated, modifiedDate ) : humanTimeDiff( modified ); const revisionLabel = getRevisionLabel( id, @@ -160,6 +171,27 @@ function RevisionsButtons( { userRevisions, selectedRevisionId, onChange } ) { ) } + { isSelected && canApplyRevision && ( + <> +

    + { getRevisionChanges( + revision, + index < userRevisions.length + ? userRevisions[ index + 1 ] + : {} + ) } +

    + + + ) } ); } ) } diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss index 6598fcb5ce1c74..810ebaa0f4132d 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss +++ b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss @@ -89,6 +89,13 @@ .edit-site-global-styles-screen-revisions__button { justify-content: center; width: 100%; + margin: 0 0 10px 10px; +} + +.edit-site-global-styles-screen-revision__button { + justify-content: center; + width: 90%; + margin: 0 0 $grid-unit-10 $grid-unit-20; } .edit-site-global-styles-screen-revisions__description { From cab578ac3f65accb0e56b3a5b43e4c5424b5db62 Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 27 Nov 2023 12:27:17 +1100 Subject: [PATCH 02/21] Testing a message to indicate that the revisions state is the same as the editor state. --- .../screen-revisions/revisions-buttons.js | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js index b4aa1b3f5bfb7c..de3fd0fe701b1f 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js @@ -171,7 +171,7 @@ function RevisionsButtons( { ) } - { isSelected && canApplyRevision && ( + { isSelected && ( <>

    { getRevisionChanges( @@ -181,15 +181,22 @@ function RevisionsButtons( { : {} ) }

    - + { canApplyRevision ? ( + + ) : ( +

    + The revision is the same as the saved + state. +

    + ) } ) } From 6e0bfcaf5e8f9e0127b903030260624d3434aa8b Mon Sep 17 00:00:00 2001 From: ramon Date: Tue, 28 Nov 2023 10:15:40 +1100 Subject: [PATCH 03/21] Working on change list, adding translations. WIP --- .../screen-revisions/get-revision-changes.js | 157 ++++++++++++------ .../screen-revisions/revisions-buttons.js | 37 +++-- .../global-styles/screen-revisions/style.scss | 16 ++ 3 files changed, 145 insertions(+), 65 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index 071894779478d1..b83d86ab54f5c8 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -1,17 +1,36 @@ /** * WordPress dependencies */ -import { __ } from '@wordpress/i18n'; +import { __, sprintf } from '@wordpress/i18n'; const translationMap = { - blocks: __( 'Block styles' ), border: __( 'Border' ), color: __( 'Colors' ), - elements: __( 'Elements' ), - link: __( 'Links' ), - layout: __( 'Layout' ), spacing: __( 'Spacing' ), typography: __( 'Typography' ), + 'settings.color.palette': __( 'color palette' ), + 'settings.color.gradients': __( 'gradients' ), + 'settings.color.duotone': __( 'duotone colors' ), + 'settings.typography.fontFamilies': __( 'font family settings' ), + 'settings.typography.fontSizes': __( 'font size settings' ), + 'color.text': __( 'text color' ), + 'color.background': __( 'background color' ), + 'spacing.margin.top': __( 'margin top' ), + 'spacing.margin.bottom': __( 'margin bottom' ), + 'spacing.margin.left': __( 'margin left' ), + 'spacing.margin.right': __( 'margin right' ), + 'spacing.padding.top': __( 'padding top' ), + 'spacing.padding.bottom': __( 'padding bottom' ), + 'spacing.padding.left': __( 'padding left' ), + 'spacing.padding.right': __( 'padding right' ), + 'spacing.blockGap': __( 'block gap' ), + 'settings.layout.contentSize': __( 'content size' ), + 'settings.layout.wideSize': __( 'wide size' ), + 'typography.fontStyle': __( 'font style' ), + 'typography.fontSize': __( 'font size' ), + 'typography.lineHeight': __( 'line height' ), + 'typography.fontFamily': __( 'font family' ), + 'typography.fontWeight': __( 'font weight' ), }; /** @@ -36,102 +55,130 @@ export function getGlobalStylesChanges( { settings = {}, styles = {} } ) { return changes; } -const shuffle = ( array ) => { +function getTranslation( key ) { + console.log( 'key', key ); + + if ( translationMap[ key ] ) { + return translationMap[ key ]; + } + const keyArray = key.split( '.' ); + + if ( keyArray?.[ 0 ] === 'blocks' ) { + let blockName = keyArray[ 1 ].split( '/' )?.[ 1 ]; + blockName = blockName.charAt( 0 ).toUpperCase() + blockName.slice( 1 ); + // @todo maybe getBlockTypes() and find the block name from the block type. + return sprintf( + // translators: %1$s: block name, %2$s: changed property. + __( '%1$s block %2$s' ), + blockName.replace( /-/g, ' ' ), + keyArray?.[ 2 ] + ); + } + + if ( keyArray?.[ 0 ] === 'elements' ) { + return sprintf( + // translators: %1$s: block name, %2$s: changed property. + __( '%1$s element %2$s' ), + keyArray?.[ 1 ], + keyArray?.[ 2 ] + ); + } +} + +function shuffle( array ) { for ( let i = array.length - 1; i > 0; i-- ) { // eslint-disable-next-line no-restricted-syntax const j = Math.floor( Math.random() * ( i + 1 ) ); [ array[ i ], array[ j ] ] = [ array[ j ], array[ i ] ]; } return array; -}; +} -const cache = {}; // Should be a Set? +const cache = new Map(); export function getRevisionChanges( revision, previousRevision, - maxResults = 10 + maxResults = 4 ) { - if ( cache[ revision.id ] ) { - return cache[ revision.id ]; + if ( cache.has( revision.id ) ) { + return cache.get( revision.id ); } -console.log( 'revision, previousRevision', revision, previousRevision ); + const changedValueTree = deepCompare( { blocks: revision?.styles?.blocks, - elements: revision?.styles?.elements, color: revision?.styles?.color, typography: revision?.styles?.typography, - dimensions: revision?.styles?.dimensions, spacing: revision?.styles?.spacing, - border: revision?.styles?.border, - settingsColor: revision?.settings?.color, + elements: revision?.styles?.elements, + settings: revision?.settings, }, { blocks: previousRevision?.styles?.blocks, - elements: previousRevision?.styles?.elements, color: previousRevision?.styles?.color, typography: previousRevision?.styles?.typography, - dimensions: previousRevision?.styles?.dimensions, spacing: previousRevision?.styles?.spacing, - border: previousRevision?.styles?.border, - settingsColor: previousRevision?.settings?.color, + elements: previousRevision?.styles?.elements, + settings: previousRevision?.settings, } ); - - - console.log( 'flattenedChanges', changedValueTree); - const shuffled = shuffle( - changedValueTree.filter( ( { hasChanged, path } ) => hasChanged ) - ); - console.log( 'shuffled', shuffled ); - /* cache[ revision.id ] = shuffled + const hasMore = changedValueTree.length > maxResults; + // Remove dupes and shuffle results. + let result = shuffle( [ ...new Set( changedValueTree ) ] ) + // Limit to max results. .slice( 0, maxResults ) - .map( ( { path } ) => path ) + // Translate the keys. + .map( ( key ) => getTranslation( key ) ) + .filter( ( str ) => !! str ) .join( ', ' ); - return cache[ revision.id ];*/ + if ( hasMore ) { + result += '…'; + } + + cache.set( revision.id, result ); + + return result; } function isObject( obj ) { return obj !== null && typeof obj === 'object'; } -function deepCompare( revisionValue, configValue, depth = 0, parentPath = '' ) { - if ( ! isObject( revisionValue ) && ! isObject( configValue ) ) { - return [ - { - path: parentPath, - revisionValue, - configValue, - hasChanged: revisionValue !== configValue, - }, - ]; +function deepCompare( + changedObject, + originalObject, + depth = 0, + parentPath = '' +) { + if ( ! isObject( changedObject ) && ! isObject( originalObject ) ) { + // Only return a path if the value has changed. + // And then only the path name up to 3 levels deep. + return changedObject !== originalObject + ? parentPath.split( '.' ).slice( 0, 3 ).join( '.' ) + : undefined; } - revisionValue = isObject( revisionValue ) ? revisionValue : {}; - configValue = isObject( configValue ) ? configValue : {}; + changedObject = isObject( changedObject ) ? changedObject : {}; + originalObject = isObject( originalObject ) ? originalObject : {}; - const keys1 = Object.keys( revisionValue ); - const keys2 = Object.keys( configValue ); - const allKeys = new Set( [ ...keys1, ...keys2 ] ); + const changedKeys = Object.keys( changedObject ); + const originalKeys = Object.keys( originalObject ); + const allKeys = new Set( [ ...changedKeys, ...originalKeys ] ); let diffs = []; for ( const key of allKeys ) { const path = parentPath ? parentPath + '.' + key : key; - const subDiffs = deepCompare( - revisionValue[ key ], - configValue[ key ], + const changedPath = deepCompare( + changedObject[ key ], + originalObject[ key ], depth + 1, path ); - diffs = diffs.concat( subDiffs ); + if ( changedPath ) { + diffs = diffs.concat( changedPath ); + } } -/* diffs = diffs.filter( - ( diff ) => - diff.path.includes( 'behaviors' ) || - diff.path.includes( 'settings' ) || - diff.path.includes( 'styles' ) - );*/ return diffs; } diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js index de3fd0fe701b1f..4a1049aee6dfd6 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js @@ -19,6 +19,20 @@ import { getRevisionChanges } from './get-revision-changes'; const DAY_IN_MILLISECONDS = 60 * 60 * 1000 * 24; +function ChangedSummary( { revision, previousRevision } ) { + const summary = getRevisionChanges( revision, previousRevision ); + + if ( ! summary ) { + return null; + } + + return ( + + { summary } + + ); +} + /** * Returns a button label for the revision. * @@ -104,9 +118,10 @@ function RevisionsButtons( { const revisionAuthor = isUnsaved ? currentUser : author; const authorDisplayName = revisionAuthor?.name || __( 'User' ); const authorAvatar = revisionAuthor?.avatar_urls?.[ '48' ]; + const isFirstItem = index === 0; const isSelected = selectedRevisionId ? selectedRevisionId === id - : index === 0; + : isFirstItem; const isReset = 'parent' === id; const modifiedDate = getDate( modified ); const displayDate = @@ -173,14 +188,15 @@ function RevisionsButtons( { { isSelected && ( <> -

    - { getRevisionChanges( - revision, + + } + /> + { /* eslint-disable-next-line no-nested-ternary */ } { canApplyRevision ? ( + + ) } { isLoadingRevisionWithUnsavedChanges && ( { @@ -125,7 +126,7 @@ function RevisionsButtons( { role="group" > { userRevisions.map( ( revision, index ) => { - const { id, isLatest, author, modified } = revision; + const { id, author, modified } = revision; const isUnsaved = 'unsaved' === id; // Unsaved changes are created by the current user. const revisionAuthor = isUnsaved ? currentUser : author; @@ -135,6 +136,7 @@ function RevisionsButtons( { const isSelected = selectedRevisionId ? selectedRevisionId === id : isFirstItem; + const areStylesEqual = ! canApplyRevision && isSelected; const isReset = 'parent' === id; const modifiedDate = getDate( modified ); const displayDate = @@ -144,9 +146,9 @@ function RevisionsButtons( { : humanTimeDiff( modified ); const revisionLabel = getRevisionLabel( id, - isLatest, authorDisplayName, - dateI18n( datetimeAbbreviated, modifiedDate ) + dateI18n( datetimeAbbreviated, modifiedDate ), + areStylesEqual ); return ( @@ -155,6 +157,7 @@ function RevisionsButtons( { 'edit-site-global-styles-screen-revisions__revision-item', { 'is-selected': isSelected, + 'is-active': areStylesEqual, 'is-reset': isReset, } ) } @@ -166,7 +169,7 @@ function RevisionsButtons( { onClick={ () => { onChange( revision ); } } - label={ revisionLabel } + aria-label={ revisionLabel } > { isReset ? ( @@ -200,34 +203,14 @@ function RevisionsButtons( { ) } { isSelected && ( - <> - - { /* eslint-disable-next-line no-nested-ternary */ } - { canApplyRevision ? ( - - ) : ( - - { __( - 'Revision matches current editor styles.' - ) } - - ) } - + ) } ); diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss index def0293abcaaa7..780c27251ebbed 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss +++ b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss @@ -17,6 +17,10 @@ overflow: hidden; cursor: pointer; + &.is-active { + box-shadow: inset 5px 0 $black; + } + &:hover { background: rgba(var(--wp-admin-theme-color--rgb), 0.04); .edit-site-global-styles-screen-revisions__date { @@ -89,16 +93,6 @@ .edit-site-global-styles-screen-revisions__button { justify-content: center; width: 100%; - margin: 0 0 10px 10px; -} - -.edit-site-global-styles-screen-revision__button { - justify-content: center; - width: 90%; - margin: 0 0 $grid-unit-10 $grid-unit-20; - &:hover { - background: $white; - } } .edit-site-global-styles-screen-revisions__description { From a31cd2aad67cd069354233a80db1bd4275fe3410 Mon Sep 17 00:00:00 2001 From: ramon Date: Thu, 30 Nov 2023 20:03:12 +1100 Subject: [PATCH 06/21] Removing shuffle function and fixing up block spacing translation --- .../screen-revisions/get-revision-changes.js | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index fc92368d08473d..3400c8c17601fe 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -14,7 +14,7 @@ const translationMap = { 'color.background': __( 'background colors' ), 'spacing.margin': __( 'margin styles' ), 'spacing.padding': __( 'padding styles' ), - 'spacing.blockGap': __( 'block gap' ), + 'spacing.blockGap': __( 'block spacing' ), 'settings.layout': __( 'layout settings' ), 'typography.fontStyle': __( 'font style' ), 'typography.fontSize': __( 'font size' ), @@ -69,15 +69,6 @@ function getTranslation( key, blockNames ) { } } -function shuffle( array ) { - for ( let i = array.length - 1; i > 0; i-- ) { - // eslint-disable-next-line no-restricted-syntax - const j = Math.floor( Math.random() * ( i + 1 ) ); - [ array[ i ], array[ j ] ] = [ array[ j ], array[ i ] ]; - } - return array; -} - const cache = new Map(); export function getRevisionChanges( @@ -109,10 +100,11 @@ export function getRevisionChanges( } ); - // Remove dupes and shuffle results. - const result = shuffle( [ ...new Set( changedValueTree ) ] ) + // Remove duplicate results. + const result = [ ...new Set( changedValueTree ) ] // Translate the keys. .map( ( key ) => getTranslation( key, blockNames ) ) + // Remove duplicate or empty translations. .reduce( ( acc, curr ) => { if ( curr && ! acc.includes( curr ) ) { acc.push( curr ); From d27d6c0ecd71944cf058e16da092c033b2851c22 Mon Sep 17 00:00:00 2001 From: ramon Date: Fri, 1 Dec 2023 17:31:53 +1100 Subject: [PATCH 07/21] Using the revision has the Map key. This allows us to cache the revision sets themselves and account for changes to Unsaved revisions. --- .../global-styles/screen-revisions/get-revision-changes.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index 3400c8c17601fe..3e3b766d5f59d6 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -77,8 +77,8 @@ export function getRevisionChanges( blockNames, maxResults = 5 ) { - if ( cache.has( revision.id ) ) { - return cache.get( revision.id ); + if ( cache.has( revision ) ) { + return cache.get( revision ); } const changedValueTree = deepCompare( @@ -119,7 +119,7 @@ export function getRevisionChanges( joined += '…'; } - cache.set( revision.id, joined ); + cache.set( revision, joined ); return joined; } From 087084d839e15178b4b250f5c9b7de4646bf212a Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 4 Dec 2023 08:59:58 +1100 Subject: [PATCH 08/21] Used WeakMap in favour of Map for garbage collection, if it helps at all --- .../global-styles/screen-revisions/get-revision-changes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index 3e3b766d5f59d6..f7eb8730a2e224 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -69,7 +69,7 @@ function getTranslation( key, blockNames ) { } } -const cache = new Map(); +const cache = new WeakMap(); export function getRevisionChanges( revision, From dcca429ae463405f0c78c99c5ec0d71fa98076b6 Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 4 Dec 2023 09:08:37 +1100 Subject: [PATCH 09/21] Remove hasMore var - unneeded because it's only used once --- .../global-styles/screen-revisions/get-revision-changes.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index f7eb8730a2e224..9717a1662753e6 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -113,9 +113,8 @@ export function getRevisionChanges( }, [] ); let joined = result.slice( 0, maxResults ).join( ', ' ); - const hasMore = result.length > maxResults; - if ( hasMore ) { + if ( result.length > maxResults ) { joined += '…'; } From 23a2615f8900be160958e83edaa0ae33be8a9422 Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 4 Dec 2023 09:14:15 +1100 Subject: [PATCH 10/21] Tidying up - remove .map loop --- .../screen-revisions/get-revision-changes.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index 9717a1662753e6..8ca9bf13e173da 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -103,23 +103,24 @@ export function getRevisionChanges( // Remove duplicate results. const result = [ ...new Set( changedValueTree ) ] // Translate the keys. - .map( ( key ) => getTranslation( key, blockNames ) ) // Remove duplicate or empty translations. .reduce( ( acc, curr ) => { - if ( curr && ! acc.includes( curr ) ) { - acc.push( curr ); + const translation = getTranslation( curr, blockNames ); + if ( translation && ! acc.includes( translation ) ) { + acc.push( translation ); } return acc; }, [] ); - let joined = result.slice( 0, maxResults ).join( ', ' ); + const slicedResult = result.slice( 0, maxResults ); if ( result.length > maxResults ) { - joined += '…'; + // translators: follows comma-separated list of changed styles. + slicedResult.push( __( 'and more.' ) ); } + const joined = slicedResult.join( ', ' ); cache.set( revision, joined ); - return joined; } From 803c4d9ee88dadae88500a35209475301c5561a5 Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 4 Dec 2023 09:22:33 +1100 Subject: [PATCH 11/21] getGlobalStylesChanges was doing nothing! Removed. --- .../screen-revisions/get-revision-changes.js | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index 8ca9bf13e173da..ad1dbc6e1cbb86 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -23,28 +23,6 @@ const translationMap = { 'typography.fontWeight': __( 'font weight' ), }; -/** - * Returns an array of translated strings describing high level global styles and settings. - * - * @param {Object} revision - * @param {Object} revision.settings Global styles settings. - * @param {Object} revision.styles Global styles. - * @return {string[] | []} An array of translated labels. - */ -export function getGlobalStylesChanges( { settings = {}, styles = {} } ) { - const changes = []; - if ( Object.keys( settings ).length > 0 ) { - changes.push( __( 'Global settings' ) ); - } - Object.keys( styles ).forEach( ( key ) => { - if ( translationMap[ key ] ) { - changes.push( translationMap[ key ] ); - } - } ); - - return changes; -} - function getTranslation( key, blockNames ) { if ( translationMap[ key ] ) { return translationMap[ key ]; From 0d2d4257312341e2c28f5faf81cb756320c445d4 Mon Sep 17 00:00:00 2001 From: ramon Date: Wed, 6 Dec 2023 13:55:47 +1100 Subject: [PATCH 12/21] Using revision + previousRevision combo for cache key to ensure that the results are cached for the same two objects Returning from cache where maxResults value is smaller than cached results Added first tests --- .../screen-revisions/get-revision-changes.js | 155 ++++++++------ .../screen-revisions/revisions-buttons.js | 2 +- .../test/get-revision-changes.js | 196 ++++++++++++++++++ 3 files changed, 292 insertions(+), 61 deletions(-) create mode 100644 packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index ad1dbc6e1cbb86..8e49cd188f808f 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -3,6 +3,8 @@ */ import { __, sprintf } from '@wordpress/i18n'; +const globalStylesChangesCache = new WeakMap(); + const translationMap = { caption: __( 'caption' ), link: __( 'link' ), @@ -23,19 +25,30 @@ const translationMap = { 'typography.fontWeight': __( 'font weight' ), }; +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} 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 sprintf( - // translators: %s: block name. - __( '%s block' ), - blockName - ); + return blockName + ? sprintf( + // translators: %s: block name. + __( '%s block' ), + blockName + ) + : keyArray[ 1 ]; } if ( keyArray?.[ 0 ] === 'elements' ) { @@ -45,20 +58,89 @@ function getTranslation( key, blockNames ) { translationMap[ keyArray[ 1 ] ] ); } + + return undefined; } -const cache = new WeakMap(); +/** + * 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 + ? 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 : {}; -export function getRevisionChanges( + 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 a concatenated summary of translated global styles changes. + * Results are cached using a WeakMap key of `{ revision, previousRevision }`. + * + * @param {Object} revision The changed object to compare. + * @param {Object} previousRevision The original object to compare against. + * @param {Record} blockNames A key/value pair object of block names and their rendered titles. + * @param {number?} maxResults The maximum number of changed items to feature in the returned summary. + * @return {string} A comma-separated list of changes. + */ +export default function getRevisionChanges( revision, previousRevision, blockNames, - maxResults = 5 + maxResults ) { - if ( cache.has( revision ) ) { - return cache.get( revision ); + const cacheKey = { revision, previousRevision }; + + if ( globalStylesChangesCache.has( cacheKey ) ) { + const cachedResult = globalStylesChangesCache.get( cacheKey ); + if ( ! maxResults ) { + return cachedResult; + } + + // We may need to update the cache if a new maxResults is passed. + const cachedResultArray = cachedResult.split( ', ' ); + const cachedResultArrayLength = cachedResultArray.length; + if ( maxResults === cachedResultArrayLength ) { + return cachedResult; + } + + // If the cached result has more results than the max results, return a spliced cached result. + if ( maxResults < cachedResultArrayLength ) { + return cachedResultArray.slice( 0, maxResults ).join( ', ' ); + } } + // Compare the two revisions with normalized keys. const changedValueTree = deepCompare( { blocks: revision?.styles?.blocks, @@ -90,55 +172,8 @@ export function getRevisionChanges( return acc; }, [] ); - const slicedResult = result.slice( 0, maxResults ); - - if ( result.length > maxResults ) { - // translators: follows comma-separated list of changed styles. - slicedResult.push( __( 'and more.' ) ); - } - - const joined = slicedResult.join( ', ' ); - cache.set( revision, joined ); + const changes = maxResults ? result.slice( 0, maxResults ) : result; + const joined = changes.join( ', ' ); + globalStylesChangesCache.set( cacheKey, joined ); return joined; } - -function isObject( obj ) { - return obj !== null && typeof obj === 'object'; -} - -function deepCompare( - changedObject, - originalObject, - depth = 0, - parentPath = '' -) { - 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 - ? parentPath.split( '.' ).slice( 0, 2 ).join( '.' ) - : undefined; - } - - changedObject = isObject( changedObject ) ? changedObject : {}; - originalObject = isObject( originalObject ) ? originalObject : {}; - - const changedKeys = Object.keys( changedObject ); - const originalKeys = Object.keys( originalObject ); - const allKeys = new Set( [ ...changedKeys, ...originalKeys ] ); - - let diffs = []; - for ( const key of allKeys ) { - const path = parentPath ? parentPath + '.' + key : key; - const changedPath = deepCompare( - changedObject[ key ], - originalObject[ key ], - depth + 1, - path - ); - if ( changedPath ) { - diffs = diffs.concat( changedPath ); - } - } - return diffs; -} diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js index 6532fe391093c6..b40c189467de2c 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js @@ -17,7 +17,7 @@ import { getBlockTypes } from '@wordpress/blocks'; /** * Internal dependencies */ -import { getRevisionChanges } from './get-revision-changes'; +import getRevisionChanges from './get-revision-changes'; const DAY_IN_MILLISECONDS = 60 * 60 * 1000 * 24; diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js new file mode 100644 index 00000000000000..75072a42fa761b --- /dev/null +++ b/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js @@ -0,0 +1,196 @@ +/** + * Internal dependencies + */ +import getRevisionChanges from '../get-revision-changes'; + +describe( 'getRevisionChanges', () => { + const revision = { + id: 10, + styles: { + typography: { + fontSize: 'var(--wp--preset--font-size--potato)', + fontStyle: 'normal', + fontWeight: '600', + lineHeight: '1.85', + fontFamily: 'var(--wp--preset--font-family--asparagus)', + }, + spacing: { + padding: { + top: '36px', + right: '89px', + bottom: '133px', + left: 'var(--wp--preset--spacing--20)', + }, + blockGap: '114px', + }, + elements: { + heading: { + typography: { + letterSpacing: '37px', + }, + }, + caption: { + color: { + text: 'var(--wp--preset--color--pineapple)', + }, + }, + }, + color: { + text: 'var(--wp--preset--color--tomato)', + }, + blocks: { + 'core/paragraph': { + color: { + text: '#000000', + }, + }, + }, + }, + settings: { + color: { + palette: { + theme: [ + { + slug: 'one', + color: 'pink', + }, + ], + }, + }, + }, + }; + const previousRevision = { + id: 9, + styles: { + typography: { + fontSize: 'var(--wp--preset--font-size--fungus)', + fontStyle: 'normal', + fontWeight: '600', + lineHeight: '1.85', + fontFamily: 'var(--wp--preset--font-family--grapes)', + }, + spacing: { + padding: { + top: '36px', + right: '89px', + bottom: '133px', + left: 'var(--wp--preset--spacing--20)', + }, + blockGap: '114px', + }, + elements: { + heading: { + typography: { + letterSpacing: '37px', + }, + }, + caption: { + typography: { + fontSize: '1.11rem', + fontStyle: 'normal', + fontWeight: '600', + }, + }, + link: { + typography: { + lineHeight: 2, + textDecoration: 'line-through', + }, + color: { + text: 'var(--wp--preset--color--egg)', + }, + }, + }, + color: { + text: 'var(--wp--preset--color--tomato)', + background: 'var(--wp--preset--color--pumpkin)', + }, + blocks: { + 'core/paragraph': { + color: { + text: '#fff', + }, + }, + }, + }, + settings: { + color: { + palette: { + theme: [ + { + slug: 'one', + color: 'blue', + }, + ], + }, + }, + }, + }; + const blockNames = { + 'core/paragraph': 'Paragraph', + }; + it( 'returns a comma-separated list of changes and caches them', () => { + const resultA = getRevisionChanges( + revision, + previousRevision, + blockNames + ); + expect( resultA ).toEqual( + 'Paragraph block, background colors, font size, font family, caption element, link element, color settings' + ); + + const resultB = getRevisionChanges( + revision, + previousRevision, + blockNames + ); + + expect( resultA ).toBe( resultB ); + } ); + + it( 'returns a comma-separated list of changes with max results', () => { + const result = getRevisionChanges( + revision, + previousRevision, + blockNames, + 2 + ); + expect( result ).toEqual( 'Paragraph block, background colors' ); + } ); + + it( 'skips unknown keys', () => { + const result = getRevisionChanges( + { + styles: { + frogs: { + legs: 'green', + }, + typography: { + owl: 'hoot', + }, + settings: { + '': { + '': 'foo', + }, + }, + }, + }, + { + styles: { + frogs: { + legs: 'yellow', + }, + typography: { + cat: 'meow', + }, + settings: { + '': { + '': 'bar', + }, + }, + }, + } + ); + expect( result ).toEqual( '' ); + } ); +} ); From fe76b982f958edbe7067ae51164d73e2f78c4723 Mon Sep 17 00:00:00 2001 From: ramon Date: Wed, 6 Dec 2023 14:37:14 +1100 Subject: [PATCH 13/21] Moving maxResults decisions to consuming component. getRevisionChanges returns an unadulterated array. --- .../screen-revisions/get-revision-changes.js | 32 ++++--------------- .../screen-revisions/revisions-buttons.js | 14 ++++++-- .../test/get-revision-changes.js | 28 +++++++--------- 3 files changed, 30 insertions(+), 44 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index 8e49cd188f808f..6de9d085f654e3 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -104,40 +104,23 @@ function deepCompare( changedObject, originalObject, parentPath = '' ) { } /** - * Get a concatenated summary of translated global styles changes. + * Get an array of translated summarized global styles changes. * Results are cached using a WeakMap key of `{ revision, previousRevision }`. * * @param {Object} revision The changed object to compare. * @param {Object} previousRevision The original object to compare against. * @param {Record} blockNames A key/value pair object of block names and their rendered titles. - * @param {number?} maxResults The maximum number of changed items to feature in the returned summary. - * @return {string} A comma-separated list of changes. + * @return {string[]} An array of translated changes. */ export default function getRevisionChanges( revision, previousRevision, - blockNames, - maxResults + blockNames ) { const cacheKey = { revision, previousRevision }; if ( globalStylesChangesCache.has( cacheKey ) ) { - const cachedResult = globalStylesChangesCache.get( cacheKey ); - if ( ! maxResults ) { - return cachedResult; - } - - // We may need to update the cache if a new maxResults is passed. - const cachedResultArray = cachedResult.split( ', ' ); - const cachedResultArrayLength = cachedResultArray.length; - if ( maxResults === cachedResultArrayLength ) { - return cachedResult; - } - - // If the cached result has more results than the max results, return a spliced cached result. - if ( maxResults < cachedResultArrayLength ) { - return cachedResultArray.slice( 0, maxResults ).join( ', ' ); - } + return globalStylesChangesCache.get( cacheKey ); } // Compare the two revisions with normalized keys. @@ -172,8 +155,7 @@ export default function getRevisionChanges( return acc; }, [] ); - const changes = maxResults ? result.slice( 0, maxResults ) : result; - const joined = changes.join( ', ' ); - globalStylesChangesCache.set( cacheKey, joined ); - return joined; + globalStylesChangesCache.set( cacheKey, result ); + + return result; } diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js index b40c189467de2c..78281b2ef44b27 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js @@ -20,6 +20,7 @@ import { getBlockTypes } from '@wordpress/blocks'; import getRevisionChanges from './get-revision-changes'; const DAY_IN_MILLISECONDS = 60 * 60 * 1000 * 24; +const MAX_CHANGES = 7; function ChangedSummary( { revision, previousRevision } ) { const blockNames = useMemo( () => { @@ -29,19 +30,26 @@ function ChangedSummary( { revision, previousRevision } ) { return accumulator; }, {} ); }, [] ); - const summary = getRevisionChanges( + const changes = getRevisionChanges( revision, previousRevision, blockNames ); - if ( ! summary ) { + const changesLength = changes.length; + + if ( ! changesLength ) { return null; } + // Truncate to `n` results. + if ( changesLength > MAX_CHANGES ) { + changes.splice( MAX_CHANGES, changesLength - MAX_CHANGES, __( '…' ) ); + } + return ( - { summary } + { changes.join( ', ' ) } ); } diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js index 75072a42fa761b..25ad13ff9e2746 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js @@ -129,15 +129,21 @@ describe( 'getRevisionChanges', () => { const blockNames = { 'core/paragraph': 'Paragraph', }; - it( 'returns a comma-separated list of changes and caches them', () => { + it( 'returns a list of changes and caches them', () => { const resultA = getRevisionChanges( revision, previousRevision, blockNames ); - expect( resultA ).toEqual( - 'Paragraph block, background colors, font size, font family, caption element, link element, color settings' - ); + expect( resultA ).toEqual( [ + 'Paragraph block', + 'background colors', + 'font size', + 'font family', + 'caption element', + 'link element', + 'color settings', + ] ); const resultB = getRevisionChanges( revision, @@ -145,17 +151,7 @@ describe( 'getRevisionChanges', () => { blockNames ); - expect( resultA ).toBe( resultB ); - } ); - - it( 'returns a comma-separated list of changes with max results', () => { - const result = getRevisionChanges( - revision, - previousRevision, - blockNames, - 2 - ); - expect( result ).toEqual( 'Paragraph block, background colors' ); + expect( resultA ).toStrictEqual( resultB ); } ); it( 'skips unknown keys', () => { @@ -191,6 +187,6 @@ describe( 'getRevisionChanges', () => { }, } ); - expect( result ).toEqual( '' ); + expect( result ).toEqual( [] ); } ); } ); From c070f9fde0e9d161852111bf96a482cece10f5e6 Mon Sep 17 00:00:00 2001 From: ramon Date: Wed, 6 Dec 2023 14:58:05 +1100 Subject: [PATCH 14/21] Move get blockNames to main component --- .../screen-revisions/revisions-buttons.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js index 78281b2ef44b27..4f8a850f3da0c3 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js @@ -22,14 +22,7 @@ import getRevisionChanges from './get-revision-changes'; const DAY_IN_MILLISECONDS = 60 * 60 * 1000 * 24; const MAX_CHANGES = 7; -function ChangedSummary( { revision, previousRevision } ) { - const blockNames = useMemo( () => { - const blockTypes = getBlockTypes(); - return blockTypes.reduce( ( accumulator, { name, title } ) => { - accumulator[ name ] = title; - return accumulator; - }, {} ); - }, [] ); +function ChangedSummary( { revision, previousRevision, blockNames } ) { const changes = getRevisionChanges( revision, previousRevision, @@ -124,6 +117,13 @@ function RevisionsButtons( { currentUser: getCurrentUser(), }; }, [] ); + const blockNames = useMemo( () => { + const blockTypes = getBlockTypes(); + return blockTypes.reduce( ( accumulator, { name, title } ) => { + accumulator[ name ] = title; + return accumulator; + }, {} ); + }, [] ); const dateNowInMs = getDate().getTime(); const { datetimeAbbreviated } = getSettings().formats; @@ -212,6 +212,7 @@ function RevisionsButtons( { { isSelected && ( Date: Wed, 6 Dec 2023 15:11:22 +1100 Subject: [PATCH 15/21] Have to use map because WeakMap wants the same reference as the object key. --- .../global-styles/screen-revisions/get-revision-changes.js | 4 ++-- .../screen-revisions/test/get-revision-changes.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index 6de9d085f654e3..c0886ea4ec763a 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -3,7 +3,7 @@ */ import { __, sprintf } from '@wordpress/i18n'; -const globalStylesChangesCache = new WeakMap(); +const globalStylesChangesCache = new Map(); const translationMap = { caption: __( 'caption' ), @@ -117,7 +117,7 @@ export default function getRevisionChanges( previousRevision, blockNames ) { - const cacheKey = { revision, previousRevision }; + const cacheKey = JSON.stringify( { revision, previousRevision } ); if ( globalStylesChangesCache.has( cacheKey ) ) { return globalStylesChangesCache.get( cacheKey ); diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js index 25ad13ff9e2746..ef2f67a0970912 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js @@ -151,7 +151,7 @@ describe( 'getRevisionChanges', () => { blockNames ); - expect( resultA ).toStrictEqual( resultB ); + expect( resultA ).toBe( resultB ); } ); it( 'skips unknown keys', () => { From f53ce5313855074efb3db5eec545910e5f1d5920 Mon Sep 17 00:00:00 2001 From: ramon Date: Wed, 6 Dec 2023 19:33:29 +1100 Subject: [PATCH 16/21] Remove the trailing comma on truncated results --- .../screen-revisions/revisions-buttons.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js index 4f8a850f3da0c3..ba050cfa76b036 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js @@ -23,11 +23,7 @@ const DAY_IN_MILLISECONDS = 60 * 60 * 1000 * 24; const MAX_CHANGES = 7; function ChangedSummary( { revision, previousRevision, blockNames } ) { - const changes = getRevisionChanges( - revision, - previousRevision, - blockNames - ); + let changes = getRevisionChanges( revision, previousRevision, blockNames ); const changesLength = changes.length; @@ -37,12 +33,14 @@ function ChangedSummary( { revision, previousRevision, blockNames } ) { // Truncate to `n` results. if ( changesLength > MAX_CHANGES ) { - changes.splice( MAX_CHANGES, changesLength - MAX_CHANGES, __( '…' ) ); + changes = changes.slice( 0, MAX_CHANGES ).join( ', ' ) + __( '…' ); + } else { + changes = changes.join( ', ' ); } return ( - { changes.join( ', ' ) } + { changes } ); } From d5dd7e79ae7758fee7c1b93e7e85f59adcb94565 Mon Sep 17 00:00:00 2001 From: ramon Date: Thu, 7 Dec 2023 14:23:14 +1100 Subject: [PATCH 17/21] Test commit: listing changes, showing `and n more` --- .../screen-revisions/get-revision-changes.js | 6 ++++ .../screen-revisions/revisions-buttons.js | 28 +++++++++++++++---- .../global-styles/screen-revisions/style.scss | 20 +++++++++---- 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index c0886ea4ec763a..470b9ad642771d 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -4,6 +4,7 @@ import { __, sprintf } from '@wordpress/i18n'; const globalStylesChangesCache = new Map(); +const EMPTY_ARRAY = []; const translationMap = { caption: __( 'caption' ), @@ -143,6 +144,11 @@ export default function getRevisionChanges( } ); + if ( ! changedValueTree.length ) { + globalStylesChangesCache.set( cacheKey, EMPTY_ARRAY ); + return EMPTY_ARRAY; + } + // Remove duplicate results. const result = [ ...new Set( changedValueTree ) ] // Translate the keys. diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js index ba050cfa76b036..e323a2606fb82c 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js @@ -23,7 +23,11 @@ const DAY_IN_MILLISECONDS = 60 * 60 * 1000 * 24; const MAX_CHANGES = 7; function ChangedSummary( { revision, previousRevision, blockNames } ) { - let changes = getRevisionChanges( revision, previousRevision, blockNames ); + const changes = getRevisionChanges( + revision, + previousRevision, + blockNames + ); const changesLength = changes.length; @@ -33,14 +37,28 @@ function ChangedSummary( { revision, previousRevision, blockNames } ) { // Truncate to `n` results. if ( changesLength > MAX_CHANGES ) { - changes = changes.slice( 0, MAX_CHANGES ).join( ', ' ) + __( '…' ); - } else { - changes = changes.join( ', ' ); + const deleteCount = changesLength - MAX_CHANGES; + changes.splice( + MAX_CHANGES, + deleteCount, + sprintf( + /* translators: %d remaining changes that aren't displayed */ + __( 'and %d more…' ), + deleteCount + ) + ); } return ( - { changes } +

    + { __( 'Changes:' ) } +
    +
      + { changes.map( ( change ) => ( +
    • { change }
      • + ) ) } +
    ); } diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss index 780c27251ebbed..361029be2ccd55 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss +++ b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss @@ -127,9 +127,8 @@ margin: $grid-unit-30 auto !important; } -.edit-site-global-styles-screen-revision__matches, .edit-site-global-styles-screen-revision__changes { - margin: 0 $grid-unit-20 $grid-unit-20 $grid-unit-20; + margin: 0 $grid-unit-20 $grid-unit-20 $grid-unit-30; color: $gray-900; display: block; &::first-letter { @@ -137,8 +136,19 @@ } } -.edit-site-global-styles-screen-revision__matches { - font-style: italic; +.edit-site-global-styles-screen-revision__changes-title { + font-weight: 700; + display: inline-block; + margin-right: $grid-unit-05; + text-transform: uppercase; font-size: 12px; - color: $gray-600; + color: $gray-800; + margin-bottom: $grid-unit-05; +} + +.edit-site-global-styles-screen-revision__changes-list { + margin-left: $grid-unit-15; + font-size: 12px; + list-style: circle; + color: $gray-700; } From ec31842824d5f8daf9a3ef74110749f2e9c19610 Mon Sep 17 00:00:00 2001 From: ramon Date: Thu, 7 Dec 2023 15:16:18 +1100 Subject: [PATCH 18/21] Test commit: grouping changes using tuples --- .../screen-revisions/get-revision-changes.js | 94 +++++++++---------- .../screen-revisions/revisions-buttons.js | 66 ++++++++----- .../global-styles/screen-revisions/style.scss | 30 ++++-- .../test/get-revision-changes.js | 14 +-- 4 files changed, 112 insertions(+), 92 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index 470b9ad642771d..c973d9c80546f0 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -1,29 +1,33 @@ /** * WordPress dependencies */ -import { __, sprintf } from '@wordpress/i18n'; +import { __ } 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' ), - 'color.text': __( 'text colors' ), - 'color.background': __( 'background colors' ), - 'spacing.margin': __( 'margin styles' ), - 'spacing.padding': __( 'padding styles' ), - 'spacing.blockGap': __( 'block spacing' ), - 'settings.layout': __( 'layout settings' ), - 'typography.fontStyle': __( 'font style' ), - 'typography.fontSize': __( 'font size' ), - 'typography.lineHeight': __( 'line height' ), - 'typography.fontFamily': __( 'font family' ), - 'typography.fontWeight': __( 'font weight' ), + blocks: __( 'Blocks' ), + elements: __( 'Elements' ), + 'elements.caption': __( 'Caption' ), + 'elements.link': __( 'Link' ), + 'elements.button': __( 'Button' ), + 'elements.heading': __( 'Heading' ), + settings: __( 'Settings' ), + 'settings.color': __( 'Color' ), + 'settings.typography': __( 'Typography' ), + 'settings.layout': __( 'Layout' ), + styles: __( 'Styles' ), + 'styles.color.text': __( 'Text' ), + 'styles.color.background': __( 'Background' ), + 'styles.spacing.margin': __( 'Margin' ), + 'styles.spacing.padding': __( 'Padding' ), + 'styles.spacing.blockGap': __( 'Block spacing' ), + 'styles.typography.fontStyle': __( 'Font style' ), + 'styles.typography.fontSize': __( 'Font size' ), + 'styles.typography.lineHeight': __( 'Line height' ), + 'styles.typography.fontFamily': __( 'Font family' ), + 'styles.typography.fontWeight': __( 'Font weight' ), }; const isObject = ( obj ) => obj !== null && typeof obj === 'object'; @@ -38,26 +42,9 @@ 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 ] ] - ); + if ( key.startsWith( 'blocks.' ) ) { + const keyArray = key.split( '.' ); + return blockNames[ keyArray[ 1 ] ] || keyArray[ 1 ]; } return undefined; @@ -74,9 +61,11 @@ 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. + // And then only the path name up to `n` levels deep to reduce the results. + const splitKeys = parentPath.split( '.' ); + const depth = 'styles' === splitKeys[ 0 ] ? 3 : 2; return changedObject !== originalObject - ? parentPath.split( '.' ).slice( 0, 2 ).join( '.' ) + ? splitKeys.slice( 0, depth ).join( '.' ) : undefined; } @@ -106,12 +95,12 @@ function deepCompare( changedObject, originalObject, parentPath = '' ) { /** * Get an array of translated summarized global styles changes. - * Results are cached using a WeakMap key of `{ revision, previousRevision }`. + * 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} blockNames A key/value pair object of block names and their rendered titles. - * @return {string[]} An array of translated changes. + * @return {Array[]} An 2-dimensional array of tuples: [ "group", "translated change" ]. */ export default function getRevisionChanges( revision, @@ -128,17 +117,21 @@ export default function getRevisionChanges( const changedValueTree = deepCompare( { blocks: revision?.styles?.blocks, - color: revision?.styles?.color, - typography: revision?.styles?.typography, - spacing: revision?.styles?.spacing, + styles: { + color: revision?.styles?.color, + typography: revision?.styles?.typography, + spacing: revision?.styles?.spacing, + }, elements: revision?.styles?.elements, settings: revision?.settings, }, { blocks: previousRevision?.styles?.blocks, - color: previousRevision?.styles?.color, - typography: previousRevision?.styles?.typography, - spacing: previousRevision?.styles?.spacing, + styles: { + color: previousRevision?.styles?.color, + typography: previousRevision?.styles?.typography, + spacing: previousRevision?.styles?.spacing, + }, elements: previousRevision?.styles?.elements, settings: previousRevision?.settings, } @@ -156,7 +149,10 @@ export default function getRevisionChanges( .reduce( ( acc, curr ) => { const translation = getTranslation( curr, blockNames ); if ( translation && ! acc.includes( translation ) ) { - acc.push( translation ); + acc.push( [ + translationMap[ curr.split( '.' )[ 0 ] ], + translation, + ] ); } return acc; }, [] ); diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js index e323a2606fb82c..fd39c6d41c6ecd 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js @@ -6,7 +6,7 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { __, sprintf } from '@wordpress/i18n'; +import { __, _n, sprintf } from '@wordpress/i18n'; import { Button } from '@wordpress/components'; import { dateI18n, getDate, humanTimeDiff, getSettings } from '@wordpress/date'; import { store as coreStore } from '@wordpress/core-data'; @@ -23,11 +23,7 @@ const DAY_IN_MILLISECONDS = 60 * 60 * 1000 * 24; const MAX_CHANGES = 7; function ChangedSummary( { revision, previousRevision, blockNames } ) { - const changes = getRevisionChanges( - revision, - previousRevision, - blockNames - ); + let changes = getRevisionChanges( revision, previousRevision, blockNames ); const changesLength = changes.length; @@ -37,29 +33,47 @@ function ChangedSummary( { revision, previousRevision, blockNames } ) { // Truncate to `n` results. if ( changesLength > MAX_CHANGES ) { - const deleteCount = changesLength - MAX_CHANGES; - changes.splice( - MAX_CHANGES, - deleteCount, - sprintf( - /* translators: %d remaining changes that aren't displayed */ - __( 'and %d more…' ), - deleteCount - ) - ); + changes = changes.slice( 0, MAX_CHANGES ); } + const moreChanges = changesLength - changes.length; + + changes = changes.reduce( ( acc, curr ) => { + acc[ curr[ 0 ] ] = ! acc[ curr[ 0 ] ] + ? [ curr[ 1 ] ] + : [ ...acc[ curr[ 0 ] ], curr[ 1 ] ]; + return acc; + }, {} ); + return ( - -
    - { __( 'Changes:' ) } -
    -
      - { changes.map( ( change ) => ( -
    • { change }
      • - ) ) } -
    -     +
    + { Object.entries( changes )?.map( ( [ key, changeValues ] ) => ( +
    + + { key }: + + + { changeValues.join( ', ' ) } + +
    + ) ) } + { moreChanges > 0 ? ( + + { sprintf( + // translators: %d: number of global styles changes that are not displayed in the UI. + _n( + '…and %d more change.', + '…and %d more changes.', + moreChanges + ), + moreChanges + ) } + + ) : null } +
    ); } diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss index 361029be2ccd55..eb11688ea44462 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss +++ b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss @@ -129,26 +129,36 @@ .edit-site-global-styles-screen-revision__changes { margin: 0 $grid-unit-20 $grid-unit-20 $grid-unit-30; - color: $gray-900; - display: block; - &::first-letter { - text-transform: uppercase; - } +} + +.edit-site-global-styles-screen-revision__header { + text-transform: uppercase; + margin-bottom: $grid-unit-05; + font-weight: 700; + color: $gray-700; + font-size: 12px; +} + +.edit-site-global-styles-screen-revision__changes-group { + margin-bottom: $grid-unit-05; + color: $gray-800; } .edit-site-global-styles-screen-revision__changes-title { font-weight: 700; - display: inline-block; margin-right: $grid-unit-05; - text-transform: uppercase; font-size: 12px; - color: $gray-800; - margin-bottom: $grid-unit-05; + color: $gray-700; } .edit-site-global-styles-screen-revision__changes-list { - margin-left: $grid-unit-15; font-size: 12px; list-style: circle; color: $gray-700; } + +.edit-site-global-styles-screen-revision__more { + font-style: italic; + color: $gray-700; + font-size: 12px; +} diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js index ef2f67a0970912..4aee0be71b2a3e 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js @@ -136,13 +136,13 @@ describe( 'getRevisionChanges', () => { blockNames ); expect( resultA ).toEqual( [ - 'Paragraph block', - 'background colors', - 'font size', - 'font family', - 'caption element', - 'link element', - 'color settings', + [ 'Blocks', 'Paragraph' ], + [ 'Styles', 'Background' ], + [ 'Styles', 'Font size' ], + [ 'Styles', 'Font family' ], + [ 'Elements', 'Caption' ], + [ 'Elements', 'Link' ], + [ 'Settings', 'Color' ], ] ); const resultB = getRevisionChanges( From fde41c810c0663da5607c99988642bf7d4e79915 Mon Sep 17 00:00:00 2001 From: ramon Date: Fri, 8 Dec 2023 11:03:20 +1100 Subject: [PATCH 19/21] Reverting back to comma-separate list of changes Added e2e assertion --- .../screen-revisions/get-revision-changes.js | 88 ++++++++++--------- .../global-styles/screen-revisions/index.js | 11 +-- .../screen-revisions/revisions-buttons.js | 66 +++++--------- .../global-styles/screen-revisions/style.scss | 37 ++------ .../test/get-revision-changes.js | 19 ++-- .../user-global-styles-revisions.spec.js | 5 ++ 6 files changed, 90 insertions(+), 136 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js index c973d9c80546f0..fed075eb923ff4 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/get-revision-changes.js @@ -1,33 +1,21 @@ /** * WordPress dependencies */ -import { __ } from '@wordpress/i18n'; +import { __, sprintf } from '@wordpress/i18n'; const globalStylesChangesCache = new Map(); const EMPTY_ARRAY = []; const translationMap = { - blocks: __( 'Blocks' ), - elements: __( 'Elements' ), - 'elements.caption': __( 'Caption' ), - 'elements.link': __( 'Link' ), - 'elements.button': __( 'Button' ), - 'elements.heading': __( 'Heading' ), - settings: __( 'Settings' ), - 'settings.color': __( 'Color' ), - 'settings.typography': __( 'Typography' ), - 'settings.layout': __( 'Layout' ), - styles: __( 'Styles' ), - 'styles.color.text': __( 'Text' ), - 'styles.color.background': __( 'Background' ), - 'styles.spacing.margin': __( 'Margin' ), - 'styles.spacing.padding': __( 'Padding' ), - 'styles.spacing.blockGap': __( 'Block spacing' ), - 'styles.typography.fontStyle': __( 'Font style' ), - 'styles.typography.fontSize': __( 'Font size' ), - 'styles.typography.lineHeight': __( 'Line height' ), - 'styles.typography.fontFamily': __( 'Font family' ), - 'styles.typography.fontWeight': __( 'Font weight' ), + 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'; @@ -42,9 +30,26 @@ function getTranslation( key, blockNames ) { if ( translationMap[ key ] ) { return translationMap[ key ]; } - if ( key.startsWith( 'blocks.' ) ) { - const keyArray = key.split( '.' ); - return blockNames[ keyArray[ 1 ] ] || keyArray[ 1 ]; + + 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; @@ -60,12 +65,12 @@ function getTranslation( key, blockNames ) { 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 `n` levels deep to reduce the results. - const splitKeys = parentPath.split( '.' ); - const depth = 'styles' === splitKeys[ 0 ] ? 3 : 2; + /* + * Only return a path if the value has changed. + * And then only the path name up to 2 levels deep. + */ return changedObject !== originalObject - ? splitKeys.slice( 0, depth ).join( '.' ) + ? parentPath.split( '.' ).slice( 0, 2 ).join( '.' ) : undefined; } @@ -100,7 +105,7 @@ function deepCompare( changedObject, originalObject, parentPath = '' ) { * @param {Object} revision The changed object to compare. * @param {Object} previousRevision The original object to compare against. * @param {Record} blockNames A key/value pair object of block names and their rendered titles. - * @return {Array[]} An 2-dimensional array of tuples: [ "group", "translated change" ]. + * @return {string[]} An array of translated changes. */ export default function getRevisionChanges( revision, @@ -113,25 +118,29 @@ export default function getRevisionChanges( return globalStylesChangesCache.get( cacheKey ); } - // Compare the two revisions with normalized keys. + /* + * 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( { - blocks: revision?.styles?.blocks, styles: { color: revision?.styles?.color, typography: revision?.styles?.typography, spacing: revision?.styles?.spacing, }, + blocks: revision?.styles?.blocks, elements: revision?.styles?.elements, settings: revision?.settings, }, { - blocks: previousRevision?.styles?.blocks, styles: { color: previousRevision?.styles?.color, typography: previousRevision?.styles?.typography, spacing: previousRevision?.styles?.spacing, }, + blocks: previousRevision?.styles?.blocks, elements: previousRevision?.styles?.elements, settings: previousRevision?.settings, } @@ -144,15 +153,14 @@ export default function getRevisionChanges( // Remove duplicate results. const result = [ ...new Set( changedValueTree ) ] - // Translate the keys. - // Remove duplicate or empty translations. + /* + * Translate the keys. + * Remove duplicate or empty translations. + */ .reduce( ( acc, curr ) => { const translation = getTranslation( curr, blockNames ); if ( translation && ! acc.includes( translation ) ) { - acc.push( [ - translationMap[ curr.split( '.' )[ 0 ] ], - translation, - ] ); + acc.push( translation ); } return acc; }, [] ); 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 c5e102caf68526..aa380c5a9fbd0b 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,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'; @@ -157,7 +156,7 @@ function ScreenRevisions() { { isLoading && ( ) } - { shouldShowRevisions ? ( + { shouldShowRevisions && ( <> ) } - ) : ( - - { - // 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/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js index fd39c6d41c6ecd..b1fcf3fa52e3ed 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js @@ -23,57 +23,35 @@ const DAY_IN_MILLISECONDS = 60 * 60 * 1000 * 24; const MAX_CHANGES = 7; function ChangedSummary( { revision, previousRevision, blockNames } ) { - let changes = getRevisionChanges( revision, previousRevision, blockNames ); - + const changes = getRevisionChanges( + revision, + previousRevision, + blockNames + ); const changesLength = changes.length; if ( ! changesLength ) { return null; } - // Truncate to `n` results. + // Truncate to `n` results if necessary. if ( changesLength > MAX_CHANGES ) { - changes = changes.slice( 0, MAX_CHANGES ); + const deleteCount = changesLength - MAX_CHANGES; + const andMoreText = sprintf( + // translators: %d: number of global styles changes that are not displayed in the UI. + _n( '…and %d more change.', '…and %d more changes.', deleteCount ), + deleteCount + ); + changes.splice( MAX_CHANGES, deleteCount, andMoreText ); } - const moreChanges = changesLength - changes.length; - - changes = changes.reduce( ( acc, curr ) => { - acc[ curr[ 0 ] ] = ! acc[ curr[ 0 ] ] - ? [ curr[ 1 ] ] - : [ ...acc[ curr[ 0 ] ], curr[ 1 ] ]; - return acc; - }, {} ); - return ( -
    - { Object.entries( changes )?.map( ( [ key, changeValues ] ) => ( -
    - - { key }: - - - { changeValues.join( ', ' ) } - -
    - ) ) } - { moreChanges > 0 ? ( - - { sprintf( - // translators: %d: number of global styles changes that are not displayed in the UI. - _n( - '…and %d more change.', - '…and %d more changes.', - moreChanges - ), - moreChanges - ) } - - ) : null } -
    + + { changes.join( ', ' ) } + ); } @@ -98,7 +76,7 @@ function getRevisionLabel( if ( 'unsaved' === id ) { return sprintf( - /* translators: %s author display name */ + /* translators: %s: author display name */ __( 'Unsaved changes by %s' ), authorDisplayName ); @@ -106,7 +84,7 @@ function getRevisionLabel( return areStylesEqual ? sprintf( - /* translators: %1$s author display name, %2$s: revision creation date */ + // translators: %1$s: author display name, %2$s: revision creation date. __( 'Changes saved by %1$s on %2$s. This revision matches current editor styles.' ), @@ -114,7 +92,7 @@ function getRevisionLabel( formattedModifiedDate ) : sprintf( - /* translators: %1$s author display name, %2$s: revision creation date */ + // translators: %1$s: author display name, %2$s: revision creation date. __( 'Changes saved by %1$s on %2$s' ), authorDisplayName, formattedModifiedDate diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss index eb11688ea44462..1fd0d0bda6c881 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss +++ b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss @@ -129,36 +129,9 @@ .edit-site-global-styles-screen-revision__changes { margin: 0 $grid-unit-20 $grid-unit-20 $grid-unit-30; -} - -.edit-site-global-styles-screen-revision__header { - text-transform: uppercase; - margin-bottom: $grid-unit-05; - font-weight: 700; - color: $gray-700; - font-size: 12px; -} - -.edit-site-global-styles-screen-revision__changes-group { - margin-bottom: $grid-unit-05; - color: $gray-800; -} - -.edit-site-global-styles-screen-revision__changes-title { - font-weight: 700; - margin-right: $grid-unit-05; - font-size: 12px; - color: $gray-700; -} - -.edit-site-global-styles-screen-revision__changes-list { - font-size: 12px; - list-style: circle; - color: $gray-700; -} - -.edit-site-global-styles-screen-revision__more { - font-style: italic; - color: $gray-700; - font-size: 12px; + color: $gray-900; + display: block; + &::first-letter { + text-transform: uppercase; + } } diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js b/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js index 4aee0be71b2a3e..be9a26b97f6885 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/test/get-revision-changes.js @@ -136,13 +136,12 @@ describe( 'getRevisionChanges', () => { blockNames ); expect( resultA ).toEqual( [ - [ 'Blocks', 'Paragraph' ], - [ 'Styles', 'Background' ], - [ 'Styles', 'Font size' ], - [ 'Styles', 'Font family' ], - [ 'Elements', 'Caption' ], - [ 'Elements', 'Link' ], - [ 'Settings', 'Color' ], + 'Colors', + 'Typography', + 'Paragraph block', + 'Caption element', + 'Link element', + 'Color settings', ] ); const resultB = getRevisionChanges( @@ -154,7 +153,7 @@ describe( 'getRevisionChanges', () => { expect( resultA ).toBe( resultB ); } ); - it( 'skips unknown keys', () => { + it( 'skips unknown and unchanged keys', () => { const result = getRevisionChanges( { styles: { @@ -162,7 +161,7 @@ describe( 'getRevisionChanges', () => { legs: 'green', }, typography: { - owl: 'hoot', + fontSize: '1rem', }, settings: { '': { @@ -177,7 +176,7 @@ describe( 'getRevisionChanges', () => { legs: 'yellow', }, typography: { - cat: 'meow', + fontSize: '1rem', }, settings: { '': { 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 2d51b5ac5014b8..a27bb28adbb911 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 @@ -55,6 +55,11 @@ test.describe( 'Global styles revisions', () => { name: /^Changes saved by /, } ); + // Shows changes made in the revision. + await expect( + page.getByTestId( 'global-styles-revision-changes' ) + ).toHaveText( 'Colors' ); + // There should be 2 revisions not including the reset to theme defaults button. await expect( revisionButtons ).toHaveCount( currentRevisions.length + 1 From 559cd3ecf43086c3446e0a1543895999d42ec81a Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 11 Dec 2023 15:06:52 +1100 Subject: [PATCH 20/21] Swapping order of author name and changes block Moving everything into the button so it's clickable. --- .../screen-revisions/revisions-buttons.js | 24 +++++++++---------- .../global-styles/screen-revisions/style.scss | 19 ++++++--------- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js index b1fcf3fa52e3ed..96819d3add5c78 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js @@ -48,7 +48,7 @@ function ChangedSummary( { revision, previousRevision, blockNames } ) { return ( { changes.join( ', ' ) } @@ -208,6 +208,17 @@ function RevisionsButtons( { { displayDate } ) } + { isSelected && ( + + ) } { ) } - { isSelected && ( - - ) } ); } ) } diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss index 1fd0d0bda6c881..d1325f84772a62 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss +++ b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss @@ -17,10 +17,6 @@ overflow: hidden; cursor: pointer; - &.is-active { - box-shadow: inset 5px 0 $black; - } - &:hover { background: rgba(var(--wp-admin-theme-color--rgb), 0.04); .edit-site-global-styles-screen-revisions__date { @@ -70,7 +66,7 @@ width: 100%; height: auto; display: block; - padding: $grid-unit-15 $grid-unit-15 $grid-unit-15 $grid-unit-30; + padding: $grid-unit-15 $grid-unit-15 $grid-unit-10 $grid-unit-30; &:focus, &:active { outline: 0; @@ -107,6 +103,7 @@ } } +.edit-site-global-styles-screen-revisions__changes, .edit-site-global-styles-screen-revisions__meta { color: $gray-600; display: flex; @@ -114,7 +111,6 @@ width: 100%; align-items: center; font-size: 12px; - img { width: $grid-unit-20; height: $grid-unit-20; @@ -127,11 +123,10 @@ margin: $grid-unit-30 auto !important; } -.edit-site-global-styles-screen-revision__changes { - margin: 0 $grid-unit-20 $grid-unit-20 $grid-unit-30; +.edit-site-global-styles-screen-revisions__changes { + margin-bottom: $grid-unit-05; + text-align: left; color: $gray-900; - display: block; - &::first-letter { - text-transform: uppercase; - } + line-height: $default-line-height; } + From 25b8a159ea3a7c9c9f5af074ed6bd9c0bd73ec3c Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 11 Dec 2023 15:14:51 +1100 Subject: [PATCH 21/21] Don't live in the past, man --- .../global-styles/screen-revisions/revisions-buttons.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js index 96819d3add5c78..08930069425729 100644 --- a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js +++ b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js @@ -22,7 +22,7 @@ import getRevisionChanges from './get-revision-changes'; const DAY_IN_MILLISECONDS = 60 * 60 * 1000 * 24; const MAX_CHANGES = 7; -function ChangedSummary( { revision, previousRevision, blockNames } ) { +function ChangesSummary( { revision, previousRevision, blockNames } ) { const changes = getRevisionChanges( revision, previousRevision, @@ -209,7 +209,7 @@ function RevisionsButtons( { ) } { isSelected && ( -