From ec31842824d5f8daf9a3ef74110749f2e9c19610 Mon Sep 17 00:00:00 2001 From: ramon Date: Thu, 7 Dec 2023 15:16:18 +1100 Subject: [PATCH] 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(