-
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 Previews: dynamic preview size #19987
Conversation
I like the idea of being able to customize the preview in this way. Because of the way the |
@@ -251,6 +251,12 @@ export class InserterMenu extends Component { | |||
! isEmpty( itemsPerCollection ); | |||
const hoveredItemBlockType = hoveredItem ? getBlockType( hoveredItem.name ) : null; | |||
const hasHelpPanel = hasItems && showInserterHelpPanel; | |||
const viewportWidth = |
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.
@scruffian Neat! Prop pass through with value fallback :)
I lack a bit of project context for this update. In isoloation, the change makes sense to me.
For implementation, I feel like we can clean up the &&
checks by using lodash.get (since the package already has lodash
as a dependency)
That way, you can do something like this:
import { get } from 'lodash'
const viewportWidth = get(hoveredItemBlockType, 'example.viewportWidth', 500)
Would love your feedback 😊
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.
Great idea thanks! Done in ec596e0.
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.
@ItsJonQ Curious, does gutenberg support the optional chaining operator (meaning, is it transpiled in the build tools for browser compatibility)?
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.
@creativecoder Good question! I remember this was mentioned recently. It looks like something was merged #19831
I haven't personally tried it yet. @gziolo Is this project optional chaining ready? :D
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 preview is supposed to scale to the container width no matter the "viewportSize". Why do you think it's not happening with this block? |
@youknowriad that's not the behaviour I observe when I've used it for any blocks - the viewportWidth sets a scale for the block, so that with a large viewportWidth text is scaled down. Is that not how it should work? |
@scruffian As you can test with the Core blocks (columns, image, cover,...), the preview is scaled to match the panel size. It seems like there's something going wrong with the blocks you're trying to preview. Should we dig deeper to try to understand why it doesn't behave as expected? |
Ok, I think I got to the bottom of this. The blocks I was trying to preview were in |
Description
This allows block to specify the viewport width for their example. This is useful because some block previews look better at smaller viewport sizes.
How has this been tested?
In chrome.
Screenshots
Before:
After:
Types of changes
New feature
Checklist: