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.

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

Pattern block: Add experimental flag and syncStatus attrib to allow testing of partial syncing #50533

Merged
merged 15 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
4 changes: 2 additions & 2 deletions docs/reference-guides/core-blocks.md
Original file line number Diff line number Diff line change
Expand Up @@ -464,8 +464,8 @@ Show a block pattern. ([Source](https://github.com/WordPress/gutenberg/tree/trun

- **Name:** core/pattern
- **Category:** theme
- **Supports:** ~~html~~, ~~inserter~~
- **Attributes:** slug
- **Supports:**
- **Attributes:** inheritedAlignment, layout, slug, syncStatus

## Post Author

Expand Down
3 changes: 3 additions & 0 deletions lib/experimental/editor-settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ function gutenberg_enable_experiments() {
if ( $gutenberg_experiments && array_key_exists( 'gutenberg-theme-previews', $gutenberg_experiments ) ) {
wp_add_inline_script( 'wp-block-editor', 'window.__experimentalEnableThemePreviews = true', 'before' );
}
if ( $gutenberg_experiments && array_key_exists( 'gutenberg-pattern-enhancements', $gutenberg_experiments ) ) {
wp_add_inline_script( 'wp-block-editor', 'window.__experimentalEnablePatternEnhancements = true', 'before' );
}

}

Expand Down
12 changes: 12 additions & 0 deletions lib/experiments-page.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,18 @@ function gutenberg_initialize_experiments_settings() {
)
);

add_settings_field(
'gutenberg-pattern-enhancements',
__( 'Pattern enhancements', 'gutenberg' ),
'gutenberg_display_experiment_field',
'gutenberg-experiments',
'gutenberg_experiments_section',
array(
'label' => __( 'Test the Pattern block enhancements', 'gutenberg' ),
'id' => 'gutenberg-pattern-enhancements',
)
);

register_setting(
'gutenberg-experiments',
'gutenberg-experiments'
Expand Down
18 changes: 16 additions & 2 deletions packages/block-library/src/pattern/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,27 @@
"category": "theme",
"description": "Show a block pattern.",
"supports": {
"html": false,
"inserter": false
"__experimentalLayout": {
"allowEditing": false
}
},
"textdomain": "default",
"attributes": {
"slug": {
"type": "string"
},
"inheritedAlignment": {
Copy link
Contributor

Choose a reason for hiding this comment

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

The naming of this attribute, inheritedAlignment, could be a little awkward as "inheriting" from a pattern's inner or child blocks doesn't make much sense to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be worth moving this change into a separate PR, as I'm sure there will be some other thoughts on it. I think the main thing right now is to get a first implementation of the partial syncing behavior, so this isn't something of high urgency, though it is something that needs to be fixed.

Some random thoughts - I wonder if it could be made part of the align block supports. While other blocks might have something like align: [ 'wide', 'full' ], this could possibly be align: 'auto'? When set to that it could avoid showing the UI. Just throwing some ideas out there. It might be good to start some discussions with other contributors who may have thought about this problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As they say 'Insanity is inherited, you get it from your children', so the same could apply here 😄. I have removed it now anyway. As Dan suggests, probably better as a follow-up PR so as not to slow things down if people have different opinions on it.

"type": "string"
},
"layout": {
"type": "object",
"default": {
"type": "constrained"
}
},
"syncStatus": {
"type": [ "string", "boolean" ],
"enum": [ "full", "partial" ]
}
}
}
107 changes: 91 additions & 16 deletions packages/block-library/src/pattern/edit.js
Original file line number Diff line number Diff line change
@@ -1,46 +1,121 @@
/**
* WordPress dependencies
*/
import { cloneBlock } from '@wordpress/blocks';
import { useSelect, useDispatch } from '@wordpress/data';
import { useEffect } from '@wordpress/element';
import {
store as blockEditorStore,
useBlockProps,
useInnerBlocksProps,
} from '@wordpress/block-editor';

const PatternEdit = ( { attributes, clientId } ) => {
const selectedPattern = useSelect(
( select ) =>
select( blockEditorStore ).__experimentalGetParsedPattern(
attributes.slug
),
[ attributes.slug ]
const PatternEdit = ( { attributes, clientId, setAttributes } ) => {
const { inheritedAlignment, slug, syncStatus } = attributes;
const { selectedPattern, innerBlocks } = useSelect(
( select ) => {
return {
selectedPattern:
select( blockEditorStore ).__experimentalGetParsedPattern(
slug
),
innerBlocks:
select( blockEditorStore ).getBlock( clientId )
?.innerBlocks,
};
},
[ slug, clientId ]
);

const { replaceBlocks, __unstableMarkNextChangeAsNotPersistent } =
useDispatch( blockEditorStore );
const {
replaceBlocks,
replaceInnerBlocks,
__unstableMarkNextChangeAsNotPersistent,
} = useDispatch( blockEditorStore );

// Run this effect when the component loads.
// This adds the Pattern's contents to the post.
// This change won't be saved.
// It will continue to pull from the pattern file unless changes are made to its respective template part.
useEffect( () => {
if ( selectedPattern?.blocks ) {
if ( selectedPattern?.blocks && ! innerBlocks?.length ) {
// We batch updates to block list settings to avoid triggering cascading renders
// for each container block included in a tree and optimize initial render.
// Since the above uses microtasks, we need to use a microtask here as well,
// because nested pattern blocks cannot be inserted if the parent block supports
// inner blocks but doesn't have blockSettings in the state.
window.queueMicrotask( () => {
__unstableMarkNextChangeAsNotPersistent();
if ( syncStatus === 'partial' ) {
replaceInnerBlocks(
clientId,
selectedPattern.blocks.map( ( block ) =>
cloneBlock( block )
)
);
return;
}
replaceBlocks( clientId, selectedPattern.blocks );
} );
}
}, [ clientId, selectedPattern?.blocks ] );
}, [
clientId,
selectedPattern?.blocks,
replaceInnerBlocks,
__unstableMarkNextChangeAsNotPersistent,
innerBlocks,
syncStatus,
replaceBlocks,
] );

useEffect( () => {
if ( syncStatus !== 'partial' ) {
return;
}
const alignments = [ 'wide', 'full' ];
const blocks = innerBlocks;
if ( ! blocks || blocks.length === 0 ) {
return;
}
// Determine the widest setting of all the contained blocks.
const widestAlignment = blocks.reduce( ( accumulator, block ) => {
const { align } = block.attributes;
return alignments.indexOf( align ) >
alignments.indexOf( accumulator )
? align
: accumulator;
}, undefined );

const props = useBlockProps();
// Set the attribute of the Pattern block to match the widest
// alignment.
setAttributes( {
glendaviesnz marked this conversation as resolved.
Show resolved Hide resolved
inheritedAlignment: widestAlignment ?? '',
} );
}, [
innerBlocks,
selectedPattern?.blocks,
setAttributes,
inheritedAlignment,
syncStatus,
] );

return <div { ...props } />;
const blockProps = useBlockProps( {
className:
syncStatus === 'partial' &&
inheritedAlignment &&
`align${ inheritedAlignment }`,
} );

const innerBlocksProps = useInnerBlocksProps( blockProps, {
templateLock: syncStatus === 'partial' ? 'contentOnly' : false,
} );

if ( syncStatus !== 'partial' ) {
return <div { ...blockProps } />;
}

return (
<>
glendaviesnz marked this conversation as resolved.
Show resolved Hide resolved
<div { ...innerBlocksProps } />
</>
);
};

export default PatternEdit;
10 changes: 6 additions & 4 deletions packages/block-library/src/pattern/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
*/
import initBlock from '../utils/init-block';
import metadata from './block.json';
import PatternEdit from './edit';
import PatternEditV1 from './v1/edit';
import PatternEditV2 from './edit';
import PatternSave from './save';

const { name } = metadata;
export { metadata, name };

export const settings = {
edit: PatternEdit,
};
export const settings = window?.__experimentalEnablePatternEnhancements
? { edit: PatternEditV2, save: PatternSave }
: { edit: PatternEditV1 };

export const init = () => initBlock( { name, metadata, settings } );
14 changes: 11 additions & 3 deletions packages/block-library/src/pattern/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,31 @@ function register_block_core_pattern() {
/**
* Renders the `core/pattern` block on the server.
*
* @param array $attributes Block attributes.
* @param array $attributes Block attributes.
* @param string $content The block rendered content.
*
* @return string Returns the output of the pattern.
*/
function render_block_core_pattern( $attributes ) {
function render_block_core_pattern( $attributes, $content ) {
if ( empty( $attributes['slug'] ) ) {
return '';
}

$align_class = isset($attributes['inheritedAlignment']) ? 'class="align' . $attributes['inheritedAlignment'] . '"' : '';
$wrapper = '<div '. $align_class . ' data-pattern-slug="' . $attributes['slug'] . '">%s</div>';
glendaviesnz marked this conversation as resolved.
Show resolved Hide resolved

if ( isset( $attributes['syncStatus'] ) && 'unsynced' === $attributes['syncStatus'] ) {
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't appear to be possible to ever have unsynced as a value:
Screenshot 2023-05-15 at 10 28 40

Copy link
Contributor Author

@glendaviesnz glendaviesnz May 15, 2023

Choose a reason for hiding this comment

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

The thinking was that if there is no syncStatus, ie. syncStatus===undefined then the pattern is not synced, which is partly a backwards compat approach as currently none of the patterns in the wild are synced and they have no syncStatus attrib. We could add another syncStatus of unsynced but we will then have to account for both that and undefined as an indicator of not synced.

Adding unsynced however may make it clearer to people browsing the block.json that there is an unsynced option and avoid the confusion you experienced. I don't have a strong opinion either way. @talldan, @aaronrobertshaw what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch @gziolo 👍

The thinking was that if there is no syncStatus, ie. syncStatus===undefined then the pattern is not synced

That's what the block.json attribute definition indicates. What I think @gziolo is questioning is the index.php code checking for an unsynced value. That appears to be a leftover from the original PR where unsynced was a valid syncStatus value and its default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦 missed the first code snippet in the original comment! Ignore all of my previous comment, I will get the index.php file sorted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

return sprintf( $wrapper, $content );
}

$slug = $attributes['slug'];
$registry = WP_Block_Patterns_Registry::get_instance();
if ( ! $registry->is_registered( $slug ) ) {
return '';
}

$pattern = $registry->get_registered( $slug );
return do_blocks( $pattern['content'] );
return sprintf( $wrapper, do_blocks( $pattern['content'] ) );
}

add_action( 'init', 'register_block_core_pattern' );
18 changes: 18 additions & 0 deletions packages/block-library/src/pattern/save.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* WordPress dependencies
*/
import { useBlockProps, useInnerBlocksProps } from '@wordpress/block-editor';

export default function save( {
attributes: { inheritedAlignment, syncStatus },
innerBlocks,
} ) {
if ( innerBlocks?.length === 0 || syncStatus !== 'partial' ) {
return;
}
const blockProps = useBlockProps.save( {
className: inheritedAlignment && `align${ inheritedAlignment }`,
} );
const innerBlocksProps = useInnerBlocksProps.save( blockProps );
return <div { ...innerBlocksProps }>{ innerBlocksProps.children }</div>;
}
51 changes: 51 additions & 0 deletions packages/block-library/src/pattern/v1/edit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* WordPress dependencies
*/
import { useSelect, useDispatch } from '@wordpress/data';
import { useEffect } from '@wordpress/element';
import {
store as blockEditorStore,
useBlockProps,
} from '@wordpress/block-editor';

const PatternEdit = ( { attributes, clientId } ) => {
const selectedPattern = useSelect(
( select ) =>
select( blockEditorStore ).__experimentalGetParsedPattern(
attributes.slug
),
[ attributes.slug ]
);

const { replaceBlocks, __unstableMarkNextChangeAsNotPersistent } =
useDispatch( blockEditorStore );

// Run this effect when the component loads.
// This adds the Pattern's contents to the post.
// This change won't be saved.
// It will continue to pull from the pattern file unless changes are made to its respective template part.
useEffect( () => {
if ( selectedPattern?.blocks ) {
// We batch updates to block list settings to avoid triggering cascading renders
// for each container block included in a tree and optimize initial render.
// Since the above uses microtasks, we need to use a microtask here as well,
// because nested pattern blocks cannot be inserted if the parent block supports
// inner blocks but doesn't have blockSettings in the state.
window.queueMicrotask( () => {
__unstableMarkNextChangeAsNotPersistent();
replaceBlocks( clientId, selectedPattern.blocks );
} );
}
}, [
clientId,
selectedPattern?.blocks,
__unstableMarkNextChangeAsNotPersistent,
replaceBlocks,
] );

const props = useBlockProps();

return <div { ...props } />;
};

export default PatternEdit;
7 changes: 6 additions & 1 deletion test/integration/fixtures/blocks/core__pattern.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
"name": "core/pattern",
"isValid": true,
"attributes": {
"slug": "core/text-two-columns"
"forcedAlignment": "full",
"layout": {
"type": "constrained"
},
"slug": "core/text-two-columns",
"syncStatus": "synced"
},
"innerBlocks": []
}
Expand Down