From 55cc60cf9d740b7aca77a83263608647b7340bda Mon Sep 17 00:00:00 2001 From: ramon Date: Thu, 27 Jun 2024 15:46:19 +1000 Subject: [PATCH] Removed unused style fixture --- .../src/hooks/block-style-variation.js | 27 ++--- packages/block-editor/src/hooks/index.js | 2 +- packages/block-editor/src/private-apis.js | 4 +- .../src/components/revisions/index.js | 4 +- .../block-style-variations.spec.js | 99 +++++++++++++++---- 5 files changed, 101 insertions(+), 35 deletions(-) diff --git a/packages/block-editor/src/hooks/block-style-variation.js b/packages/block-editor/src/hooks/block-style-variation.js index f00c96d640a57..21259966d8a63 100644 --- a/packages/block-editor/src/hooks/block-style-variation.js +++ b/packages/block-editor/src/hooks/block-style-variation.js @@ -69,18 +69,14 @@ function OverrideStyles( { override } ) { * This component is used to generate new block style variation overrides * based on an incoming theme config. If a matching style is found in the config, * a new override is created and returned. The overrides can be used in conjunction with - * useStyleOverride to apply the new styles to the editor. - * NOTE: This component is required to load global styles revisions config. Style variation overrides are - * generated using the canvas's current global styles (see useBlockStyleVariation()). This component is, - * however, due to be refactored and therefore it's use elsewhere is not recommended. + * useStyleOverride to apply the new styles to the editor. Its use is + * subject to change. * * @param {Object} props Props. * @param {Object} props.config A global styles object, containing settings and styles. - * @return {JSX.Element} An array of new block variation overrides. + * @return {JSX.Element|undefined} An array of new block variation overrides. */ -export function ExperimentalBlockStyleVariationOverridesWithConfig( { - config, -} ) { +export function __unstableBlockStyleVariationOverridesWithConfig( { config } ) { const { getBlockStyles, overrides } = useSelect( ( select ) => ( { getBlockStyles: select( blocksStore ).getBlockStyles, @@ -91,8 +87,11 @@ export function ExperimentalBlockStyleVariationOverridesWithConfig( { const { getBlockName } = useSelect( blockEditorStore ); const overridesWithConfig = useMemo( () => { + if ( ! overrides?.length ) { + return; + } const newOverrides = []; - const clientIds = []; + const overriddenClientIds = []; for ( const [ , override ] of overrides ) { if ( override?.variation && @@ -101,7 +100,7 @@ export function ExperimentalBlockStyleVariationOverridesWithConfig( { * Because this component overwrites existing style overrides, * filter out any overrides that are already present in the store. */ - ! clientIds.includes( override.clientId ) + ! overriddenClientIds.includes( override.clientId ) ) { const blockName = getBlockName( override.clientId ); const configStyles = @@ -161,12 +160,16 @@ export function ExperimentalBlockStyleVariationOverridesWithConfig( { // correct CSS cascade is maintained. clientId: override.clientId, } ); - clientIds.push( override.clientId ); + overriddenClientIds.push( override.clientId ); } } } return newOverrides; - }, [ config, overrides, getBlockStyles ] ); + }, [ config, overrides, getBlockStyles, getBlockName ] ); + + if ( ! overridesWithConfig || ! overridesWithConfig.length ) { + return; + } return ( <> diff --git a/packages/block-editor/src/hooks/index.js b/packages/block-editor/src/hooks/index.js index 6e79d2b3dbab8..bd1835571fdd4 100644 --- a/packages/block-editor/src/hooks/index.js +++ b/packages/block-editor/src/hooks/index.js @@ -88,4 +88,4 @@ export { getTypographyClassesAndStyles } from './use-typography-props'; export { getGapCSSValue } from './gap'; export { useCachedTruthy } from './use-cached-truthy'; export { useZoomOut } from './use-zoom-out'; -export { ExperimentalBlockStyleVariationOverridesWithConfig } from './block-style-variation'; +export { __unstableBlockStyleVariationOverridesWithConfig } from './block-style-variation'; diff --git a/packages/block-editor/src/private-apis.js b/packages/block-editor/src/private-apis.js index b408693e37534..bfa6ac0c90c84 100644 --- a/packages/block-editor/src/private-apis.js +++ b/packages/block-editor/src/private-apis.js @@ -23,7 +23,7 @@ import { BlockRemovalWarningModal } from './components/block-removal-warning-mod import { useLayoutClasses, useLayoutStyles, - ExperimentalBlockStyleVariationOverridesWithConfig, + __unstableBlockStyleVariationOverridesWithConfig, } from './hooks'; import DimensionsTool from './components/dimensions-tool'; import ResolutionTool from './components/resolution-tool'; @@ -92,5 +92,5 @@ lock( privateApis, { PrivatePublishDateTimePicker, useSpacingSizes, useBlockDisplayTitle, - ExperimentalBlockStyleVariationOverridesWithConfig, + __unstableBlockStyleVariationOverridesWithConfig, } ); diff --git a/packages/edit-site/src/components/revisions/index.js b/packages/edit-site/src/components/revisions/index.js index 932905a8effd0..d43b5e8d2ac02 100644 --- a/packages/edit-site/src/components/revisions/index.js +++ b/packages/edit-site/src/components/revisions/index.js @@ -25,7 +25,7 @@ const { ExperimentalBlockEditorProvider, GlobalStylesContext, useGlobalStylesOutputWithConfig, - ExperimentalBlockStyleVariationOverridesWithConfig, + __unstableBlockStyleVariationOverridesWithConfig, } = unlock( blockEditorPrivateApis ); const { mergeBaseAndUserConfigs } = unlock( editorPrivateApis ); @@ -93,7 +93,7 @@ function Revisions( { userConfig, blocks } ) { * so they can access any registered style overrides. */ } - diff --git a/test/e2e/specs/site-editor/block-style-variations.spec.js b/test/e2e/specs/site-editor/block-style-variations.spec.js index 88539c844a6a8..fedcd0946eac8 100644 --- a/test/e2e/specs/site-editor/block-style-variations.spec.js +++ b/test/e2e/specs/site-editor/block-style-variations.spec.js @@ -16,7 +16,6 @@ test.describe( 'Block Style Variations', () => { requestUtils.activateTheme( 'gutenberg-test-themes/style-variations' ), - requestUtils.deleteAllPages(), ] ); stylesPostId = await requestUtils.getCurrentThemeGlobalStylesPostId(); } ); @@ -57,32 +56,43 @@ test.describe( 'Block Style Variations', () => { await page.getByRole( 'tab', { name: 'Styles' } ).click(); await page .getByRole( 'button', { name: 'Block Style Variation A' } ) - .click( { force: true } ); + .click(); - // Check parent styles. + // Check parent styles have variation A styles. await expect( firstGroup ).toHaveCSS( 'border-style', 'dotted' ); await expect( firstGroup ).toHaveCSS( 'border-width', '1px' ); - // Check nested child and grandchild group block inherited styles. + // Check nested child and grandchild group block variation A inherited styles. await expect( secondGroup ).toHaveCSS( 'border-style', 'solid' ); await expect( secondGroup ).toHaveCSS( 'border-width', '11px' ); await expect( thirdGroup ).toHaveCSS( 'border-style', 'solid' ); await expect( thirdGroup ).toHaveCSS( 'border-width', '11px' ); - // Apply a block style to the first, nested Group block. + // Apply a block style to the nested child Group block. await editor.selectBlocks( secondGroup ); await page.getByRole( 'tab', { name: 'Styles' } ).click(); await page .getByRole( 'button', { name: 'Block Style Variation B' } ) - .click( { force: true } ); + .click(); - // Check nested child styles. + // Check nested child styles have variation B styles. await expect( secondGroup ).toHaveCSS( 'border-style', 'dashed' ); await expect( secondGroup ).toHaveCSS( 'border-width', '2px' ); - // Check nested grandchild group block inherited styles. + // Check nested grandchild Group block variation B inherited styles. await expect( thirdGroup ).toHaveCSS( 'border-style', 'groove' ); await expect( thirdGroup ).toHaveCSS( 'border-width', '22px' ); + + // Apply a block style to the nested grandchild Group block. + await editor.selectBlocks( thirdGroup ); + await page.getByRole( 'tab', { name: 'Styles' } ).click(); + await page + .getByRole( 'button', { name: 'Block Style Variation A' } ) + .click(); + + // Check that the child's inner block styles from variation B are overridden by the grandchild's block style variation A. + await expect( thirdGroup ).toHaveCSS( 'border-style', 'dotted' ); + await expect( thirdGroup ).toHaveCSS( 'border-width', '1px' ); } ); test( 'update block style variations in global styles and check revisions match styles', async ( { @@ -108,16 +118,17 @@ test.describe( 'Block Style Variations', () => { await page.getByRole( 'tab', { name: 'Styles' } ).click(); await page .getByRole( 'button', { name: 'Block Style Variation A' } ) - .click( { force: true } ); + .click(); // Apply a block style to the first, nested Group block. await editor.selectBlocks( secondGroup ); await page.getByRole( 'tab', { name: 'Styles' } ).click(); await page .getByRole( 'button', { name: 'Block Style Variation B' } ) - .click( { force: true } ); + .click(); // Update user global styles with new block style variation values. + // First revision. await siteEditorBlockStyleVariations.saveRevision( stylesPostId, { blocks: { 'core/group': { @@ -138,14 +149,41 @@ test.describe( 'Block Style Variations', () => { }, }, } ); + // The save button has been re-enabled. + await expect( + page + .getByRole( 'region', { name: 'Editor top bar' } ) + .getByRole( 'button', { name: 'Save' } ) + ).toBeEnabled(); + // Second revision (current). + await siteEditorBlockStyleVariations.saveRevision( stylesPostId, { + blocks: { + 'core/group': { + variations: { + 'block-style-variation-a': { + border: { + width: '7px', + style: 'dotted', + }, + }, + 'block-style-variation-b': { + border: { + width: '8px', + style: 'dotted', + }, + }, + }, + }, + }, + } ); // Check parent styles. - await expect( firstGroup ).toHaveCSS( 'border-style', 'outset' ); - await expect( firstGroup ).toHaveCSS( 'border-width', '3px' ); + await expect( firstGroup ).toHaveCSS( 'border-style', 'dotted' ); + await expect( firstGroup ).toHaveCSS( 'border-width', '7px' ); // Check nested child styles. - await expect( secondGroup ).toHaveCSS( 'border-style', 'double' ); - await expect( secondGroup ).toHaveCSS( 'border-width', '4px' ); + await expect( secondGroup ).toHaveCSS( 'border-style', 'dotted' ); + await expect( secondGroup ).toHaveCSS( 'border-width', '8px' ); // Check nested grandchild group block inherited styles. await expect( thirdGroup ).toHaveCSS( 'border-style', 'groove' ); @@ -182,21 +220,46 @@ test.describe( 'Block Style Variations', () => { .locator( '[data-type="core/group"]' ) .nth( 2 ); - // Check parent styles. + // Check parent styles have current revision. await expect( revisionFirstGroup ).toHaveCSS( 'border-style', - 'outset' + 'dotted' + ); + await expect( revisionFirstGroup ).toHaveCSS( 'border-width', '7px' ); + + // Check nested child styles have current revision. + await expect( revisionSecondGroup ).toHaveCSS( + 'border-style', + 'dotted' ); + await expect( revisionSecondGroup ).toHaveCSS( 'border-width', '8px' ); + + // Check nested grandchild group block inherited styles from current revision. + await expect( revisionThirdGroup ).toHaveCSS( + 'border-style', + 'groove' + ); + await expect( revisionThirdGroup ).toHaveCSS( 'border-width', '22px' ); + + // Click on previous revision. + await page + .getByRole( 'button', { + name: /^Changes saved by /, + } ) + .nth( 1 ) + .click(); + + // Check parent styles have previous revision. await expect( revisionFirstGroup ).toHaveCSS( 'border-width', '3px' ); - // Check nested child styles. + // Check nested child style have previous revision. await expect( revisionSecondGroup ).toHaveCSS( 'border-style', 'double' ); await expect( revisionSecondGroup ).toHaveCSS( 'border-width', '4px' ); - // Check nested grandchild group block inherited styles. + // Check nested grandchild group block inherited styles from previous revision. await expect( revisionThirdGroup ).toHaveCSS( 'border-style', 'groove'