From c7d85aa330025e9a9321d6bff41aab10a1f5d2f4 Mon Sep 17 00:00:00 2001 From: Ramon Date: Mon, 19 Feb 2024 11:45:35 +1100 Subject: [PATCH] Global style changes: refactor output for a more flexible UI and grouping (#59055) * Refactoring output for a more flexible UI and grouping * update e2e test * Update comments and tests remove top margin * Localize comma separators and period Co-authored-by: ramonjd Co-authored-by: andrewserong Co-authored-by: t-hamano Co-authored-by: jasmussen Co-authored-by: jameskoster --- .../get-global-styles-changes.js | 102 +++++++--- .../test/get-global-styles-changes.js | 189 +++++++++++------- .../screen-revisions/revisions-buttons.js | 12 +- .../global-styles/screen-revisions/style.scss | 17 +- .../entities-saved-states/entity-type-list.js | 8 +- .../entities-saved-states/style.scss | 8 +- .../user-global-styles-revisions.spec.js | 7 +- 7 files changed, 223 insertions(+), 120 deletions(-) diff --git a/packages/block-editor/src/components/global-styles/get-global-styles-changes.js b/packages/block-editor/src/components/global-styles/get-global-styles-changes.js index 9bbd11fb7d797..fb15355f3b6f9 100644 --- a/packages/block-editor/src/components/global-styles/get-global-styles-changes.js +++ b/packages/block-editor/src/components/global-styles/get-global-styles-changes.js @@ -22,8 +22,8 @@ const translationMap = { h4: __( 'H4' ), h5: __( 'H5' ), h6: __( 'H6' ), - 'settings.color': __( 'Color settings' ), - 'settings.typography': __( 'Typography settings' ), + 'settings.color': __( 'Color' ), + 'settings.typography': __( 'Typography' ), 'styles.color': __( 'Colors' ), 'styles.spacing': __( 'Spacing' ), 'styles.typography': __( 'Typography' ), @@ -54,12 +54,7 @@ function getTranslation( key ) { } if ( keyArray?.[ 0 ] === 'elements' ) { - const elementName = translationMap[ keyArray[ 1 ] ] || keyArray[ 1 ]; - return sprintf( - // translators: %s: element name, e.g., heading button, link, caption. - __( '%s element' ), - elementName - ); + return translationMap[ keyArray[ 1 ] ] || keyArray[ 1 ]; } return undefined; @@ -114,9 +109,9 @@ function deepCompare( changedObject, originalObject, parentPath = '' ) { * * @param {Object} next The changed object to compare. * @param {Object} previous The original object to compare against. - * @return {string[]} An array of translated changes. + * @return {Array[]} A 2-dimensional array of tuples: [ "group", "translated change" ]. */ -function getGlobalStylesChangelist( next, previous ) { +export function getGlobalStylesChangelist( next, previous ) { const cacheKey = JSON.stringify( { next, previous } ); if ( globalStylesChangesCache.has( cacheKey ) ) { @@ -160,12 +155,12 @@ function getGlobalStylesChangelist( next, previous ) { const result = [ ...new Set( changedValueTree ) ] /* * Translate the keys. - * Remove duplicate or empty translations. + * Remove empty translations. */ .reduce( ( acc, curr ) => { const translation = getTranslation( curr ); - if ( translation && ! acc.includes( translation ) ) { - acc.push( translation ); + if ( translation ) { + acc.push( [ curr.split( '.' )[ 0 ], translation ] ); } return acc; }, [] ); @@ -176,29 +171,80 @@ function getGlobalStylesChangelist( next, previous ) { } /** - * From a getGlobalStylesChangelist() result, returns a truncated array of translated changes. - * Appends a translated string indicating the number of changes that were truncated. + * From a getGlobalStylesChangelist() result, returns an array of translated global styles changes, grouped by type. + * The types are 'blocks', 'elements', 'settings', and 'styles'. * * @param {Object} next The changed object to compare. * @param {Object} previous The original object to compare against. * @param {{maxResults:number}} options Options. maxResults: results to return before truncating. - * @return {string[]} An array of translated changes. + * @return {string[]} An array of translated changes. */ export default function getGlobalStylesChanges( next, previous, options = {} ) { - const changes = getGlobalStylesChangelist( next, previous ); - const changesLength = changes.length; + let changeList = getGlobalStylesChangelist( next, previous ); + const changesLength = changeList.length; const { maxResults } = options; - // Truncate to `n` results if necessary. - if ( !! maxResults && changesLength && changesLength > maxResults ) { - const deleteCount = changesLength - maxResults; - 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( maxResults, deleteCount, andMoreText ); + if ( changesLength ) { + // Truncate to `n` results if necessary. + if ( !! maxResults && changesLength > maxResults ) { + changeList = changeList.slice( 0, maxResults ); + } + return Object.entries( + changeList.reduce( ( acc, curr ) => { + const group = acc[ curr[ 0 ] ] || []; + if ( ! group.includes( curr[ 1 ] ) ) { + acc[ curr[ 0 ] ] = [ ...group, curr[ 1 ] ]; + } + return acc; + }, {} ) + ).map( ( [ key, changeValues ] ) => { + const changeValuesLength = changeValues.length; + const joinedChangesValue = changeValues.join( __( ', ' ) ); + switch ( key ) { + case 'blocks': { + return sprintf( + // translators: %2$s: a list of block names separated by a comma. + _n( '%2$s block.', '%2$s blocks.', changeValuesLength ), + changeValuesLength, + joinedChangesValue + ); + } + case 'elements': { + return sprintf( + // translators: %2$s: a list of element names separated by a comma. + _n( + '%2$s element.', + '%2$s elements.', + changeValuesLength + ), + changeValuesLength, + joinedChangesValue + ); + } + case 'settings': { + return sprintf( + // translators: %s: a list of theme.json setting labels separated by a comma. + __( '%s settings.' ), + joinedChangesValue + ); + } + case 'styles': { + return sprintf( + // translators: %s: a list of theme.json top-level styles labels separated by a comma. + __( '%s styles.' ), + joinedChangesValue + ); + } + default: { + return sprintf( + // translators: %s: a list of global styles changes separated by a comma. + __( '%s.' ), + joinedChangesValue + ); + } + } + } ); } - return changes; + return EMPTY_ARRAY; } diff --git a/packages/block-editor/src/components/global-styles/test/get-global-styles-changes.js b/packages/block-editor/src/components/global-styles/test/get-global-styles-changes.js index 2e7a68dab1f7b..9ff840dc76730 100644 --- a/packages/block-editor/src/components/global-styles/test/get-global-styles-changes.js +++ b/packages/block-editor/src/components/global-styles/test/get-global-styles-changes.js @@ -1,7 +1,9 @@ /** * Internal dependencies */ -import getGlobalStylesChanges from '../get-global-styles-changes'; +import getGlobalStylesChanges, { + getGlobalStylesChangelist, +} from '../get-global-styles-changes'; /** * WordPress dependencies @@ -12,24 +14,8 @@ import { getBlockTypes, } from '@wordpress/blocks'; -describe( 'getGlobalStylesChanges', () => { - beforeEach( () => { - registerBlockType( 'core/test-fiori-di-zucca', { - save: () => {}, - category: 'text', - title: 'Test pumpkin flowers', - edit: () => {}, - } ); - } ); - - afterEach( () => { - getBlockTypes().forEach( ( block ) => { - unregisterBlockType( block.name ); - } ); - } ); - - const revision = { - id: 10, +describe( 'getGlobalStylesChanges and utils', () => { + const next = { styles: { typography: { fontSize: 'var(--wp--preset--font-size--potato)', @@ -85,11 +71,18 @@ describe( 'getGlobalStylesChanges', () => { }, ], }, + gradients: [ + { + name: 'Something something', + gradient: + 'linear-gradient(105deg,rgba(6,147,100,1) 0%,rgb(155,81,100) 100%)', + slug: 'something-something', + }, + ], }, }, }; - const previousRevision = { - id: 9, + const previous = { styles: { typography: { fontSize: 'var(--wp--preset--font-size--fungus)', @@ -161,74 +154,120 @@ describe( 'getGlobalStylesChanges', () => { color: 'blue', }, ], + custom: [ + { + slug: 'one', + color: 'tomato', + }, + ], }, + gradients: [ + { + name: 'Vivid cyan blue to vivid purple', + gradient: + 'linear-gradient(135deg,rgba(6,147,227,1) 0%,rgb(155,81,224) 100%)', + slug: 'vivid-cyan-blue-to-vivid-purple', + }, + ], + }, + typography: { + fluid: true, }, }, }; - it( 'returns a list of changes and caches them', () => { - const resultA = getGlobalStylesChanges( revision, previousRevision ); - expect( resultA ).toEqual( [ - 'Colors', - 'Typography', - 'Test pumpkin flowers', - 'H3 element', - 'Caption element', - 'H6 element', - 'Link element', - 'Color settings', - ] ); - - const resultB = getGlobalStylesChanges( revision, previousRevision ); - - expect( resultA ).toBe( resultB ); + beforeEach( () => { + registerBlockType( 'core/test-fiori-di-zucca', { + save: () => {}, + category: 'text', + title: 'Test pumpkin flowers', + edit: () => {}, + } ); } ); - it( 'returns a list of truncated changes', () => { - const resultA = getGlobalStylesChanges( revision, previousRevision, { - maxResults: 3, + afterEach( () => { + getBlockTypes().forEach( ( block ) => { + unregisterBlockType( block.name ); } ); - expect( resultA ).toEqual( [ - 'Colors', - 'Typography', - 'Test pumpkin flowers', - '…and 5 more changes', - ] ); } ); - it( 'skips unknown and unchanged keys', () => { - const result = getGlobalStylesChanges( - { - styles: { - frogs: { - legs: 'green', - }, - typography: { - fontSize: '1rem', - }, - settings: { - '': { - '': 'foo', + describe( 'getGlobalStylesChanges()', () => { + it( 'returns a list of changes', () => { + const result = getGlobalStylesChanges( next, previous ); + expect( result ).toEqual( [ + 'Colors, Typography styles.', + 'Test pumpkin flowers block.', + 'H3, Caption, H6, Link elements.', + 'Color, Typography settings.', + ] ); + } ); + + it( 'returns a list of truncated changes', () => { + const resultA = getGlobalStylesChanges( next, previous, { + maxResults: 3, + } ); + expect( resultA ).toEqual( [ + 'Colors, Typography styles.', + 'Test pumpkin flowers block.', + ] ); + } ); + + it( 'skips unknown and unchanged keys', () => { + const result = getGlobalStylesChanges( + { + styles: { + frogs: { + legs: 'green', + }, + typography: { + fontSize: '1rem', + }, + settings: { + '': { + '': 'foo', + }, }, }, }, - }, - { - styles: { - frogs: { - legs: 'yellow', - }, - typography: { - fontSize: '1rem', - }, - settings: { - '': { - '': 'bar', + { + styles: { + frogs: { + legs: 'yellow', + }, + typography: { + fontSize: '1rem', + }, + settings: { + '': { + '': 'bar', + }, }, }, - }, - } - ); - expect( result ).toEqual( [] ); + } + ); + expect( result ).toEqual( [] ); + } ); + } ); + + describe( 'getGlobalStylesChangelist()', () => { + it( 'compares two objects and returns a cached list of changed keys', () => { + const resultA = getGlobalStylesChangelist( next, previous ); + + expect( resultA ).toEqual( [ + [ 'styles', 'Colors' ], + [ 'styles', 'Typography' ], + [ 'blocks', 'Test pumpkin flowers' ], + [ 'elements', 'H3' ], + [ 'elements', 'Caption' ], + [ 'elements', 'H6' ], + [ 'elements', 'Link' ], + [ 'settings', 'Color' ], + [ 'settings', 'Typography' ], + ] ); + + const resultB = getGlobalStylesChangelist( next, previous ); + + expect( resultB ).toEqual( resultA ); + } ); } ); } ); 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 07d479251add9..0670710de2e3b 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 @@ -25,19 +25,20 @@ function ChangesSummary( { revision, previousRevision } ) { const changes = getGlobalStylesChanges( revision, previousRevision, { maxResults: 7, } ); - const changesLength = changes.length; - if ( ! changesLength ) { + if ( ! changes.length ) { return null; } return ( - - { changes.join( ', ' ) }. - + { changes.map( ( change ) => ( +
  • { change }
    • + ) ) } + ); } @@ -219,6 +220,7 @@ function RevisionsButtons( { ) : (