From 41a483de5999859ee08f1dfdd84e6c75645ca4e9 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Thu, 18 Apr 2024 03:08:35 +0800 Subject: [PATCH] Further simplify pattern overrides flow (#60769) * Adjust allow pattern overrides UX flow * Minor tweaks * Fix e2e tests * Simplify button text * Use * Update packages/patterns/src/components/allow-overrides-modal.js Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com> * Fix tests --------- Co-authored-by: kevin940726 Co-authored-by: richtabor Co-authored-by: t-hamano Co-authored-by: jameskoster Co-authored-by: SaxonF --- .../src/components/allow-overrides-modal.js | 89 +++++++++++++++---- .../components/pattern-overrides-controls.js | 71 ++++++++------- packages/patterns/src/components/style.scss | 1 + .../editor/various/pattern-overrides.spec.js | 8 +- 4 files changed, 116 insertions(+), 53 deletions(-) diff --git a/packages/patterns/src/components/allow-overrides-modal.js b/packages/patterns/src/components/allow-overrides-modal.js index 21d306022fd58..e2389575171a3 100644 --- a/packages/patterns/src/components/allow-overrides-modal.js +++ b/packages/patterns/src/components/allow-overrides-modal.js @@ -5,6 +5,7 @@ import { __experimentalHStack as HStack, __experimentalVStack as VStack, Button, + __experimentalText as Text, TextControl, Modal, } from '@wordpress/components'; @@ -12,25 +13,28 @@ import { __, sprintf } from '@wordpress/i18n'; import { useState, useId } from '@wordpress/element'; import { speak } from '@wordpress/a11y'; -export default function AllowOverridesModal( { +export function AllowOverridesModal( { placeholder, + initialName = '', onClose, onSave, } ) { - const [ editedBlockName, setEditedBlockName ] = useState( '' ); + const [ editedBlockName, setEditedBlockName ] = useState( initialName ); const descriptionId = useId(); const isNameValid = !! editedBlockName.trim(); const handleSubmit = () => { - const message = sprintf( - /* translators: %s: new name/label for the block */ - __( 'Block name changed to: "%s".' ), - editedBlockName - ); + if ( editedBlockName !== initialName ) { + const message = sprintf( + /* translators: %s: new name/label for the block */ + __( 'Block name changed to: "%s".' ), + editedBlockName + ); - // Must be assertive to immediately announce change. - speak( message, 'assertive' ); + // Must be assertive to immediately announce change. + speak( message, 'assertive' ); + } onSave( editedBlockName ); // Immediate close avoids ability to hit save multiple times. @@ -39,9 +43,8 @@ export default function AllowOverridesModal( { return ( -

- { __( 'Enter a custom name for this block.' ) } -

- + + + { __( + 'Overrides are changes you make to a block within a synced pattern instance. Use overrides to customize a synced pattern instance to suit its new context. Name this block to specify an override.' + ) } + - { __( 'Save' ) } + { __( 'Enable' ) } + + + + + + ); +} + +export function DisallowOverridesModal( { onClose, onSave } ) { + const descriptionId = useId(); + + return ( + +
{ + event.preventDefault(); + onSave(); + onClose(); + } } + > + + + { __( + 'Are you sure you want to disable overrides? Disabling overrides will revert all applied overrides for this block throughout instances of this pattern.' + ) } + + + + + + diff --git a/packages/patterns/src/components/pattern-overrides-controls.js b/packages/patterns/src/components/pattern-overrides-controls.js index c55bcb8b2d7f8..1cfe953b4a455 100644 --- a/packages/patterns/src/components/pattern-overrides-controls.js +++ b/packages/patterns/src/components/pattern-overrides-controls.js @@ -1,9 +1,9 @@ /** * WordPress dependencies */ -import { useState, useId, useRef, flushSync } from '@wordpress/element'; +import { useState, useId } from '@wordpress/element'; import { InspectorControls } from '@wordpress/block-editor'; -import { ToggleControl, BaseControl, Button } from '@wordpress/components'; +import { BaseControl, Button } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; /** @@ -13,7 +13,10 @@ import { PARTIAL_SYNCING_SUPPORTED_BLOCKS, PATTERN_OVERRIDES_BINDING_SOURCE, } from '../constants'; -import AllowOverridesModal from './allow-overrides-modal'; +import { + AllowOverridesModal, + DisallowOverridesModal, +} from './allow-overrides-modal'; function removeBindings( bindings, syncedAttributes ) { let updatedBindings = {}; @@ -47,9 +50,10 @@ function addBindings( bindings, syncedAttributes ) { function PatternOverridesControls( { attributes, name, setAttributes } ) { const controlId = useId(); - const toggleRef = useRef(); const [ showAllowOverridesModal, setShowAllowOverridesModal ] = useState( false ); + const [ showDisallowOverridesModal, setShowDisallowOverridesModal ] = + useState( false ); const syncedAttributes = PARTIAL_SYNCING_SUPPORTED_BLOCKS[ name ]; const attributeSources = syncedAttributes.map( @@ -83,7 +87,12 @@ function PatternOverridesControls( { attributes, name, setAttributes } ) { // Avoid overwriting other (e.g. meta) bindings. if ( isConnectedToOtherSources ) return null; - const hasName = attributes.metadata?.name; + const hasName = !! attributes.metadata?.name; + const allowOverrides = + hasName && + attributeSources.some( + ( source ) => source === PATTERN_OVERRIDES_BINDING_SOURCE + ); return ( <> @@ -92,45 +101,43 @@ function PatternOverridesControls( { attributes, name, setAttributes } ) { id={ controlId } label={ __( 'Overrides' ) } help={ __( - 'Allow attributes within this block to be overridden by pattern instances.' + 'Allow changes to this block throughout instances of this pattern.' ) } > - { hasName ? ( - - source === PATTERN_OVERRIDES_BINDING_SOURCE - ) } - onChange={ ( isChecked ) => { - updateBindings( isChecked ); - } } - ref={ toggleRef } - /> - ) : ( - - ) } + { showAllowOverridesModal && ( setShowAllowOverridesModal( false ) } onSave={ ( newName ) => { - flushSync( () => { - updateBindings( true, newName ); - } ); - toggleRef.current?.focus(); + updateBindings( true, newName ); } } /> ) } + { showDisallowOverridesModal && ( + setShowDisallowOverridesModal( false ) } + onSave={ () => updateBindings( false ) } + /> + ) } ); } diff --git a/packages/patterns/src/components/style.scss b/packages/patterns/src/components/style.scss index 36c441ed20659..86af3df7ff3b9 100644 --- a/packages/patterns/src/components/style.scss +++ b/packages/patterns/src/components/style.scss @@ -43,3 +43,4 @@ width: 100%; justify-content: center; } + diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index df61ad63d90fd..4dbe65436aa95 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -90,8 +90,12 @@ test.describe( 'Pattern Overrides', () => { .getByRole( 'button', { name: 'Advanced' } ) .click(); await editorSettings - .getByRole( 'checkbox', { name: 'Allow overrides' } ) - .setChecked( true ); + .getByRole( 'button', { name: 'Enable overrides' } ) + .click(); + await page + .getByRole( 'dialog', { name: 'Enable overrides' } ) + .getByRole( 'button', { name: 'Enable' } ) + .click(); await expect.poll( editor.getBlocks ).toMatchObject( [ {