Image Block: Add border radius support #27667
Conversation
aaronrobertshaw
added
[Type] Enhancement
|
Size Change: +155 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
|
Does this obviate the need for the Rounded style? |
|
@shaunandrews that's a good question. As it stands there is some unfortunate overlap between the border radius support and the rounded style. The rounded style does provide a theme developer with an easy option to apply a uniform radius to all images with that style applied, while this change allows more individualised ability to round corners. The rounded style also applies a very large radius beyond that offered by the border radius control at the moment. If the current border radius support is extended to allow a configurable max radius value and/or different units (e.g. pixels, percentage) we could remove the rounded style. I'm not sure how we could effectively replace the Happy to hear your thoughts on this one. |
|
The direction regarding border radius block support and new sidebar controls/panels has changed. Until the new sidebar controls & component system lands, the preferred approach is to allow this sort of functionality via theme.json and attributes only, avoiding adding new panels or controls to the editor sidebar. Given this PR is just a simple opt-in for the block support and a tweak of the CSS, the code hasn't changed, but the method of testing this should. The testing instructions have been updated accordingly. Not including the UI controls for the border radius support also reduces the awkwardness of also having the rounded block style for the time being. |
aaronrobertshaw
force-pushed
the
add/border-radius-to-image-block
branch
from
01f3319 to
be3581c
Compare
aaronrobertshaw
requested review from
ajitbohra,
chrisvanpatten and
mkaz
as code owners
|
Retested and confirmed this approach is working 👍 |
That is a very good point! |
aaronrobertshaw
force-pushed
the
add/border-radius-to-image-block
branch
from
9b9f676 to
5be5022
Compare
There was a problem hiding this comment.
This is testing well for me according to the instructions.
✅ Without an updated theme.json file the feature is not activated
✅ With an updated theme.json file, the border radius is applied to the default style
✅ The 'Rounded' style overrides the Default style and any custom border radius
✅ Custom border radius in the sidebar overrides the Default style
I noticed one issue, which is that if the border radius is set to a non-zero px in the theme.json file, then the custom border radius slider in the sidebar cannot set the border radius to 0 as 0 will result in no inline style being set on the image block. I can recreate the same issue if I set the border radius for the core/group block to a non-zero value in theme.json so this issue isn't particular to this PR.
If the current border radius support is extended to allow a configurable max radius value and/or different units (e.g. pixels, percentage) we could remove the rounded style
That would be a good enhancement to the border radius feature. I noticed if I set the border radius to, say, 150px, it felt strange that the custom override could only go up to 50px, rather than go all the way up to the value set in theme.json.
|
Thanks for reviewing this @andrewserong!
I can confirm this is currently a shortcoming in the implementation of the border radius support. Before updating the style attribute it passes the results through a call to
Expanding on the idea of using the theme.json for values, perhaps a more versatile option might be to allow configuration of the minimum and maximum border radius values via the block supports flags and theme.json. In the example give, you could set the border radius in theme.json to 150px but might actually want the max to be 200px. There could be a case for a theme developer also wanting to set a minimum value that isn't zero. Both of the above issues would be best addressed via a separate PR iterating on the border radius support. In the meantime, I'll merge this one. |
| @@ -22,6 +22,10 @@ figure.wp-block-image:not(.wp-block) { | |||
| margin-top: -9px; | |||
| margin-left: -9px; | |||
| } | |||
|
|
|||
| &:not(.is-style-rounded) > div { | |||
There was a problem hiding this comment.
Can this be serialized to the inner element (img) in the edit function by using __experimentalSkipSerialization? Adding JS/CSS/etc. to a block so it can use some block supports is a smell that we need to do something differently.
This is now relevant for this PR, in which we're making the block selector to target the inner img. See https://github.com/WordPress/gutenberg/pull/34073/files#r688580510
There was a problem hiding this comment.
I believe it can be serialized to the img element now. This wasn't possible when this PR was done as the __experimentalSkipSerialization option didn't exist yet.
There is actually already a PR that moves the border radius support styles to the inner image element as part of adopting border color and width support for the image block. #31366
I can sort out the style mentioned here as part of that.
Fixes: #26559
Description
Opts into border radius support for the image block.
How has this been tested?
Manually.
Testing Instructions
lib/experiment-default-theme.jsonfile into your active theme's root folder asexperimental-theme.jsoncore/imagecontext to your current theme.json file from step 2. It must be a top level property in the JSON as perglobal.border.customRadiusflagScreenshots
Editor:
Frontend:
Types of changes
Enhancement/New Feature
Checklist: