Spacing Support: Add margin block support with configurable sides #30608
Conversation
aaronrobertshaw
added
[Type] Enhancement
|
Size Change: +228 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
aaronrobertshaw
force-pushed
the
update/padding-support-with-configurable-sides
branch
from
fac2ed0 to
f6668f7
Compare
aaronrobertshaw
force-pushed
the
add/margin-support-with-configurable-sides
branch
from
8ae61ea to
faa04c8
Compare
aaronrobertshaw
force-pushed
the
update/padding-support-with-configurable-sides
branch
from
f6668f7 to
786f915
Compare
aaronrobertshaw
force-pushed
the
add/margin-support-with-configurable-sides
branch
from
faa04c8 to
c09835c
Compare
aaronrobertshaw
force-pushed
the
update/padding-support-with-configurable-sides
branch
from
786f915 to
99995d3
Compare
aaronrobertshaw
requested review from
ajitbohra and
chrisvanpatten
as code owners
aaronrobertshaw
force-pushed
the
add/margin-support-with-configurable-sides
branch
2 times, most recently
from
2029c03 to
0fe1917
Compare
aaronrobertshaw
force-pushed
the
update/padding-support-with-configurable-sides
branch
from
c4144eb to
d3588ea
Compare
aaronrobertshaw
force-pushed
the
add/margin-support-with-configurable-sides
branch
from
0fe1917 to
dfe3ff8
Compare
|
This has been rebased after the changes on trunk to padding.js and spacing-panel.js fixing custom CSS units. |
|
I see this PR has all checks green (including the approval) but I don't see anyone's approval on the reviewers' roster. Did we break GitHub? Going to give it a shot anyway. Feel free to merge if there was a previous approval. If I find anything worth fixing, we can always prepare follow-ups. |
|
@nosolosw I think it's because it's not targeting trunk yet. |
|
I left a comment at #30608 (comment) Other than that, it looks fine. Though that concern doesn't seem to belong in this PR, as I've just realized this wasn't based on |
aaronrobertshaw
force-pushed
the
add/margin-support-with-configurable-sides
branch
2 times, most recently
from
badcb34 to
07b49e7
Compare
I'd be happy to help make steps towards this happening though unfortunately I'm not very clear on any progress, or the plan to integrate G2 components into Gutenberg as per the screenshot. Similar issues arise with the border support panel refinements requested in #31337.
There are a number of issues blocked by not having the ability to configure margins. Getting this one merged and then iterating on the UI would definitely get my vote. |
The designs in #27331 are not reliant on any G2 components being added to the block editor. The system is entirely compoment agnostic, and is purely an expression of which design tools are surfaced in which panels, according to block supports. |
Thanks for the additional info. I'll look into it. Perhaps I was taking the design screenshots too literally. |
I mean, we'll need some padding, margin, CSS, styling updates, and ideally we have a system that helps float/flex/rearrange/correctly place the form controls based on what's supported and so on, in a 2 or 3 column system. But there are steps that can be taken before that. |
Thanks @MaggieCabrera, you should be able to test this margin support via the theme.json for the separator block now. In case it helps, here is the theme.json I used while testing. |
Yeah, I was able to see the spacing menu on the editor, what I meant was that I couldn't do something like this on my theme.json file: This is going to be important for certain blocks so that the theme can set some good default values and we can actually do it for the padding right now. |
|
@MaggieCabrera sorry if I wasn't clear in my comment. I meant you could now do exactly what you were expecting in setting the margins within the theme.json styles. The linked gist actually set the top and bottom margins. "styles": {
"blocks": {
"core/separator": {
"spacing": {
"margin": {
"top": "100px",
"bottom": "100px"
}
}
}
}
}Please let me know if there are any other issues you were facing. |
Yeah you are totally right, it was a theme issue not letting me see the changes. I tested again with your theme.json file using emptytheme and I can see the margins on the frontend but the editor shows 15px margin on the separators isntead of 100px:
|
There was a 15px default value applied to the separator block specifically that was overriding the theme value. That was removed in 0f57553 before I advised it should be right to test again. Can you confirm if you pulled the latest changes from #30609 before re-testing? This is what I see when loading the editor with the theme.json gist linked above, no defaults in the control.
|
|
It's also worth mentioning there isn't a means at present to determine the default merged ( default, theme & user ) style value to populate the block support controls within the block editor, only the full site editor. If we wish to address that, it's an issue for all current block support controls and not unique to this PR. A separate PR would be best to discuss that further if desired. |
Yes, I pulled the latest from the branch and it works nicely, awesome!! Thanks for your help and for the PR, can't wait to be able to play this :D |
|
I think the dimensions UI redesign can be considered a follow-up. |
aaronrobertshaw
force-pushed
the
add/margin-support-with-configurable-sides
branch
from
f976e9e to
a3c0200
Compare
|
I've rebased again to resolve a conflict. Should be ok to merge once tests pass. Then we can iterate on the dimensions UI redesign as suggested. |
Description
This adds margin block support with the configurable sides feature provided in #30606.
The intention is to merge this into #30607 so that both the updated padding support and this new margin support are consistent.
How has this been tested?
Manually.
Test Instructions
Easiest means of testing these changes is to checkout #30609 and follow its test instructions.
Screenshots
Types of changes
New feature.
Checklist:
*.native.jsfiles for terms that need renaming or removal).