-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Template part - add patterns placeholder selection. #31155
Template part - add patterns placeholder selection. #31155
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point the component works pretty well. It appears when I would expect it to (creating a new 'blank' template part or removing all content from an existing one).
What is not working at this point is when we "choose" a specific pattern, it replaces the template part block itself instead of adding content inside of it.
@@ -266,3 +266,83 @@ function gutenberg_block_header_area() { | |||
function gutenberg_block_footer_area() { | |||
gutenberg_block_template_part( 'footer' ); | |||
} | |||
|
|||
register_block_pattern( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are just copies of tt1-blocks template parts thrown in here as patterns for testing purposes.
A ❓ question to answer before shipping is - should we register any template-part block patterns in Gutenberg (or leave that to themes/plugins)? And if so what should those patterns be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It largely depends on what comes up from the work on the navigation block and patterns for header areas. We would include some patterns for headers once those are ready.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This question is coming up for a lot of blocks. 😄 I've opened #31231 for us to start exploring some of these patterns.
It strikes me that when we combine template part patterns with all of our query block patterns, we're basically including a handful of mix-and-match themes for folks to choose from. 🤔
Size Change: +74 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
some e2e's are expected to fail at this point - the patterns we have temporarily registered in this PR for testing purposes will trigger an extra selection step those tests don't account for. @jameskoster how does this flow feel currently? Iirc from previous discussions the patterns selection should also be triggered if the user removes all their blocks from inside the template part? 🤔 |
Yeah I posed that for discussion, but I don't think there was any strong feelings either way. If pattern selection were to move to a modal (#31033) I suspect this flow wouldn't work so well, so we can probably discount it for now.
This is working as described. The tricky thing here is determining which patterns to display for general template parts. In some ways it feels kind of strange to see Header / Footer patterns when I haven't specified an Two options I can see:
Taking a step back, without knowing the intended area for the template part, the chances of us presenting a pattern that is relevant to the users intentions seems quite low, simply due to the enormous variety of template part use cases. This may need some more thought. |
I think we could leave it up to who registers the pattern to determine where it shows up. For example, I could register both of these example patterns so that they ONLY show up for their expected variations and never the general case. So I registered the header pattern as Leaving it up to how the pattern is registered would give flexibility, and if there ever IS a pattern that makes sense for the general case it can be registered to show up for the general case. But as a standard 'rule' we could advise not registering header/footer patterns to show up for that case. 🤔 |
Updated for this now. So the general case should skip the patterns selection entirely now since we don't have any patterns registered that make sense for that case. |
packages/block-library/src/template-part/edit/entity-content/patterns-setup.js
Outdated
Show resolved
Hide resolved
Awesome, that actually simplifies things quite a bit on the technical side by allowing us to use patterns selection to happen before entity creation. Some version of patterns transforms would probably make more sense for the pre-existing template part case. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm very happy with this iteration. Great work Addison! 💯
I've left a minor comment to address and I think a rebase will make the tests succeed, as they are related to the new components.
packages/block-library/src/template-part/edit/placeholder/index.js
Outdated
Show resolved
Hide resolved
This reverts commit 5dabb7b89987fe36a15058dea37e0e33bb0f7fe1.
11c84b7
to
9683e77
Compare
Description
Attempts to add the newly added (in #29602) patterns placeholder component to the template part block.
How has this been tested?
First, verify the existing template part creation flow continues to work as expected.
Second, register some template part block patterns and test the creation flow when patterns are present. We can register header and footer patterns in php using the example below. For testing, I added this at the end of template-parts.php:
Test that the patterns placeholder component appears for the Template Part block when we select "Create New" from the placeholder block flow for a header or footer ( block inserter -> "Header" or "Footer").
The "Header" and "Footer" variations should only show the header or footer pattern respectively. The generic / "Template Part" variation should show no patterns selection step, since none are currently registered for the generic case. We can test patterns to show up in the general case by adding
core/template-part
to theirblockTypes
array in registration.Screenshots
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).