Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Block editor: hooks: manage BlockListBlock filters in one place #56875

Merged
merged 6 commits into from
Dec 8, 2023
Merged

Block editor: hooks: manage BlockListBlock filters in one place #56875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3aeb8dc
Block editor: hooks: manage BlockListBlock filters in one place
ellatrix Dec 7, 2023
a4662a6
Add style and align
ellatrix Dec 7, 2023
4d1f51b
Add color, border
ellatrix Dec 8, 2023
b722b46
Render in parallel
ellatrix Dec 8, 2023
f4b426b
Add font size
ellatrix Dec 8, 2023
5279781
Add layout
ellatrix Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 4 additions & 37 deletions packages/block-editor/src/hooks/align.js
View file
Open in desktop
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import classnames from 'classnames';
/** /**
* WordPress dependencies * WordPress dependencies
*/ */
import { createHigherOrderComponent } from '@wordpress/compose';
import { addFilter } from '@wordpress/hooks'; import { addFilter } from '@wordpress/hooks';
import { import {
getBlockSupport, getBlockSupport,
Expand Down Expand Up @@ -155,54 +154,27 @@ function BlockEditAlignmentToolbarControlsPure( {
export default { export default {
shareWithChildBlocks: true, shareWithChildBlocks: true,
edit: BlockEditAlignmentToolbarControlsPure, edit: BlockEditAlignmentToolbarControlsPure,
useBlockProps,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe useBlockEditProps, I can see us having useBlockSaveProps, or useBlockProps for both?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should keep it consistent with the block API? useBlockProps and useBlockProps.save? One is a hook and the other one is static.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But yes, it's not entirely the same so maybe a different naming is appropriate.

attributeKeys: [ 'align' ], attributeKeys: [ 'align' ],
hasSupport( name ) { hasSupport( name ) {
return hasBlockSupport( name, 'align', false ); return hasBlockSupport( name, 'align', false );
}, },
}; };


function BlockListBlockWithDataAlign( { block: BlockListBlock, props } ) { function useBlockProps( { name, align } ) {
const { name, attributes } = props;
const { align } = attributes;
const blockAllowedAlignments = getValidAlignments( const blockAllowedAlignments = getValidAlignments(
getBlockSupport( name, 'align' ), getBlockSupport( name, 'align' ),
hasBlockSupport( name, 'alignWide', true ) hasBlockSupport( name, 'alignWide', true )
); );
const validAlignments = useAvailableAlignments( blockAllowedAlignments ); const validAlignments = useAvailableAlignments( blockAllowedAlignments );


let wrapperProps = props.wrapperProps;
if ( validAlignments.some( ( alignment ) => alignment.name === align ) ) { if ( validAlignments.some( ( alignment ) => alignment.name === align ) ) {
wrapperProps = { ...wrapperProps, 'data-align': align }; return { 'data-align': align };
} }


return <BlockListBlock { ...props } wrapperProps={ wrapperProps } />; return {};
} }


/**
* Override the default block element to add alignment wrapper props.
*
* @param {Function} BlockListBlock Original component.
*
* @return {Function} Wrapped component.
*/
export const withDataAlign = createHigherOrderComponent(
( BlockListBlock ) => ( props ) => {
// If an alignment is not assigned, there's no need to go through the
// effort to validate or assign its value.
if ( props.attributes.align === undefined ) {
return <BlockListBlock { ...props } />;
}

return (
<BlockListBlockWithDataAlign
block={ BlockListBlock }
props={ props }
/>
);
},
'withDataAlign'
);

/** /**
* Override props assigned to save component to inject alignment class name if * Override props assigned to save component to inject alignment class name if
* block supports it. * block supports it.
Expand Down Expand Up @@ -237,11 +209,6 @@ addFilter(
'core/editor/align/addAttribute', 'core/editor/align/addAttribute',
addAttribute addAttribute
); );
addFilter(
'editor.BlockListBlock',
'core/editor/align/with-data-align',
withDataAlign
);
addFilter( addFilter(
'blocks.getSaveContent.extraProps', 'blocks.getSaveContent.extraProps',
'core/editor/align/addAssignedAlign', 'core/editor/align/addAssignedAlign',
Expand Down
49 changes: 13 additions & 36 deletions packages/block-editor/src/hooks/border.js
View file
Open in desktop
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import classnames from 'classnames';
*/ */
import { getBlockSupport } from '@wordpress/blocks'; import { getBlockSupport } from '@wordpress/blocks';
import { __experimentalHasSplitBorders as hasSplitBorders } from '@wordpress/components'; import { __experimentalHasSplitBorders as hasSplitBorders } from '@wordpress/components';
import { createHigherOrderComponent, pure } from '@wordpress/compose'; import { pure } from '@wordpress/compose';
import { Platform, useCallback, useMemo } from '@wordpress/element'; import { Platform, useCallback, useMemo } from '@wordpress/element';
import { addFilter } from '@wordpress/hooks'; import { addFilter } from '@wordpress/hooks';
import { useSelect } from '@wordpress/data'; import { useSelect } from '@wordpress/data';
Expand Down Expand Up @@ -330,25 +330,14 @@ function addEditProps( settings ) {
return settings; return settings;
} }


/** function useBlockProps( { name, borderColor, style } ) {
* This adds inline styles for color palette colors.
* Ideally, this is not needed and themes should load their palettes on the editor.
*
* @param {Function} BlockListBlock Original component.
*
* @return {Function} Wrapped component.
*/
export const withBorderColorPaletteStyles = createHigherOrderComponent(
( BlockListBlock ) => ( props ) => {
const { name, attributes } = props;
const { borderColor, style } = attributes;
const { colors } = useMultipleOriginColorsAndGradients(); const { colors } = useMultipleOriginColorsAndGradients();


if ( if (
! hasBorderSupport( name, 'color' ) || ! hasBorderSupport( name, 'color' ) ||
shouldSkipSerialization( name, BORDER_SUPPORT_KEY, 'color' ) shouldSkipSerialization( name, BORDER_SUPPORT_KEY, 'color' )
) { ) {
return <BlockListBlock { ...props } />; return {};
} }


const { color: borderColorValue } = getMultiOriginColor( { const { color: borderColorValue } = getMultiOriginColor( {
Expand All @@ -366,9 +355,7 @@ export const withBorderColorPaletteStyles = createHigherOrderComponent(


const { color: borderBottomColor } = getMultiOriginColor( { const { color: borderBottomColor } = getMultiOriginColor( {
colors, colors,
namedColor: getColorSlugFromVariable( namedColor: getColorSlugFromVariable( style?.border?.bottom?.color ),
style?.border?.bottom?.color
),
} ); } );
const { color: borderLeftColor } = getMultiOriginColor( { const { color: borderLeftColor } = getMultiOriginColor( {
colors, colors,
Expand All @@ -381,21 +368,17 @@ export const withBorderColorPaletteStyles = createHigherOrderComponent(
borderBottomColor: borderBottomColor || borderColorValue, borderBottomColor: borderBottomColor || borderColorValue,
borderLeftColor: borderLeftColor || borderColorValue, borderLeftColor: borderLeftColor || borderColorValue,
}; };
const cleanedExtraStyles = cleanEmptyObject( extraStyles ) || {};

let wrapperProps = props.wrapperProps;
wrapperProps = {
...props.wrapperProps,
style: {
...props.wrapperProps?.style,
...cleanedExtraStyles,
},
};


return <BlockListBlock { ...props } wrapperProps={ wrapperProps } />; return { style: cleanEmptyObject( extraStyles ) || {} };
}

export default {
useBlockProps,
attributeKeys: [ 'borderColor', 'style' ],
hasSupport( name ) {
return hasBorderSupport( name, 'color' );
}, },
'withBorderColorPaletteStyles' };
);


addFilter( addFilter(
'blocks.registerBlockType', 'blocks.registerBlockType',
Expand All @@ -414,9 +397,3 @@ addFilter(
'core/border/addEditProps', 'core/border/addEditProps',
addEditProps addEditProps
); );

addFilter(
'editor.BlockListBlock',
'core/border/with-border-color-palette-styles',
withBorderColorPaletteStyles
);
42 changes: 10 additions & 32 deletions packages/block-editor/src/hooks/color.js
View file
Open in desktop
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import classnames from 'classnames';
import { addFilter } from '@wordpress/hooks'; import { addFilter } from '@wordpress/hooks';
import { getBlockSupport } from '@wordpress/blocks'; import { getBlockSupport } from '@wordpress/blocks';
import { useMemo, Platform, useCallback } from '@wordpress/element'; import { useMemo, Platform, useCallback } from '@wordpress/element';
import { createHigherOrderComponent, pure } from '@wordpress/compose'; import { pure } from '@wordpress/compose';
import { useSelect } from '@wordpress/data'; import { useSelect } from '@wordpress/data';


/** /**
Expand Down Expand Up @@ -364,18 +364,7 @@ function ColorEditPure( { clientId, name, setAttributes, settings } ) {
// and not the whole attributes object. // and not the whole attributes object.
export const ColorEdit = pure( ColorEditPure ); export const ColorEdit = pure( ColorEditPure );


/** function useBlockProps( { name, backgroundColor, textColor } ) {
* This adds inline styles for color palette colors.
* Ideally, this is not needed and themes should load their palettes on the editor.
*
* @param {Function} BlockListBlock Original component.
*
* @return {Function} Wrapped component.
*/
export const withColorPaletteStyles = createHigherOrderComponent(
( BlockListBlock ) => ( props ) => {
const { name, attributes } = props;
const { backgroundColor, textColor } = attributes;
const [ userPalette, themePalette, defaultPalette ] = useSettings( const [ userPalette, themePalette, defaultPalette ] = useSettings(
'color.palette.custom', 'color.palette.custom',
'color.palette.theme', 'color.palette.theme',
Expand All @@ -394,7 +383,7 @@ export const withColorPaletteStyles = createHigherOrderComponent(
! hasColorSupport( name ) || ! hasColorSupport( name ) ||
shouldSkipSerialization( name, COLOR_SUPPORT_KEY ) shouldSkipSerialization( name, COLOR_SUPPORT_KEY )
) { ) {
return <BlockListBlock { ...props } />; return {};
} }
const extraStyles = {}; const extraStyles = {};


Expand All @@ -417,19 +406,14 @@ export const withColorPaletteStyles = createHigherOrderComponent(
)?.color; )?.color;
} }


let wrapperProps = props.wrapperProps; return { style: extraStyles };
wrapperProps = { }
...props.wrapperProps,
style: {
...extraStyles,
...props.wrapperProps?.style,
},
};


return <BlockListBlock { ...props } wrapperProps={ wrapperProps } />; export default {
}, useBlockProps,
'withColorPaletteStyles' attributeKeys: [ 'backgroundColor', 'textColor' ],
); hasSupport: hasColorSupport,
};


const MIGRATION_PATHS = { const MIGRATION_PATHS = {
linkColor: [ [ 'style', 'elements', 'link', 'color', 'text' ] ], linkColor: [ [ 'style', 'elements', 'link', 'color', 'text' ] ],
Expand Down Expand Up @@ -477,12 +461,6 @@ addFilter(
addEditProps addEditProps
); );


addFilter(
'editor.BlockListBlock',
'core/color/with-color-palette-styles',
withColorPaletteStyles
);

addFilter( addFilter(
'blocks.switchToBlockType.transformedBlock', 'blocks.switchToBlockType.transformedBlock',
'core/color/addTransforms', 'core/color/addTransforms',
Expand Down
65 changes: 18 additions & 47 deletions packages/block-editor/src/hooks/duotone.js
View file
Open in desktop
Original file line number Original file line Diff line number Diff line change
@@ -1,7 +1,6 @@
/** /**
* External dependencies * External dependencies
*/ */
import classnames from 'classnames';
import { extend } from 'colord'; import { extend } from 'colord';
import namesPlugin from 'colord/plugins/names'; import namesPlugin from 'colord/plugins/names';


Expand All @@ -13,7 +12,7 @@ import {
getBlockType, getBlockType,
hasBlockSupport, hasBlockSupport,
} from '@wordpress/blocks'; } from '@wordpress/blocks';
import { createHigherOrderComponent, useInstanceId } from '@wordpress/compose'; import { useInstanceId } from '@wordpress/compose';
import { addFilter } from '@wordpress/hooks'; import { addFilter } from '@wordpress/hooks';
import { useMemo, useEffect } from '@wordpress/element'; import { useMemo, useEffect } from '@wordpress/element';


Expand Down Expand Up @@ -178,6 +177,7 @@ function DuotonePanelPure( { style, setAttributes, name } ) {
export default { export default {
shareWithChildBlocks: true, shareWithChildBlocks: true,
edit: DuotonePanelPure, edit: DuotonePanelPure,
useBlockProps,
attributeKeys: [ 'style' ], attributeKeys: [ 'style' ],
hasSupport( name ) { hasSupport( name ) {
return hasBlockSupport( name, 'filter.duotone' ); return hasBlockSupport( name, 'filter.duotone' );
Expand Down Expand Up @@ -212,7 +212,7 @@ function addDuotoneAttributes( settings ) {
return settings; return settings;
} }


function DuotoneStyles( { function useDuotoneStyles( {
clientId, clientId,
id: filterId, id: filterId,
selector: duotoneSelector, selector: duotoneSelector,
Expand Down Expand Up @@ -310,23 +310,12 @@ function DuotoneStyles( {
blockElement.style.display = display; blockElement.style.display = display;
} }
}, [ isValidFilter, blockElement ] ); }, [ isValidFilter, blockElement ] );

return null;
} }


/** function useBlockProps( { name, style } ) {
* Override the default block element to include duotone styles. const id = useInstanceId( useBlockProps );
*
* @param {Function} BlockListBlock Original component.
*
* @return {Function} Wrapped component.
*/
const withDuotoneStyles = createHigherOrderComponent(
( BlockListBlock ) => ( props ) => {
const id = useInstanceId( BlockListBlock );

const selector = useMemo( () => { const selector = useMemo( () => {
const blockType = getBlockType( props.name ); const blockType = getBlockType( name );


if ( blockType ) { if ( blockType ) {
// Backwards compatibility for `supports.color.__experimentalDuotone` // Backwards compatibility for `supports.color.__experimentalDuotone`
Expand Down Expand Up @@ -362,46 +351,28 @@ const withDuotoneStyles = createHigherOrderComponent(
fallback: true, fallback: true,
} ); } );
} }
}, [ props.name ] ); }, [ name ] );


const attribute = props?.attributes?.style?.color?.duotone; const attribute = style?.color?.duotone;


const filterClass = `wp-duotone-${ id }`; const filterClass = `wp-duotone-${ id }`;


const shouldRender = selector && attribute; const shouldRender = selector && attribute;


const className = shouldRender useDuotoneStyles( {
? classnames( props?.className, filterClass ) clientId: id,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ellatrix I noticed this when refactoring block refs in #60945: you're passing an instance ID instead of the block's clientId, and the useBlockElement( clientId ) will never get the block element, and the Safari bugfix won't work.

This is a bug introduced by this PR. The original implementation in #55288 by @andrewserong was correct.

I don't know how to fix it: the useBlockProps hook doesn't receive clientId or any other block information as a prop.

: props?.className; id: filterClass,
selector,
attribute,
} );


// CAUTION: code added before this line will be executed return {
// for all blocks, not just those that support duotone. Code added className: shouldRender ? filterClass : '',
// above this line should be carefully evaluated for its impact on };
// performance. }
return (
<>
{ shouldRender && (
<DuotoneStyles
clientId={ props.clientId }
id={ filterClass }
selector={ selector }
attribute={ attribute }
/>
) }
<BlockListBlock { ...props } className={ className } />
</>
);
},
'withDuotoneStyles'
);


addFilter( addFilter(
'blocks.registerBlockType', 'blocks.registerBlockType',
'core/editor/duotone/add-attributes', 'core/editor/duotone/add-attributes',
addDuotoneAttributes addDuotoneAttributes
); );
addFilter(
'editor.BlockListBlock',
'core/editor/duotone/with-styles',
withDuotoneStyles
);
Loading
Loading