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

i18n: Add pattern block #33217

Merged
merged 29 commits into from
Oct 29, 2021
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d9c7863
i18n: Add pattern block
scruffian Jul 6, 2021
2f131bc
i18n: Add pattern block
scruffian Jul 6, 2021
e27ef10
i18n: Add pattern block
scruffian Jul 6, 2021
587d496
Switch to using innerblocks
scruffian Jul 6, 2021
fb50aa7
add useeffect lifecycle method to replace innerblocks and hide from i…
scruffian Jul 14, 2021
46893b3
Only run useeffect once
scruffian Jul 16, 2021
f7eafa7
Update packages/block-library/src/pattern/index.php
scruffian Jul 15, 2021
86dd396
Update packages/block-library/src/pattern/edit.js
scruffian Jul 15, 2021
1c7e99e
Update packages/block-library/src/pattern/block.json
scruffian Jul 15, 2021
224c108
Update packages/block-library/src/pattern/block.json
scruffian Jul 15, 2021
8979ee4
Replace the pattern block with the innerblocks when the block is sele…
scruffian Jul 21, 2021
cae2c3c
reformat
scruffian Jul 21, 2021
25c26b4
Remove save
scruffian Aug 6, 2021
3d9ece5
Add fixtures
scruffian Aug 6, 2021
6a9e3ef
simplify how we determine if a block is selected
scruffian Aug 6, 2021
20936da
Update packages/block-library/src/pattern/edit.js
jffng Oct 7, 2021
3e602b4
Update packages/block-library/src/pattern/index.php
jffng Oct 7, 2021
076a4de
Cache pattern on slug
jffng Oct 7, 2021
82a2450
Add dependency to hook.
jffng Oct 7, 2021
3e8d7b8
Use innerBlocksProps + blockProps.
jffng Oct 7, 2021
90619bb
Fix phpcs errors.
jffng Oct 7, 2021
4fc64ba
Update tests to reference a core pattern.
jffng Oct 7, 2021
a2adecf
Add mcore/ prefix to slug.
jffng Oct 7, 2021
0cb327b
Replace pattern wrapper with group.
jffng Oct 12, 2021
e88bcdf
Fix linting error.
jffng Oct 12, 2021
4497b0e
Wrap pattern contents in div instead of group block.
jffng Oct 13, 2021
3843b3e
Replace InnerBlocks with div.
jffng Oct 28, 2021
f1ca663
Minor render callback refactoring
Mamaduka Oct 28, 2021
1f5714b
Merge branch 'trunk' into add/pattern-block
Mamaduka Oct 29, 2021
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
2 changes: 2 additions & 0 deletions lib/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ function gutenberg_reregister_core_block_types() {
'navigation-link',
'navigation-submenu',
'nextpage',
'pattern',
'paragraph',
'preformatted',
'pullquote',
Expand Down Expand Up @@ -69,6 +70,7 @@ function gutenberg_reregister_core_block_types() {
'social-link.php' => 'core/social-link',
'tag-cloud.php' => 'core/tag-cloud',
'page-list.php' => 'core/page-list',
'pattern.php' => 'core/pattern',
'post-author.php' => 'core/post-author',
'post-comment.php' => 'core/post-comment',
'post-comment-author.php' => 'core/post-comment-author',
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import * as list from './list';
import * as missing from './missing';
import * as more from './more';
import * as nextpage from './nextpage';
import * as pattern from './pattern';
import * as pageList from './page-list';
import * as preformatted from './preformatted';
import * as pullquote from './pullquote';
Expand Down Expand Up @@ -195,6 +196,7 @@ export const __experimentalGetCoreBlocks = () => [
postTerms,

logInOut,
pattern,
];

/**
Expand Down
17 changes: 17 additions & 0 deletions packages/block-library/src/pattern/block.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"apiVersion": 2,
"name": "core/pattern",
"title": "Pattern",
"category": "design",
"description": "Show a block pattern.",
"supports": {
"html": false,
"inserter": false
},
"textdomain": "default",
"attributes": {
"slug": {
"type": "string"
}
}
}
64 changes: 64 additions & 0 deletions packages/block-library/src/pattern/edit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* WordPress dependencies
*/
import { useSelect, useDispatch } from '@wordpress/data';
import { useEffect } from '@wordpress/element';
import {
store as blockEditorStore,
useBlockProps,
__experimentalUseInnerBlocksProps as useInnerBlocksProps,
} from '@wordpress/block-editor';
import { createBlock } from '@wordpress/blocks';

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

const hasSelection = useSelect(
( select ) =>
isSelected ||
select( blockEditorStore ).hasSelectedInnerBlock( clientId, true ),
[ isSelected, clientId ]
);

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

// Run this effect when the block, or any of its InnerBlocks are selected.
// This replaces the Pattern block wrapper with a Group block.
// This ensures the markup structure and alignment are consistent between editor and view.
// This change won't be saved unless further changes are made to the InnerBlocks.
Copy link
Contributor

Choose a reason for hiding this comment

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

If the user changes something else in the template outside the pattern block, it will also save a "copy" of the pattern in the template and not reference the pattern anymore, is this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the way that patterns work in general, so I think yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I misread you here, sorry. I think if the user changes the pattern then we should save a copy, but if they change other things in the template the pattern should still be loaded from the file.

useEffect( () => {
if ( hasSelection && selectedPattern?.blocks ) {
__unstableMarkNextChangeAsNotPersistent();
replaceBlocks(
clientId,
createBlock( 'core/group', {}, selectedPattern.blocks )
);
}
}, [ hasSelection, selectedPattern?.blocks ] );

// Run this effect when the component loads.
// This adds the Pattern block template as InnerBlocks.
// This change won't be saved.
useEffect( () => {
if ( selectedPattern?.blocks ) {
__unstableMarkNextChangeAsNotPersistent();
replaceInnerBlocks( clientId, selectedPattern.blocks );
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that in the editor, we kind of "never keep" the patterns block rendered and just it to fetch the pattern initially and replace the block entirely? In which case the "div" wrapper doesn't matter since this block is never actually visible in the editor. And in fact in the frontend, "no div" is better since when replacing the blocks here we're removing the temporary wrapper.

Copy link
Contributor

Choose a reason for hiding this comment

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

What's important is to be consistent

Copy link
Contributor

@jffng jffng Oct 7, 2021

Choose a reason for hiding this comment

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

There is a wrapping div initially in the editor:

Screen Shot 2021-10-07 at 3 52 40 PM

If you select the block, the wrapping div goes away. I am not sure if this is what you are referring to, though. What should change in the PR to be consistent?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we wait for the the block to be selected to replace it with its inner blocks?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we wait for the the block to be selected to replace it with its inner blocks?

IIRC, to avoid dirtying the post. See #33217 (comment). But it might just be better to use __unstableMarkNextChangeAsNotPersistent and friends.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reasoning is that if you open the site editor but then you only make modifications to the header or footer but not the pattern itself, you'll still be pulling the pattern from PHP. This means that if the pattern is updated in the future you'll see the newer version, unless you have actively changed it, which is the same behaviour as templates in general.

Copy link
Contributor

Choose a reason for hiding this comment

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

The reasoning is that if you open the site editor but then you only make modifications to the header or footer but not the pattern itself, you'll still be pulling the pattern from PHP. This means that if the pattern is updated in the future you'll see the newer version, unless you have actively changed it, which is the same behaviour as templates in general.

This seems like the opposite of the explantation here? #33217 (comment)

Anyway, if this is the behavior we want, we should always have a div wrapper which means:

  • Have a div on the frontend
  • Keep the temporary step like explained here
  • When editing the blocks, replace the block with a group block + inner blocks and not just inner blocks. (otherwise the div is gone creating differences in terms of alignments...)

The other possibility is to remove the temporary state and remove the divs everywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

When editing the blocks, replace the block with a group block + inner blocks and not just inner blocks. (otherwise the div is gone creating differences in terms of alignments...)

I tried this in 7620b99.

The result is:

  • The pattern is wrapped in a group block on the frontend
  • In the editor, when you select the pattern or any of its inner blocks, the wrapper is replaced with a group block. This change only persists if edits are made + saved, otherwise the template will continue to pull from the pattern file.

}
}, [ selectedPattern?.blocks ] );

const props = useInnerBlocksProps( useBlockProps(), {} );
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if I implemented this correctly — does anything need to be supplied here?

Copy link
Member

Choose a reason for hiding this comment

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

I think props are optional.


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

export default PatternEdit;
12 changes: 12 additions & 0 deletions packages/block-library/src/pattern/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Internal dependencies
*/
import metadata from './block.json';
import PatternEdit from './edit';

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

export const settings = {
edit: PatternEdit,
};
44 changes: 44 additions & 0 deletions packages/block-library/src/pattern/index.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php
/**
* Server-side rendering of the `core/pattern` block.
*
* @package WordPress
*/

/**
* Registers the `core/pattern` block on the server.
*
* @return void
*/
function register_block_core_pattern() {
register_block_type_from_metadata(
__DIR__ . '/pattern',
array(
'render_callback' => 'render_block_core_pattern',
)
);
}

/**
* Renders the `core/pattern` block on the server.
*
* @param array $attributes Block attributes.
*
* @return string Returns the output of the pattern.
*/
function render_block_core_pattern( $attributes ) {
if ( empty( $attributes['slug'] ) ) {
return '';
}

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

$pattern = $registry->get_registered( $slug );
return do_blocks( '<div>' . $pattern['content'] . '</div>' );
}

add_action( 'init', 'register_block_core_pattern' );
1 change: 1 addition & 0 deletions test/integration/fixtures/blocks/core__pattern.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<!-- wp:pattern {"slug":"core/text-two-columns"} /-->
12 changes: 12 additions & 0 deletions test/integration/fixtures/blocks/core__pattern.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[
{
"clientId": "_clientId_0",
"name": "core/pattern",
"isValid": true,
"attributes": {
"slug": "core/text-two-columns"
},
"innerBlocks": [],
"originalContent": ""
}
]
11 changes: 11 additions & 0 deletions test/integration/fixtures/blocks/core__pattern.parsed.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[
{
"blockName": "core/pattern",
"attrs": {
"slug": "core/text-two-columns"
},
"innerBlocks": [],
"innerHTML": "",
"innerContent": []
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<!-- wp:pattern {"slug":"core/text-two-columns"} /-->