From f918a6baf99a69b78d72bf2b5cdbb3cc33414526 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 9 Sep 2021 21:01:46 +0300 Subject: [PATCH 01/12] Show "none" as an alignment option and use contextual text to clarify settings --- .../block-alignment-control/style.scss | 5 + .../components/block-alignment-control/ui.js | 135 ++++++++++++++---- .../use-available-alignments.js | 19 ++- packages/block-editor/src/hooks/align.js | 2 +- packages/block-editor/src/style.scss | 1 + packages/components/src/menu-item/README.md | 8 ++ packages/components/src/menu-item/index.js | 8 +- packages/components/src/menu-item/style.scss | 8 +- 8 files changed, 147 insertions(+), 39 deletions(-) create mode 100644 packages/block-editor/src/components/block-alignment-control/style.scss diff --git a/packages/block-editor/src/components/block-alignment-control/style.scss b/packages/block-editor/src/components/block-alignment-control/style.scss new file mode 100644 index 00000000000000..4897b18c1bcb39 --- /dev/null +++ b/packages/block-editor/src/components/block-alignment-control/style.scss @@ -0,0 +1,5 @@ +.block-editor-block-alignment-control__menu-group { + .components-menu-item__info { + margin-top: 0; + } +} diff --git a/packages/block-editor/src/components/block-alignment-control/ui.js b/packages/block-editor/src/components/block-alignment-control/ui.js index 141b6c02b4a77a..cb1dad534580df 100644 --- a/packages/block-editor/src/components/block-alignment-control/ui.js +++ b/packages/block-editor/src/components/block-alignment-control/ui.js @@ -1,8 +1,13 @@ /** * WordPress dependencies */ -import { __ } from '@wordpress/i18n'; -import { ToolbarDropdownMenu, ToolbarGroup } from '@wordpress/components'; +import { __, sprintf } from '@wordpress/i18n'; +import { + ToolbarDropdownMenu, + ToolbarGroup, + MenuGroup, + MenuItem, +} from '@wordpress/components'; import { positionCenter, positionLeft, @@ -10,13 +15,22 @@ import { stretchFullWidth, stretchWide, } from '@wordpress/icons'; +import { useMemo } from '@wordpress/element'; /** * Internal dependencies */ import useAvailableAlignments from './use-available-alignments'; +/** + * External dependencies + */ +import classNames from 'classnames'; const BLOCK_ALIGNMENTS_CONTROLS = { + none: { + icon: positionCenter, + title: __( 'None' ), + }, left: { icon: positionLeft, title: __( 'Align left' ), @@ -39,7 +53,8 @@ const BLOCK_ALIGNMENTS_CONTROLS = { }, }; -const DEFAULT_CONTROL = 'center'; +const DEFAULT_CONTROL = 'none'; +const help = __( 'Your theme does not support wide alignments' ); const POPOVER_PROPS = { isAlternate: true, @@ -52,13 +67,37 @@ function BlockAlignmentUI( { isToolbar, isCollapsed = true, } ) { - const enabledControls = useAvailableAlignments( controls ); - if ( enabledControls.length === 0 ) { + const { + enabledControls, + layout: { contentSize, wideSize } = {}, + wideAlignmentsSupport, + } = useAvailableAlignments( controls ); + const hasEnabledControls = !! enabledControls.length; + const alignmentsInfo = useMemo( () => { + if ( ! hasEnabledControls ) { + return; + } + const info = {}; + if ( !! contentSize ) { + // translators: %s: container size (i.e. 600px etc) + info.none = sprintf( __( 'Max %s wide' ), contentSize ); + } + if ( wideAlignmentsSupport && !! wideSize ) { + // translators: %s: container size (i.e. 600px etc) + info.wide = sprintf( __( 'Max %s wide' ), wideSize ); + } + return info; + }, [ hasEnabledControls, contentSize, wideSize, wideAlignmentsSupport ] ); + + if ( ! hasEnabledControls ) { return null; } + // Always add the `none` option. + enabledControls.unshift( 'none' ); function applyOrUnset( align ) { - return () => onChange( value === align ? undefined : align ); + return () => + onChange( [ value, 'none' ].includes( align ) ? undefined : align ); } const activeAlignmentControl = BLOCK_ALIGNMENTS_CONTROLS[ value ]; @@ -66,29 +105,69 @@ function BlockAlignmentUI( { BLOCK_ALIGNMENTS_CONTROLS[ DEFAULT_CONTROL ]; const UIComponent = isToolbar ? ToolbarGroup : ToolbarDropdownMenu; - const extraProps = isToolbar ? { isCollapsed } : {}; + const commonProps = { + popoverProps: POPOVER_PROPS, + icon: activeAlignmentControl + ? activeAlignmentControl.icon + : defaultAlignmentControl.icon, + label: __( 'Align' ), + toggleProps: { describedBy: __( 'Change alignment' ) }, + }; + const extraProps = isToolbar + ? { + isCollapsed, + controls: enabledControls.map( ( control ) => { + return { + ...BLOCK_ALIGNMENTS_CONTROLS[ control ], + isActive: value === control, + role: isCollapsed ? 'menuitemradio' : undefined, + onClick: applyOrUnset( control ), + }; + } ), + } + : { + children: () => { + return ( + <> + + { enabledControls.map( ( control ) => { + const { + icon, + title, + } = BLOCK_ALIGNMENTS_CONTROLS[ control ]; + // check when `undefined` to select `none`.. + const isSelected = + control === value || + ( ! value && control === 'none' ); + + return ( + + { title } + + ); + } ) } + + { ! wideAlignmentsSupport &&
{ help }
} + + ); + }, + }; - return ( - { - return { - ...BLOCK_ALIGNMENTS_CONTROLS[ control ], - isActive: value === control, - role: isCollapsed ? 'menuitemradio' : undefined, - onClick: applyOrUnset( control ), - }; - } ) } - { ...extraProps } - /> - ); + return ; } export default BlockAlignmentUI; diff --git a/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js b/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js index 6f420f0ca8ca92..882d973d7f24f1 100644 --- a/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js +++ b/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js @@ -30,14 +30,18 @@ export default function useAvailableAlignments( controls = DEFAULT_CONTROLS ) { const layoutAlignments = layoutType.getAlignments( layout ); if ( themeSupportsLayout ) { - return layoutAlignments.filter( ( control ) => - controls.includes( control ) - ); + return { + enabledControls: layoutAlignments.filter( ( control ) => + controls.includes( control ) + ), + layout, + wideAlignmentsSupport: true, + }; } // Starting here, it's the fallback for themes not supporting the layout config. if ( layoutType.name !== 'default' ) { - return []; + return { enabledControls: [] }; } const { alignments: availableAlignments = DEFAULT_CONTROLS } = layout; const enabledControls = controls.filter( @@ -48,5 +52,10 @@ export default function useAvailableAlignments( controls = DEFAULT_CONTROLS ) { availableAlignments.includes( control ) ); - return enabledControls; + return { + enabledControls, + layout, + // TODO check if this is the check we need to show the `doesn't support wide alignments` text. + wideAlignmentsSupport: layout.alignments || wideControlsEnabled, + }; } diff --git a/packages/block-editor/src/hooks/align.js b/packages/block-editor/src/hooks/align.js index eb46933b8a0ed8..6fb4004c4a8ced 100644 --- a/packages/block-editor/src/hooks/align.js +++ b/packages/block-editor/src/hooks/align.js @@ -173,7 +173,7 @@ export const withDataAlign = createHigherOrderComponent( getBlockSupport( name, 'align' ), hasBlockSupport( name, 'alignWide', true ) ); - const validAlignments = useAvailableAlignments( + const { enabledControls: validAlignments } = useAvailableAlignments( blockAllowedAlignments ); diff --git a/packages/block-editor/src/style.scss b/packages/block-editor/src/style.scss index ebc5481a281166..22a48857359ac7 100644 --- a/packages/block-editor/src/style.scss +++ b/packages/block-editor/src/style.scss @@ -1,4 +1,5 @@ @import "./autocompleters/style.scss"; +@import "./components/block-alignment-control/style.scss"; @import "./components/block-alignment-matrix-control/style.scss"; @import "./components/block-icon/style.scss"; @import "./components/block-inspector/style.scss"; diff --git a/packages/components/src/menu-item/README.md b/packages/components/src/menu-item/README.md index 94087548619613..9df572b9fccabd 100644 --- a/packages/components/src/menu-item/README.md +++ b/packages/components/src/menu-item/README.md @@ -50,6 +50,14 @@ Refer to documentation for [`label`](#label). Refer to documentation for [Button's `icon` prop](/packages/components/src/icon-button/README.md#icon). +### `iconPosition` + +- Type: `string` +- Required: No +- Default: `'right'` + +Determines where to display the provided `icon`. + ### `isSelected` - Type: `boolean` diff --git a/packages/components/src/menu-item/index.js b/packages/components/src/menu-item/index.js index f8a97b801b0a02..2d89af5ce8d941 100644 --- a/packages/components/src/menu-item/index.js +++ b/packages/components/src/menu-item/index.js @@ -23,6 +23,7 @@ export function MenuItem( props, ref ) { info, className, icon, + iconPosition = 'right', shortcut, isSelected, role = 'menuitem', @@ -42,7 +43,9 @@ export function MenuItem( props, ref ) { if ( icon && ! isString( icon ) ) { icon = cloneElement( icon, { - className: 'components-menu-items__item-icon', + className: classnames( 'components-menu-items__item-icon', { + 'has-icon-right': iconPosition === 'right', + } ), } ); } @@ -56,6 +59,7 @@ export function MenuItem( props, ref ) { : undefined } role={ role } + icon={ iconPosition === 'left' && icon } className={ className } { ...buttonProps } > @@ -64,7 +68,7 @@ export function MenuItem( props, ref ) { className="components-menu-item__shortcut" shortcut={ shortcut } /> - { icon && } + { icon && iconPosition === 'right' && } ); } diff --git a/packages/components/src/menu-item/style.scss b/packages/components/src/menu-item/style.scss index 69e5c6f50fd7e4..5c88db241f4e01 100644 --- a/packages/components/src/menu-item/style.scss +++ b/packages/components/src/menu-item/style.scss @@ -12,13 +12,15 @@ } .components-menu-items__item-icon { - margin-right: -2px; // This optically balances the icon. - margin-left: $grid-unit-30; display: inline-block; flex: 0 0 auto; + &.has-icon-right { + margin-right: -2px; // This optically balances the icon. + margin-left: $grid-unit-30; + } } - .components-menu-item__shortcut + .components-menu-items__item-icon { + .components-menu-item__shortcut + .components-menu-items__item-icon.has-icon-right { margin-left: $grid-unit-10; } From 2b9501df4b6a93c495646086720adfd8b5ecfe99 Mon Sep 17 00:00:00 2001 From: Nik Tsekouras Date: Fri, 10 Sep 2021 16:07:08 +0300 Subject: [PATCH 02/12] Update packages/block-editor/src/components/block-alignment-control/ui.js Co-authored-by: Matias Ventura --- .../block-editor/src/components/block-alignment-control/ui.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/block-alignment-control/ui.js b/packages/block-editor/src/components/block-alignment-control/ui.js index cb1dad534580df..a30cf36442c35d 100644 --- a/packages/block-editor/src/components/block-alignment-control/ui.js +++ b/packages/block-editor/src/components/block-alignment-control/ui.js @@ -54,7 +54,7 @@ const BLOCK_ALIGNMENTS_CONTROLS = { }; const DEFAULT_CONTROL = 'none'; -const help = __( 'Your theme does not support wide alignments' ); +const help = __( 'The theme does not support wider alignments.' ); const POPOVER_PROPS = { isAlternate: true, From edba0c81b6620dea4b58b30d596680df16a81a1a Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Fri, 10 Sep 2021 16:26:45 +0300 Subject: [PATCH 03/12] Add the `none` icon and update the previous `center` icon --- .../src/components/block-alignment-control/ui.js | 5 +++-- packages/icons/src/index.js | 1 + packages/icons/src/library/align-none.js | 12 ++++++++++++ packages/icons/src/library/position-center.js | 2 +- 4 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 packages/icons/src/library/align-none.js diff --git a/packages/block-editor/src/components/block-alignment-control/ui.js b/packages/block-editor/src/components/block-alignment-control/ui.js index a30cf36442c35d..53be4fabb204c3 100644 --- a/packages/block-editor/src/components/block-alignment-control/ui.js +++ b/packages/block-editor/src/components/block-alignment-control/ui.js @@ -9,6 +9,7 @@ import { MenuItem, } from '@wordpress/components'; import { + alignNone, positionCenter, positionLeft, positionRight, @@ -28,7 +29,7 @@ import classNames from 'classnames'; const BLOCK_ALIGNMENTS_CONTROLS = { none: { - icon: positionCenter, + icon: alignNone, title: __( 'None' ), }, left: { @@ -135,7 +136,7 @@ function BlockAlignmentUI( { icon, title, } = BLOCK_ALIGNMENTS_CONTROLS[ control ]; - // check when `undefined` to select `none`.. + // If no value is provided, mark as selected the `none` option. const isSelected = control === value || ( ! value && control === 'none' ); diff --git a/packages/icons/src/index.js b/packages/icons/src/index.js index 78c3aa2559a94e..9b4164b0db305f 100644 --- a/packages/icons/src/index.js +++ b/packages/icons/src/index.js @@ -6,6 +6,7 @@ export { default as alignCenter } from './library/align-center'; export { default as alignJustify } from './library/align-justify'; export { default as alignJustifyAlt } from './library/align-justify-alt'; export { default as alignLeft } from './library/align-left'; +export { default as alignNone } from './library/align-none'; export { default as alignRight } from './library/align-right'; export { default as archive } from './library/archive'; export { default as archiveTitle } from './library/archive-title'; diff --git a/packages/icons/src/library/align-none.js b/packages/icons/src/library/align-none.js new file mode 100644 index 00000000000000..0948cd8d943000 --- /dev/null +++ b/packages/icons/src/library/align-none.js @@ -0,0 +1,12 @@ +/** + * WordPress dependencies + */ +import { SVG, Path } from '@wordpress/primitives'; + +const alignNone = ( + + + +); + +export default alignNone; diff --git a/packages/icons/src/library/position-center.js b/packages/icons/src/library/position-center.js index 34c272a3738b76..7c9686535ce65a 100644 --- a/packages/icons/src/library/position-center.js +++ b/packages/icons/src/library/position-center.js @@ -5,7 +5,7 @@ import { SVG, Path } from '@wordpress/primitives'; const positionCenter = ( - + ); From f876535d123a5bfbd0d60a2ede256fafac0c0213 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Tue, 14 Sep 2021 15:56:35 +0300 Subject: [PATCH 04/12] show info only when it's not a css var + remove toggling state in toolbar --- .../src/components/block-alignment-control/ui.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/block-alignment-control/ui.js b/packages/block-editor/src/components/block-alignment-control/ui.js index 53be4fabb204c3..7f5699e941e7f2 100644 --- a/packages/block-editor/src/components/block-alignment-control/ui.js +++ b/packages/block-editor/src/components/block-alignment-control/ui.js @@ -79,11 +79,23 @@ function BlockAlignmentUI( { return; } const info = {}; - if ( !! contentSize ) { + /** + * Besides checking if `contentSize` and `wideSize` have a + * value, we now show this information only if their values + * are not a `css var`. This needs to change when parsing + * css variables land. + * + * @see https://github.com/WordPress/gutenberg/pull/34710#issuecomment-918000752 + */ + if ( !! contentSize && ! contentSize?.startsWith( 'var' ) ) { // translators: %s: container size (i.e. 600px etc) info.none = sprintf( __( 'Max %s wide' ), contentSize ); } - if ( wideAlignmentsSupport && !! wideSize ) { + if ( + wideAlignmentsSupport && + !! wideSize && + ! wideSize?.startsWith( 'var' ) + ) { // translators: %s: container size (i.e. 600px etc) info.wide = sprintf( __( 'Max %s wide' ), wideSize ); } From f9250cf5efaa1688c25a8f3120f65f9ce4d41214 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Tue, 14 Sep 2021 16:02:21 +0300 Subject: [PATCH 05/12] update snapshots --- .../test/__snapshots__/index.js.snap | 16 +++++++++++++++- packages/components/src/menu-item/index.js | 2 +- .../menu-item/test/__snapshots__/index.js.snap | 4 ++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/block-alignment-control/test/__snapshots__/index.js.snap b/packages/block-editor/src/components/block-alignment-control/test/__snapshots__/index.js.snap index bf43a991115033..17f5318b43669c 100644 --- a/packages/block-editor/src/components/block-alignment-control/test/__snapshots__/index.js.snap +++ b/packages/block-editor/src/components/block-alignment-control/test/__snapshots__/index.js.snap @@ -4,6 +4,20 @@ exports[`BlockAlignmentUI should match snapshot 1`] = ` + + , + "isActive": false, + "onClick": [Function], + "role": "menuitemradio", + "title": "None", + }, Object { "icon": , "isActive": false, diff --git a/packages/components/src/menu-item/index.js b/packages/components/src/menu-item/index.js index 2d89af5ce8d941..79f129f8caa8e0 100644 --- a/packages/components/src/menu-item/index.js +++ b/packages/components/src/menu-item/index.js @@ -59,7 +59,7 @@ export function MenuItem( props, ref ) { : undefined } role={ role } - icon={ iconPosition === 'left' && icon } + icon={ iconPosition === 'left' ? icon : undefined } className={ className } { ...buttonProps } > diff --git a/packages/components/src/menu-item/test/__snapshots__/index.js.snap b/packages/components/src/menu-item/test/__snapshots__/index.js.snap index 2349726c1d07a5..a546e6078a79f4 100644 --- a/packages/components/src/menu-item/test/__snapshots__/index.js.snap +++ b/packages/components/src/menu-item/test/__snapshots__/index.js.snap @@ -19,7 +19,7 @@ exports[`MenuItem should match snapshot when all props provided 1`] = ` @@ -79,7 +79,7 @@ exports[`MenuItem should match snapshot when isSelected and role are optionally From 46e789c8d092cf15d02813148293d5ce159adacf Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Tue, 14 Sep 2021 17:25:00 +0300 Subject: [PATCH 06/12] fix e2e tests --- .../e2e-tests/specs/editor/various/block-grouping.test.js | 4 ++-- .../e2e-tests/specs/editor/various/writing-flow.test.js | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/block-grouping.test.js b/packages/e2e-tests/specs/editor/various/block-grouping.test.js index b75718fbc24d75..28043b4203c0e5 100644 --- a/packages/e2e-tests/specs/editor/various/block-grouping.test.js +++ b/packages/e2e-tests/specs/editor/various/block-grouping.test.js @@ -163,7 +163,7 @@ describe( 'Block Grouping', () => { await insertBlock( 'Image' ); await clickBlockToolbarButton( 'Align' ); const fullButton = await page.waitForXPath( - `//button[contains(@class,'components-dropdown-menu__menu-item') and contains(text(), 'Full width')]` + `//button[contains(@class,'components-dropdown-menu__menu-item')]//span[contains(text(), 'Full width')]` ); await fullButton.evaluate( ( element ) => element.scrollIntoView() @@ -174,7 +174,7 @@ describe( 'Block Grouping', () => { await insertBlock( 'Image' ); await clickBlockToolbarButton( 'Align' ); const wideButton = await page.waitForXPath( - `//button[contains(@class,'components-dropdown-menu__menu-item') and contains(text(), 'Wide width')]` + `//button[contains(@class,'components-dropdown-menu__menu-item')]//span[contains(text(), 'Wide width')]` ); await wideButton.evaluate( ( element ) => element.scrollIntoView() diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index 89885dcfca746a..39c72c866e9d47 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -9,7 +9,6 @@ import { pressKeyWithModifier, insertBlock, clickBlockToolbarButton, - clickButton, } from '@wordpress/e2e-test-utils'; const getActiveBlockName = async () => @@ -556,7 +555,10 @@ describe( 'Writing Flow', () => { await page.keyboard.type( '/image' ); await page.keyboard.press( 'Enter' ); await clickBlockToolbarButton( 'Align' ); - await clickButton( 'Wide width' ); + const wideButton = await page.waitForXPath( + `//button[contains(@class,'components-dropdown-menu__menu-item')]//span[contains(text(), 'Wide width')]` + ); + await wideButton.click(); // Select the previous block. await page.keyboard.press( 'ArrowUp' ); From a3932db1870cc7408f4360cc44408993cacd07b2 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Wed, 15 Sep 2021 12:43:04 +0300 Subject: [PATCH 07/12] update tests --- .../components/block-alignment-control/ui.js | 14 +- .../specs/editor/plugins/align-hook.test.js | 221 +++++++++--------- 2 files changed, 124 insertions(+), 111 deletions(-) diff --git a/packages/block-editor/src/components/block-alignment-control/ui.js b/packages/block-editor/src/components/block-alignment-control/ui.js index 7f5699e941e7f2..579ee5b0e9f7d6 100644 --- a/packages/block-editor/src/components/block-alignment-control/ui.js +++ b/packages/block-editor/src/components/block-alignment-control/ui.js @@ -108,9 +108,8 @@ function BlockAlignmentUI( { // Always add the `none` option. enabledControls.unshift( 'none' ); - function applyOrUnset( align ) { - return () => - onChange( [ value, 'none' ].includes( align ) ? undefined : align ); + function onChangeAlignment( align ) { + onChange( [ value, 'none' ].includes( align ) ? undefined : align ); } const activeAlignmentControl = BLOCK_ALIGNMENTS_CONTROLS[ value ]; @@ -134,12 +133,12 @@ function BlockAlignmentUI( { ...BLOCK_ALIGNMENTS_CONTROLS[ control ], isActive: value === control, role: isCollapsed ? 'menuitemradio' : undefined, - onClick: applyOrUnset( control ), + onClick: () => onChangeAlignment( control ), }; } ), } : { - children: () => { + children: ( { onClose } ) => { return ( <> @@ -165,7 +164,10 @@ function BlockAlignmentUI( { } ) } isSelected={ isSelected } - onClick={ applyOrUnset( control ) } + onClick={ () => { + onChangeAlignment( control ); + onClose(); + } } role="menuitemradio" info={ alignmentsInfo[ control ] } > diff --git a/packages/e2e-tests/specs/editor/plugins/align-hook.test.js b/packages/e2e-tests/specs/editor/plugins/align-hook.test.js index 59b10cca428100..abf0e5912d2524 100644 --- a/packages/e2e-tests/specs/editor/plugins/align-hook.test.js +++ b/packages/e2e-tests/specs/editor/plugins/align-hook.test.js @@ -14,6 +14,7 @@ import { } from '@wordpress/e2e-test-utils'; const alignLabels = { + none: 'None', left: 'Align left', center: 'Align center', right: 'Align right', @@ -21,114 +22,129 @@ const alignLabels = { full: 'Full width', }; -describe( 'Align Hook Works As Expected', () => { - beforeAll( async () => { - await activatePlugin( 'gutenberg-test-align-hook' ); +/** + * Helper function to get the `labels` of align control. It actually evaluates the + * basic label of the button without the `info` text node if exists. + * + * @param {Object} options Options for the util function + * @param {boolean} [options.getActiveButtonLabels=false] Flag for returning the active buttons labels only. + * @return {string[]} The matched labels. + */ +const getAlignmentToolbarLabels = async ( { + getActiveButtonLabels = false, +} = {} ) => { + const selector = `.components-dropdown-menu__menu button${ + getActiveButtonLabels ? '.is-active' : '' + } .components-menu-item__item`; + return page.evaluate( ( _selector ) => { + return ( + Array.from( document.querySelectorAll( _selector ) ) + /** + * We neede this for now because conditionally there could be two nodes + * with the same class(). This should be removed when the following + * issue is resolved. + * + * @see https://github.com/WordPress/gutenberg/issues/34838 + */ + .filter( ( contentNode ) => ! contentNode.childElementCount ) + .map( ( contentNode ) => { + return contentNode.innerText; + } ) + ); + }, selector ); +}; + +const expectActiveButtonLabelToBe = async ( expected ) => { + await clickBlockToolbarButton( 'Align' ); + const activeButtonLabels = await getAlignmentToolbarLabels( { + getActiveButtonLabels: true, } ); + expect( activeButtonLabels ).toHaveLength( 1 ); + expect( activeButtonLabels[ 0 ] ).toEqual( expected ); +}; - beforeEach( async () => { - await createNewPost(); +const createShowsTheExpectedButtonsTest = ( blockName, buttonLabels ) => { + it( 'Shows the expected buttons on the alignment toolbar', async () => { + await insertBlock( blockName ); + await clickBlockToolbarButton( 'Align' ); + expect( await getAlignmentToolbarLabels() ).toEqual( + expect.arrayContaining( buttonLabels ) + ); } ); +}; - afterAll( async () => { - await deactivatePlugin( 'gutenberg-test-align-hook' ); +const createAppliesNoneAlignmentByDefaultTest = ( blockName ) => { + it( 'applies none alignment by default', async () => { + await insertBlock( blockName ); + await expectActiveButtonLabelToBe( alignLabels.none ); } ); +}; + +const verifyMarkupIsValid = async ( htmlMarkup ) => { + await setPostContent( htmlMarkup ); + const blocks = await getAllBlocks(); + expect( blocks ).toHaveLength( 1 ); + expect( blocks[ 0 ].isValid ).toBeTruthy(); +}; + +const createCorrectlyAppliesAndRemovesAlignmentTest = ( + blockName, + alignment +) => { + it( 'Correctly applies the selected alignment and correctly removes the alignment', async () => { + const BUTTON_XPATH = `//button[contains(@class,'components-dropdown-menu__menu-item')]//span[contains(text(), '${ alignLabels[ alignment ] }')]`; - const getAlignmentToolbarLabels = async () => { + // set the specified alignment. + await insertBlock( blockName ); await clickBlockToolbarButton( 'Align' ); - const buttonLabels = await page.evaluate( () => { - return Array.from( - document.querySelectorAll( - '.components-dropdown-menu__menu button' - ) - ).map( ( button ) => { - return button.innerText; - } ); - } ); - return buttonLabels; - }; + await ( await page.$x( BUTTON_XPATH ) )[ 0 ].click(); - const createShowsTheExpectedButtonsTest = ( blockName, buttonLabels ) => { - it( 'Shows the expected buttons on the alignment toolbar', async () => { - await insertBlock( blockName ); - expect( await getAlignmentToolbarLabels() ).toEqual( buttonLabels ); - } ); - }; + // verify the button of the specified alignment is pressed. + await expectActiveButtonLabelToBe( alignLabels[ alignment ] ); - const createDoesNotApplyAlignmentByDefaultTest = ( blockName ) => { - it( 'Does not apply any alignment by default', async () => { - await insertBlock( blockName ); - await clickBlockToolbarButton( 'Align' ); - const pressedButtons = await page.$$( - '.components-dropdown-menu__menu button.is-active' - ); - expect( pressedButtons ).toHaveLength( 0 ); - } ); - }; - - const verifyMarkupIsValid = async ( htmlMarkup ) => { - await setPostContent( htmlMarkup ); - const blocks = await getAllBlocks(); - expect( blocks ).toHaveLength( 1 ); - expect( blocks[ 0 ].isValid ).toBeTruthy(); - }; - - const createCorrectlyAppliesAndRemovesAlignmentTest = ( - blockName, - alignment - ) => { - it( 'Correctly applies the selected alignment and correctly removes the alignment', async () => { - const BUTTON_XPATH = `//button[contains(@class,'components-dropdown-menu__menu-item') and contains(text(), '${ alignLabels[ alignment ] }')]`; - const BUTTON_PRESSED_SELECTOR = - '.components-dropdown-menu__menu button.is-active'; - // set the specified alignment. - await insertBlock( blockName ); - await clickBlockToolbarButton( 'Align' ); - await ( await page.$x( BUTTON_XPATH ) )[ 0 ].click(); + let htmlMarkup = await getEditedPostContent(); - // verify the button of the specified alignment is pressed. - await clickBlockToolbarButton( 'Align' ); - let pressedButtons = await page.$$( BUTTON_PRESSED_SELECTOR ); - expect( pressedButtons ).toHaveLength( 1 ); + // verify the markup of the selected alignment was generated. + expect( htmlMarkup ).toMatchSnapshot(); - let htmlMarkup = await getEditedPostContent(); + // verify the markup can be correctly parsed + await verifyMarkupIsValid( htmlMarkup ); - // verify the markup of the selected alignment was generated. - expect( htmlMarkup ).toMatchSnapshot(); + await selectBlockByClientId( ( await getAllBlocks() )[ 0 ].clientId ); - // verify the markup can be correctly parsed - await verifyMarkupIsValid( htmlMarkup ); + // remove the alignment. + await clickBlockToolbarButton( 'Align' ); + await ( await page.$x( BUTTON_XPATH ) )[ 0 ].click(); - await selectBlockByClientId( - ( await getAllBlocks() )[ 0 ].clientId - ); + // verify 'none' alignment button is in pressed state. + await expectActiveButtonLabelToBe( alignLabels.none ); - // remove the alignment. - await clickBlockToolbarButton( 'Align' ); - await ( await page.$x( BUTTON_XPATH ) )[ 0 ].click(); + // verify alignment markup was removed from the block. + htmlMarkup = await getEditedPostContent(); + expect( htmlMarkup ).toMatchSnapshot(); - // verify no alignment button is in pressed state. - await clickBlockToolbarButton( 'Align' ); - pressedButtons = await page.$$( BUTTON_PRESSED_SELECTOR ); - expect( pressedButtons ).toHaveLength( 0 ); + // verify the markup when no alignment is set is valid + await verifyMarkupIsValid( htmlMarkup ); - // verify alignment markup was removed from the block. - htmlMarkup = await getEditedPostContent(); - expect( htmlMarkup ).toMatchSnapshot(); + await selectBlockByClientId( ( await getAllBlocks() )[ 0 ].clientId ); - // verify the markup when no alignment is set is valid - await verifyMarkupIsValid( htmlMarkup ); + // verify alignment `none` button is in pressed state after parsing the block. + await expectActiveButtonLabelToBe( alignLabels.none ); + } ); +}; - await selectBlockByClientId( - ( await getAllBlocks() )[ 0 ].clientId - ); +describe( 'Align Hook Works As Expected', () => { + beforeAll( async () => { + await activatePlugin( 'gutenberg-test-align-hook' ); + } ); - // verify no alignment button is in pressed state after parsing the block. - await clickBlockToolbarButton( 'Align' ); - pressedButtons = await page.$$( BUTTON_PRESSED_SELECTOR ); - expect( pressedButtons ).toHaveLength( 0 ); - } ); - }; + beforeEach( async () => { + await createNewPost(); + } ); + + afterAll( async () => { + await deactivatePlugin( 'gutenberg-test-align-hook' ); + } ); describe( 'Block with no alignment set', () => { const BLOCK_NAME = 'Test No Alignment Set'; @@ -151,15 +167,12 @@ describe( 'Align Hook Works As Expected', () => { describe( 'Block with align true', () => { const BLOCK_NAME = 'Test Align True'; - createShowsTheExpectedButtonsTest( BLOCK_NAME, [ - alignLabels.left, - alignLabels.center, - alignLabels.right, - alignLabels.wide, - alignLabels.full, - ] ); + createShowsTheExpectedButtonsTest( + BLOCK_NAME, + Object.values( alignLabels ) + ); - createDoesNotApplyAlignmentByDefaultTest( BLOCK_NAME ); + createAppliesNoneAlignmentByDefaultTest( BLOCK_NAME ); createCorrectlyAppliesAndRemovesAlignmentTest( BLOCK_NAME, 'right' ); } ); @@ -168,11 +181,12 @@ describe( 'Align Hook Works As Expected', () => { const BLOCK_NAME = 'Test Align Array'; createShowsTheExpectedButtonsTest( BLOCK_NAME, [ + alignLabels.none, alignLabels.left, alignLabels.center, ] ); - createDoesNotApplyAlignmentByDefaultTest( BLOCK_NAME ); + createAppliesNoneAlignmentByDefaultTest( BLOCK_NAME ); createCorrectlyAppliesAndRemovesAlignmentTest( BLOCK_NAME, 'center' ); } ); @@ -180,14 +194,11 @@ describe( 'Align Hook Works As Expected', () => { describe( 'Block with default align', () => { const BLOCK_NAME = 'Test Default Align'; const SELECTED_ALIGNMENT_CONTROL_SELECTOR = - '//div[contains(@class, "components-dropdown-menu__menu")]//button[contains(@class, "is-active")][text()="Align right"]'; - createShowsTheExpectedButtonsTest( BLOCK_NAME, [ - alignLabels.left, - alignLabels.center, - alignLabels.right, - alignLabels.wide, - alignLabels.full, - ] ); + '//div[contains(@class, "components-dropdown-menu__menu")]//button[contains(@class, "is-active")]/span[text()="Align right"]'; + createShowsTheExpectedButtonsTest( + BLOCK_NAME, + Object.values( alignLabels ) + ); it( 'Applies the selected alignment by default', async () => { await insertBlock( BLOCK_NAME ); From a3a8722730071ec31e050953ce2ee6ee4ffa8348 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 23 Sep 2021 15:56:23 +0300 Subject: [PATCH 08/12] style and reword the no wider alignments help text --- .../src/components/block-alignment-control/style.scss | 3 +++ .../src/components/block-alignment-control/ui.js | 8 ++++++-- .../block-alignment-control/use-available-alignments.js | 1 - 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/block-alignment-control/style.scss b/packages/block-editor/src/components/block-alignment-control/style.scss index 4897b18c1bcb39..1b85e5731a8ee0 100644 --- a/packages/block-editor/src/components/block-alignment-control/style.scss +++ b/packages/block-editor/src/components/block-alignment-control/style.scss @@ -3,3 +3,6 @@ margin-top: 0; } } +.block-editor-block-alignment-control__help { + padding: $grid-unit-10; +} diff --git a/packages/block-editor/src/components/block-alignment-control/ui.js b/packages/block-editor/src/components/block-alignment-control/ui.js index 579ee5b0e9f7d6..2be22bd6f7c62c 100644 --- a/packages/block-editor/src/components/block-alignment-control/ui.js +++ b/packages/block-editor/src/components/block-alignment-control/ui.js @@ -55,7 +55,7 @@ const BLOCK_ALIGNMENTS_CONTROLS = { }; const DEFAULT_CONTROL = 'none'; -const help = __( 'The theme does not support wider alignments.' ); +const help = __( 'Wider alignments are not available.' ); const POPOVER_PROPS = { isAlternate: true, @@ -176,7 +176,11 @@ function BlockAlignmentUI( { ); } ) } - { ! wideAlignmentsSupport &&
{ help }
} + { ! wideAlignmentsSupport && ( +
+ { help } +
+ ) } ); }, diff --git a/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js b/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js index 882d973d7f24f1..7cfadda9bbf963 100644 --- a/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js +++ b/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js @@ -55,7 +55,6 @@ export default function useAvailableAlignments( controls = DEFAULT_CONTROLS ) { return { enabledControls, layout, - // TODO check if this is the check we need to show the `doesn't support wide alignments` text. wideAlignmentsSupport: layout.alignments || wideControlsEnabled, }; } From ea1cc4de8448a836c58931f79778088991c2b636 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 23 Sep 2021 16:02:06 +0300 Subject: [PATCH 09/12] small fix --- .../block-editor/src/components/block-alignment-control/ui.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/block-alignment-control/ui.js b/packages/block-editor/src/components/block-alignment-control/ui.js index 2be22bd6f7c62c..10a1c02c549e4a 100644 --- a/packages/block-editor/src/components/block-alignment-control/ui.js +++ b/packages/block-editor/src/components/block-alignment-control/ui.js @@ -151,7 +151,6 @@ function BlockAlignmentUI( { const isSelected = control === value || ( ! value && control === 'none' ); - return ( { title } From 83eb078c2643d348b235bc68e9621a4d7a83ecc1 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Fri, 24 Sep 2021 11:06:19 +0300 Subject: [PATCH 10/12] alignment objects array --- .../components/block-alignment-control/ui.js | 136 ++++++++---------- .../use-available-alignments.js | 35 ++--- packages/block-editor/src/hooks/align.js | 6 +- packages/block-editor/src/layouts/flow.js | 43 +++++- 4 files changed, 115 insertions(+), 105 deletions(-) diff --git a/packages/block-editor/src/components/block-alignment-control/ui.js b/packages/block-editor/src/components/block-alignment-control/ui.js index 10a1c02c549e4a..c037dfaf49ed91 100644 --- a/packages/block-editor/src/components/block-alignment-control/ui.js +++ b/packages/block-editor/src/components/block-alignment-control/ui.js @@ -1,7 +1,12 @@ +/** + * External dependencies + */ +import classNames from 'classnames'; + /** * WordPress dependencies */ -import { __, sprintf } from '@wordpress/i18n'; +import { __ } from '@wordpress/i18n'; import { ToolbarDropdownMenu, ToolbarGroup, @@ -16,17 +21,13 @@ import { stretchFullWidth, stretchWide, } from '@wordpress/icons'; -import { useMemo } from '@wordpress/element'; /** * Internal dependencies */ import useAvailableAlignments from './use-available-alignments'; -/** - * External dependencies - */ -import classNames from 'classnames'; +const WIDE_CONTROLS = [ 'wide', 'full' ]; const BLOCK_ALIGNMENTS_CONTROLS = { none: { icon: alignNone, @@ -68,45 +69,19 @@ function BlockAlignmentUI( { isToolbar, isCollapsed = true, } ) { - const { - enabledControls, - layout: { contentSize, wideSize } = {}, - wideAlignmentsSupport, - } = useAvailableAlignments( controls ); + const enabledControls = useAvailableAlignments( controls ); const hasEnabledControls = !! enabledControls.length; - const alignmentsInfo = useMemo( () => { - if ( ! hasEnabledControls ) { - return; - } - const info = {}; - /** - * Besides checking if `contentSize` and `wideSize` have a - * value, we now show this information only if their values - * are not a `css var`. This needs to change when parsing - * css variables land. - * - * @see https://github.com/WordPress/gutenberg/pull/34710#issuecomment-918000752 - */ - if ( !! contentSize && ! contentSize?.startsWith( 'var' ) ) { - // translators: %s: container size (i.e. 600px etc) - info.none = sprintf( __( 'Max %s wide' ), contentSize ); - } - if ( - wideAlignmentsSupport && - !! wideSize && - ! wideSize?.startsWith( 'var' ) - ) { - // translators: %s: container size (i.e. 600px etc) - info.wide = sprintf( __( 'Max %s wide' ), wideSize ); - } - return info; - }, [ hasEnabledControls, contentSize, wideSize, wideAlignmentsSupport ] ); if ( ! hasEnabledControls ) { return null; } - // Always add the `none` option. - enabledControls.unshift( 'none' ); + const wideAlignmentsSupport = enabledControls.some( ( { name } ) => + WIDE_CONTROLS.includes( name ) + ); + // Always add the `none` option if not exists. + if ( ! enabledControls.some( ( { name } ) => name === 'none' ) ) { + enabledControls.unshift( { name: 'none' } ); + } function onChangeAlignment( align ) { onChange( [ value, 'none' ].includes( align ) ? undefined : align ); @@ -128,12 +103,14 @@ function BlockAlignmentUI( { const extraProps = isToolbar ? { isCollapsed, - controls: enabledControls.map( ( control ) => { + controls: enabledControls.map( ( { name: controlName } ) => { return { - ...BLOCK_ALIGNMENTS_CONTROLS[ control ], - isActive: value === control, + ...BLOCK_ALIGNMENTS_CONTROLS[ controlName ], + isActive: + value === controlName || + ( ! value && controlName === 'none' ), role: isCollapsed ? 'menuitemradio' : undefined, - onClick: () => onChangeAlignment( control ), + onClick: () => onChangeAlignment( controlName ), }; } ), } @@ -142,38 +119,45 @@ function BlockAlignmentUI( { return ( <> - { enabledControls.map( ( control ) => { - const { - icon, - title, - } = BLOCK_ALIGNMENTS_CONTROLS[ control ]; - // If no value is provided, mark as selected the `none` option. - const isSelected = - control === value || - ( ! value && control === 'none' ); - return ( - { - onChangeAlignment( control ); - onClose(); - } } - role="menuitemradio" - info={ alignmentsInfo?.[ control ] } - > - { title } - - ); - } ) } + { enabledControls.map( + ( { name: controlName, info } ) => { + const { + icon, + title, + } = BLOCK_ALIGNMENTS_CONTROLS[ + controlName + ]; + // If no value is provided, mark as selected the `none` option. + const isSelected = + controlName === value || + ( ! value && + controlName === 'none' ); + return ( + { + onChangeAlignment( + controlName + ); + onClose(); + } } + role="menuitemradio" + info={ info } + > + { title } + + ); + } + ) } { ! wideAlignmentsSupport && (
diff --git a/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js b/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js index 7cfadda9bbf963..68157271c0ef57 100644 --- a/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js +++ b/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js @@ -30,31 +30,26 @@ export default function useAvailableAlignments( controls = DEFAULT_CONTROLS ) { const layoutAlignments = layoutType.getAlignments( layout ); if ( themeSupportsLayout ) { - return { - enabledControls: layoutAlignments.filter( ( control ) => - controls.includes( control ) - ), - layout, - wideAlignmentsSupport: true, - }; + return layoutAlignments.filter( + ( { name: alignmentName } ) => + alignmentName === 'none' || controls.includes( alignmentName ) + ); } // Starting here, it's the fallback for themes not supporting the layout config. if ( layoutType.name !== 'default' ) { - return { enabledControls: [] }; + return []; } const { alignments: availableAlignments = DEFAULT_CONTROLS } = layout; - const enabledControls = controls.filter( - ( control ) => - ( layout.alignments || // Ignore the global wideAlignment check if the layout explicitely defines alignments. - wideControlsEnabled || - ! WIDE_CONTROLS.includes( control ) ) && - availableAlignments.includes( control ) - ); + const enabledControls = controls + .filter( + ( control ) => + ( layout.alignments || // Ignore the global wideAlignment check if the layout explicitely defines alignments. + wideControlsEnabled || + ! WIDE_CONTROLS.includes( control ) ) && + availableAlignments.includes( control ) + ) + .map( ( enabledControl ) => ( { name: enabledControl } ) ); - return { - enabledControls, - layout, - wideAlignmentsSupport: layout.alignments || wideControlsEnabled, - }; + return enabledControls; } diff --git a/packages/block-editor/src/hooks/align.js b/packages/block-editor/src/hooks/align.js index 6fb4004c4a8ced..cdb953fa92171d 100644 --- a/packages/block-editor/src/hooks/align.js +++ b/packages/block-editor/src/hooks/align.js @@ -173,7 +173,7 @@ export const withDataAlign = createHigherOrderComponent( getBlockSupport( name, 'align' ), hasBlockSupport( name, 'alignWide', true ) ); - const { enabledControls: validAlignments } = useAvailableAlignments( + const validAlignments = useAvailableAlignments( blockAllowedAlignments ); @@ -184,7 +184,9 @@ export const withDataAlign = createHigherOrderComponent( } let wrapperProps = props.wrapperProps; - if ( validAlignments.includes( align ) ) { + if ( + validAlignments.some( ( alignment ) => alignment.name === align ) + ) { wrapperProps = { ...wrapperProps, 'data-align': align }; } diff --git a/packages/block-editor/src/layouts/flow.js b/packages/block-editor/src/layouts/flow.js index 796dad1d78a18f..aa7e3d68b1b4a8 100644 --- a/packages/block-editor/src/layouts/flow.js +++ b/packages/block-editor/src/layouts/flow.js @@ -6,7 +6,7 @@ import { __experimentalUseCustomUnits as useCustomUnits, __experimentalUnitControl as UnitControl, } from '@wordpress/components'; -import { __ } from '@wordpress/i18n'; +import { __, sprintf } from '@wordpress/i18n'; import { Icon, positionCenter, stretchWide } from '@wordpress/icons'; /** @@ -158,17 +158,46 @@ export default { }, getAlignments( layout ) { if ( layout.alignments !== undefined ) { - return layout.alignments; + return layout.alignments.map( ( alignment ) => ( { + name: alignment, + } ) ); } + const { contentSize, wideSize } = layout; + + const alignments = [ + { name: 'left' }, + { name: 'center' }, + { name: 'right' }, + ]; - const alignments = [ 'left', 'center', 'right' ]; + if ( contentSize ) { + alignments.unshift( { name: 'full' } ); + } - if ( layout.contentSize ) { - alignments.unshift( 'full' ); + /** + * Besides checking if `contentSize` and `wideSize` have a + * value, we now show this information only if their values + * are not a `css var`. This needs to change when parsing + * css variables land. + * + * @see https://github.com/WordPress/gutenberg/pull/34710#issuecomment-918000752 + */ + if ( wideSize ) { + const wideAlignment = { name: 'wide' }; + if ( ! wideSize?.startsWith( 'var' ) ) { + // translators: %s: container size (i.e. 600px etc) + wideAlignment.info = sprintf( __( 'Max %s wide' ), wideSize ); + } + alignments.unshift( wideAlignment ); } - if ( layout.wideSize ) { - alignments.unshift( 'wide' ); + // Add `none` alignment with info text. + if ( contentSize && ! contentSize?.startsWith( 'var' ) ) { + alignments.unshift( { + name: 'none', + // translators: %s: container size (i.e. 600px etc) + info: sprintf( __( 'Max %s wide' ), contentSize ), + } ); } return alignments; From 292ead1e3d9ce3646056c34a42ea1f937ff72c3d Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Fri, 24 Sep 2021 15:48:03 +0300 Subject: [PATCH 11/12] address feedback --- .../block-alignment-control/style.scss | 3 - .../components/block-alignment-control/ui.js | 14 ----- .../use-available-alignments.js | 11 ++-- packages/block-editor/src/layouts/flow.js | 57 ++++++++++++------- 4 files changed, 42 insertions(+), 43 deletions(-) diff --git a/packages/block-editor/src/components/block-alignment-control/style.scss b/packages/block-editor/src/components/block-alignment-control/style.scss index 1b85e5731a8ee0..4897b18c1bcb39 100644 --- a/packages/block-editor/src/components/block-alignment-control/style.scss +++ b/packages/block-editor/src/components/block-alignment-control/style.scss @@ -3,6 +3,3 @@ margin-top: 0; } } -.block-editor-block-alignment-control__help { - padding: $grid-unit-10; -} diff --git a/packages/block-editor/src/components/block-alignment-control/ui.js b/packages/block-editor/src/components/block-alignment-control/ui.js index c037dfaf49ed91..717367a16bc4cc 100644 --- a/packages/block-editor/src/components/block-alignment-control/ui.js +++ b/packages/block-editor/src/components/block-alignment-control/ui.js @@ -27,7 +27,6 @@ import { */ import useAvailableAlignments from './use-available-alignments'; -const WIDE_CONTROLS = [ 'wide', 'full' ]; const BLOCK_ALIGNMENTS_CONTROLS = { none: { icon: alignNone, @@ -56,7 +55,6 @@ const BLOCK_ALIGNMENTS_CONTROLS = { }; const DEFAULT_CONTROL = 'none'; -const help = __( 'Wider alignments are not available.' ); const POPOVER_PROPS = { isAlternate: true, @@ -75,13 +73,6 @@ function BlockAlignmentUI( { if ( ! hasEnabledControls ) { return null; } - const wideAlignmentsSupport = enabledControls.some( ( { name } ) => - WIDE_CONTROLS.includes( name ) - ); - // Always add the `none` option if not exists. - if ( ! enabledControls.some( ( { name } ) => name === 'none' ) ) { - enabledControls.unshift( { name: 'none' } ); - } function onChangeAlignment( align ) { onChange( [ value, 'none' ].includes( align ) ? undefined : align ); @@ -159,11 +150,6 @@ function BlockAlignmentUI( { } ) } - { ! wideAlignmentsSupport && ( -
- { help } -
- ) } ); }, diff --git a/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js b/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js index 68157271c0ef57..ad05b889a578c0 100644 --- a/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js +++ b/packages/block-editor/src/components/block-alignment-control/use-available-alignments.js @@ -10,10 +10,14 @@ import { useLayout } from '../block-list/layout'; import { store as blockEditorStore } from '../../store'; import { getLayoutType } from '../../layouts'; -const DEFAULT_CONTROLS = [ 'left', 'center', 'right', 'wide', 'full' ]; +const DEFAULT_CONTROLS = [ 'none', 'left', 'center', 'right', 'wide', 'full' ]; const WIDE_CONTROLS = [ 'wide', 'full' ]; export default function useAvailableAlignments( controls = DEFAULT_CONTROLS ) { + // Always add the `none` option if not exists. + if ( ! controls.includes( 'none' ) ) { + controls.unshift( 'none' ); + } const { wideControlsEnabled = false, themeSupportsLayout } = useSelect( ( select ) => { const { getSettings } = select( blockEditorStore ); @@ -30,9 +34,8 @@ export default function useAvailableAlignments( controls = DEFAULT_CONTROLS ) { const layoutAlignments = layoutType.getAlignments( layout ); if ( themeSupportsLayout ) { - return layoutAlignments.filter( - ( { name: alignmentName } ) => - alignmentName === 'none' || controls.includes( alignmentName ) + return layoutAlignments.filter( ( { name: alignmentName } ) => + controls.includes( alignmentName ) ); } diff --git a/packages/block-editor/src/layouts/flow.js b/packages/block-editor/src/layouts/flow.js index aa7e3d68b1b4a8..cbf623e04fdd8d 100644 --- a/packages/block-editor/src/layouts/flow.js +++ b/packages/block-editor/src/layouts/flow.js @@ -157,9 +157,14 @@ export default { return 'vertical'; }, getAlignments( layout ) { + const alignmentInfo = getAlignmentsInfo( layout ); if ( layout.alignments !== undefined ) { + if ( ! layout.alignments.includes( 'none' ) ) { + layout.alignments.unshift( 'none' ); + } return layout.alignments.map( ( alignment ) => ( { name: alignment, + info: alignmentInfo[ alignment ], } ) ); } const { contentSize, wideSize } = layout; @@ -174,32 +179,40 @@ export default { alignments.unshift( { name: 'full' } ); } - /** - * Besides checking if `contentSize` and `wideSize` have a - * value, we now show this information only if their values - * are not a `css var`. This needs to change when parsing - * css variables land. - * - * @see https://github.com/WordPress/gutenberg/pull/34710#issuecomment-918000752 - */ if ( wideSize ) { - const wideAlignment = { name: 'wide' }; - if ( ! wideSize?.startsWith( 'var' ) ) { - // translators: %s: container size (i.e. 600px etc) - wideAlignment.info = sprintf( __( 'Max %s wide' ), wideSize ); - } - alignments.unshift( wideAlignment ); + alignments.unshift( { name: 'wide', info: alignmentInfo.wide } ); } - // Add `none` alignment with info text. - if ( contentSize && ! contentSize?.startsWith( 'var' ) ) { - alignments.unshift( { - name: 'none', - // translators: %s: container size (i.e. 600px etc) - info: sprintf( __( 'Max %s wide' ), contentSize ), - } ); - } + alignments.unshift( { name: 'none', info: alignmentInfo.none } ); return alignments; }, }; + +/** + * Helper method to assign contextual info to clarify + * alignment settings. + * + * Besides checking if `contentSize` and `wideSize` have a + * value, we now show this information only if their values + * are not a `css var`. This needs to change when parsing + * css variables land. + * + * @see https://github.com/WordPress/gutenberg/pull/34710#issuecomment-918000752 + * + * @param {Object} layout The layout object. + * @return {Object} An object with contextual info per alignment. + */ +function getAlignmentsInfo( layout ) { + const { contentSize, wideSize } = layout; + const alignmentInfo = {}; + if ( contentSize && ! contentSize?.startsWith( 'var' ) ) { + // translators: %s: container size (i.e. 600px etc) + alignmentInfo.none = sprintf( __( 'Max %s wide' ), contentSize ); + } + if ( wideSize && ! wideSize?.startsWith( 'var' ) ) { + // translators: %s: container size (i.e. 600px etc) + alignmentInfo.wide = sprintf( __( 'Max %s wide' ), wideSize ); + } + return alignmentInfo; +} From 2e39c12869625c0f48b0429ef8e525cfd2d1520c Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Fri, 24 Sep 2021 17:54:34 +0300 Subject: [PATCH 12/12] use regex for sizes validation --- packages/block-editor/src/layouts/flow.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/layouts/flow.js b/packages/block-editor/src/layouts/flow.js index cbf623e04fdd8d..9dfec06c547155 100644 --- a/packages/block-editor/src/layouts/flow.js +++ b/packages/block-editor/src/layouts/flow.js @@ -206,11 +206,12 @@ export default { function getAlignmentsInfo( layout ) { const { contentSize, wideSize } = layout; const alignmentInfo = {}; - if ( contentSize && ! contentSize?.startsWith( 'var' ) ) { + const sizeRegex = /^(?!0)\d+(px|em|rem|vw|vh|%)?$/i; + if ( sizeRegex.test( contentSize ) ) { // translators: %s: container size (i.e. 600px etc) alignmentInfo.none = sprintf( __( 'Max %s wide' ), contentSize ); } - if ( wideSize && ! wideSize?.startsWith( 'var' ) ) { + if ( sizeRegex.test( wideSize ) ) { // translators: %s: container size (i.e. 600px etc) alignmentInfo.wide = sprintf( __( 'Max %s wide' ), wideSize ); }