-
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
Suggest Block patterns in block placeholder states #29602
Conversation
Size Change: -647 B (0%) Total Size: 1.47 MB
ℹ️ View Unchanged
|
Taking this one for a spin, a good reference, this is what an unconfigured columns block looks like, before: Here's after: One thing I like about this a lot is that this let's the placeholder state survive the squint test. That is — the placeholder is not just a white box, but it actually looks a little bit like the end result. I dig it. There are some CSS tweaks to do here and there to get close to @jameskoster's design, I'm happy to help here get this perfect, if you like. I'll wait for others to chime in, but let me know when the time's right! I recall a @mtias comment about when and where to best leverage this pattern, would be good to get a quick insight. |
Super excited to see this! In addition to the style tweaks Joen mentioned there's one thing on the technical side that I think once adjusted would improve the overall experience. In placeholder state I am currently able to see outlines when I hover blocks and inline inserter buttons which feels a bit odd. Ideally, in this preview state the blocks/canvas should be entirely non-interactive. |
I really like where this is going!
Yes! It definitely seems like this would work great for that case as well. In the template parts case I also see the I am excited for TPs and other blocks to leverage this awesome new component. 😁 Let me know if there is anything I can do to help! |
Added the needs design label as a reminder to make the style adjustments mentioned above. I hope to get to this before the weekend. |
Thanks all for reviewing/testing so far 👍 !
I'm actually waiting for a bit more feedback on this (besides the styling that will need polishing), and explorations on |
Do you think the component we have in this PR could be leveraged for the pattern transforms case as well? It seems like it would have a lot of similar functionalities and interface. |
Great to see this! I'd expect the grid view to be the exact same view as what we have in the quick inserter:
I think we'd want to move this flow into a modal to have it more focused and to also handle the problem of not enough space for so much interactivity. It might also help with the double-toolbar feel currently between the block toolbar and placeholder header. As the pattern directory matures, we'd need to possibly connect this more broadly for a better browsing experience. But that's a later stage.
There are some transforms that I'd expect to work more like: And other cases, like a header template part, where the carousel would provide a much better experience. Particularly in the "isolated template part" flow, which is virtually the same as the modal suggested above. |
lib/block-patterns.php
Outdated
|
||
|
||
// Buttons scoped pattern to test with Placeholder. | ||
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.
@kjellr can we start thinking for which blocks we might want to have bundled patterns for? It doesn't have to be many, but it'd be good to be able to not rely on dummy patterns for testing and refining the UI.
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.
Also to start thinking about: Looking forward to implementing this with template parts, Im guessing we should probably register ~2 header and 2 footer patterns in Gutenberg itself when the time comes? So that the patterns selection/transform can be used even if the theme or other plugins aren't registering any. Perhaps they should be new/unique? Or maybe we take from existing designs like tt1-blocks template parts? 🤔
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.
packages/block-editor/src/components/block-pattern-picker/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-pattern-picker/layout-setup-toolbar.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-pattern-picker/layout-setup-toolbar.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-pattern-picker/layout-setup-toolbar.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-pattern-picker/layout-setup-toolbar.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-pattern-picker/use-patterns-setup.js
Outdated
Show resolved
Hide resolved
I shared designs for transforms that consume this component in #28736 (comment). As for some blocks surfacing patterns in grid view initially, and others in the carousel view, that seems to be a simple case of allowing blocks to specify a default orientation for this component. The modal idea is interesting but there is one thing I'm struggling with: you lose the sense how the pattern will slot in alongside its siblings. This can be very important since some pattern layouts will no doubt adapt based on the dimensions of its container. If the modal is large and the target space small, the end result might not match the preview. Perhaps its possible for the modal dimensions to be influenced by the container? Or even be resizable (to draw more comparisons with isolation view (#29148))? |
What do we need to move this forward? From my side I can see these things:
Is this something that needs design decision? |
Also wondering if we can get some basic test (e2e?) coverage for this component once its ready. |
Good review items. I'll look at the scrollbars/boundary issue, and look at adding a border in the grid view. |
It seems that part may be a theme issue. I am observing the issue with tt1-blocks, but it seems perfectly fine when i test with mayland-blocks |
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 is looking great! My feedback is minor; beyond that, LGTM.
packages/block-editor/src/components/block-pattern-setup/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-pattern-setup/setup-toolbar.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-pattern-setup/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-pattern-setup/use-patterns-setup.js
Outdated
Show resolved
Hide resolved
Added gray borders for patterns inside grid mode, and fixed the sizing issue. |
- Padding on various pattern selection elements - Use `cursor: pointer` when hovering patterns - Ensure carousel/grid view toggle is centrally aligned - Clean up vertical alignment of toolbar
24b4cb7
to
4f81697
Compare
Created an issue here: #31071 |
Thanks for all the work here! |
Description
Resolves: #28735
Related: #27575
Based on the design: #28735 (comment)
When inserting something like a
Query
block, it may be useful to present the user with patterns that consume relevant block patterns and inspire them to create something more elaborate.BlockPatternPicker component
For the suggestion of block patterns in placeholder states I've created a new component
BlockPatternPicker
which can handle this problem with minimal amount of new code, in any block that wants to use this. The concept is simple and you can see an example here.Placeholder
, which was the case before too.blockTypes
property or you can provide a custom patterns filtering function (filterPatternsFn
). This might be useful fortemplate parts
(?). This empowers the developer to show specific only patterns and handles the case of showing existing patterns that are desired. I have used this only forQuery
block for now to test with.startBlankComponent
which is rendered if no block patterns are matched or we clickStart blank
.In this POC I had implemented this in
Buttons
andColumns
to test how it behaves with a block that previously had shown block variations picker(Columns
) and one that hadn't (Buttons
), but now have removed them since it's going to the last steps of landing. You can always check previous commits with the above blocks using the new component.Notes
blockTypes
property inBlock patterns
API introduced here: Block Patterns: Removescope
from Query patterns and introduceblockTypes
#30471, in which we can 'connect' block patterns with blocks.inherit
attribute as it should probably be handled better with something like this issue: Context detection for Query block #30369Showcase
Checklist: