-
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 Library - Query Loop]: Fix variation with declaredicon
object with src
#44270
Conversation
Size Change: +10 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
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.
The fix works well. Thanks, Nik.
I see that on the trunk, the variation icon is correctly rendered in a few places, like slash inserter results and block toolbar. But then fails in the Placeholder
component.
Do you think we should make the Placeholder
component more resilient like others?
Thanks for reviewing @Mamaduka! I think we shouldn't tweak Placeholder component as is a more generic one, in contrary to some of the block editor ones that take into account that the icon can be from the block type, and therefore include the |
Hey folks, I found a similar issue on the |
What?
Currently if we register a Query Loop variation with having an
icon
object withsrc
property - which is not adashicon
- it will crash the block during the insertion, due to the lack of handling this case. This PR just adds some adhoc handling for this, as this is related to some implementation details to QueryPlaceholder component..It would be good to consider a better handling of variations validation similar to block types, but that should be discussed and handled separately. By playing around a bit, it has nuances in general because handling
registerBlockVariation
isn't enough, as we operate on variations in places directly from theblockType
object.Testing Instructions
icon
withsrc
. Example: