-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Quote Block: Fix the Quote block layout supports. #55240
Conversation
Warning: Type of PR label error To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. |
1 similar comment
Warning: Type of PR label error To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. |
This pull request contains almost identical changes to the following pull request: |
This is working well for me, thanks for creating the PR @shimotmk I'd be more confident with a second ✅ from @tellthemachines or @andrewserong |
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.
Thanks for the PR! LGTM and works as expected.
I like the idea here, too, thanks for working on it! One potential challenge is that this will also affect the spacing between blocks in block markup that's out in the wild as currently (prior to this PR) no layout spacing rules are applied for Quote block children. Is that an issue? Arguably, for sites that use the layout spacing rules, I think this PR should provide more consistency as the spacing will be the same as if the blocks were wrapped in a Group block. So, while it might create a difference, that difference could arguably be thought of as a bug fix, too 🤔 |
Good question! I'd say not necessarily. If themes have provided custom spacings styles for the block they will likely have higher specificity than the default layout styles so there won't be any visible difference. If themes haven't provided those styles I'd assume they're good with whatever the default is, whether that's browser or core styles 😄 But that does raise the question of whether we should add explicit support for spacing to this block, so folks can change it if the default isn't to their liking. |
Great point, with the lower specificity in place, that does sound safer now than it would have been in the past 👍
Good idea, I think that'd be a good thing to add, and together those two points probably mitigate any major backcompat concerns as it gets the block to behave in a way that's more consistent with layout spacing styles overall, while giving users (and themes) more control. If not this PR, we could always add in the This PR is testing nicely for me otherwise! |
Thanks for the input @andrewserong! I'm going to go ahead and merge this, then we can add block gap support separately. |
What?
Fix #55076
Quote Block: Fix the Quote block layout supports.
trunk
quote.mp4
this branch
trunk.mp4
Testing Instructions
or paste the HTML below.
Added block supports in the same way as the Details block🗒
gutenberg/packages/block-library/src/details/block.json
Lines 59 to 61 in 1aae9e8