-
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
Add block transforms preview #27861
Add block transforms preview #27861
Conversation
Size Change: +34 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
return ( | ||
<MenuGroup | ||
label={ __( 'Styles' ) } | ||
className="block-editor-block-switcher__styles__menugroup" | ||
> | ||
{ hoveredClassName && ( | ||
<PreviewBlockPopover | ||
hoveredBlock={ hoveredBlock } | ||
hoveredClassName={ hoveredClassName } | ||
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.
Block styles can be applied only when a single block is selected. It's only changing the CSS class name applied so it feels like blockType.example
shouldn't be needed here. However, it looks like some sort of design decision from the past. Maybe to account for the case when a block has not enough or too much content 🤷🏻♂️
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.
Yeah, I didn't look into it deeper for now, but it looks like it's been like this for the reason you mentioned (existing content..)
The preview for styles feels a little bit like a duplication but I believe it existed before: Did it always work only when hovering an item? In the inserter, it shows the preview also when navigating with the keyboard. For block transforms it looks great, both for a single block and multiple selected blocks 👏🏻 |
Yes. I actually checked it myself when implementing because it made sense to show the preview with keyboard navigation. Maybe in a follow up? What do you think? |
Sure, a follow-up to align behavior is a great idea. I wasn't sure how it works in the main branch. |
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.
Code wise everything is good. It would be nice to get another feedback oriented towards behavior.
Thanks for the gif. From a quick glance at it, it seems like a cool feature! Except perhaps for the style variation transform, those are already thumbnails, it feels a little duplicate. It's fine probably? And maybe we can revisit? |
Same it makes me wonder whether we should just show a simple list for the style variations too. |
This had been also discussed in my first GB PR: #23028. styles design issue: #27870 |
This is great — nice that it works with multi-transforms as well. |
Description
Resolves: #26399
This PR adds previews to block transforms.