-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Add contentRole to Query block and make sure Change design always works as expected #71686
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,7 +43,16 @@ export function useBlockPatterns( clientId, attributes ) { | |
| clientId, | ||
| attributes | ||
| ); | ||
| return usePatterns( clientId, blockNameForPatterns ); | ||
| const allPatterns = usePatterns( clientId, blockNameForPatterns ); | ||
| // Filter out any patterns that don't have Query as their root block | ||
| // so that a Query block is always replaced by another Query block. | ||
| const rootBlockPatterns = useMemo( () => { | ||
| return allPatterns.filter( ( pattern ) => { | ||
| return pattern.blocks?.[ 0 ]?.name === blockNameForPatterns; | ||
| } ); | ||
| }, [ allPatterns, blockNameForPatterns ] ); | ||
|
Comment on lines
+46
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey folks! 👋 I’m maintaining Sensei LMS and noticed that one of our blocks (block variation of core/query) stopped showing the “Choose” patterns button, and it looks like this change is related. For context, the custom patterns are registered here. Could you give me a hand with finding a solution?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @m1r0 would you be able to create a new issue for this and then link to it from here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I think the problem might be we're filtering for whatever variation is active, so you won't get other variations or the default query block in the results. We should probably filter for all instances of query block, whatever the variation. I'll try and get a fix up soon! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome! Thank you so much for the quick reaction, @tellthemachines! |
||
|
|
||
| return rootBlockPatterns; | ||
| } | ||
|
|
||
| export default function PatternSelection( { | ||
|
|
||
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 working pretty well for me, and I think the trade off (that it might reduce the number of available patterns) is probably worth it.
Do you think there's a case for best practice eduction that Query patterns should ideally be wrapped in a Query block?
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 don't know. Conceivably you may want to create a complex pattern with a query in it, or even multiple queries. I think there's a place for those, but they aren't viable as replacements for a single Query block.