-
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
Block Preview: Add ability for previews to render block patterns. #42832
Conversation
Size Change: +336 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
… minimal performance cost. Tests added for verification of behavior.
9fc3fc0
to
37aa07c
Compare
I tested with a file dropped into a /patterns/ directory within TwentyTwentyTwo, as follows:
And the result was that both patterns rendered properly within the BlockPreview 🎉 |
Can you explain a bit more what's preventing the nested block patterns from showing currently in the previews? Did we disabled them on purpose or something? |
Ok so it seems the reason for this, is this perf optimization we did before #29444 (comment) It seems weird that we need |
Thanks for the PR @BE-Webdesign!
In general we would need to have it inside the block editor in a component. I'm not sure if there is a better place for it, but it's worth it to explore(not necessarily here).. Having said that, by skimming through the code here, it seems we are recursively searching the pattern's blocks to get possible pattern names, which is similar to what we do in the first place when we parse them. Have you explored adding some code in the initial parsing(around here) of the patterns and we could probably add an extra field there with the pattern names to include in previews? For example we add the blocks property in each pattern which is not a part of the patterns API. |
I have not explored that option, I think that would work fine. I think, in general, I misunderstood the performance issue #29444 (comment). I thought that it was parsing every pattern on block preview, but if the parse only grabs the minimum set of patterns then my approach is probably the same as just removing the code change for #29444 (comment). I have not tested to see the difference. From a quick glance through the code a bit more, it seems that the patterns get parsed asynchronously one by one via calls to |
Hm.. It does parse every pattern and each pattern is not parsed twice because we use selectors which return the cached result in other calls of So my thought was to do the search for inner patterns there and then use this information where needed. |
Yes, and I think my suggestion was to not do any "pre parsing" for previews, |
Closing in favor of: #44784 |
Fixes #39732.
What?
Adds support to render block patterns in block previews with minimal performance cost. Tests added for verification of behavior.
Why?
Site builders should be able to reuse patterns within other patterns and have the display render in the block preview. This will allow for more maintainable and reusable theme patterns.
How?
This will traverse the blocks in the block preview. If no pattern blocks are found we don't do anything and result is memoized. This means that performance will be the same as it currently is with only the added cost of quickly traversing the blocks in the pattern, which is very fast. For the majority of patterns performance will look exactly the same as it is right now. If pattern(s) are found they will be added to the preview. Then each pattern is checked for the existence of patterns within that pattern recursively to find all patterns needed to render the preview. This way block previews will always render with the minimum set of patterns they need to render to minimize parsing handled by the
BlockList
component.Testing Instructions
core/pattern
block in it that points to a pattern with content in it. If you are using twenty-twenty-two you can do a pattern with content equal to:<!-- wp:pattern {"slug":"twentytwentytwo/footer-blog"} /-->
This pattern will now have a pattern within its own content.