Block Support: Update border support to allow non-pixel units #31483
Conversation
aaronrobertshaw
added
[Type] Enhancement
aaronrobertshaw
requested review from
ellatrix,
spacedmonkey and
TimothyBJacobs
as code owners
|
@nosolosw Given the border block support feature is still experimental, can you confirm this change is ok to make without still handling previous unitless values? Or am I missing something? |
|
Size Change: +234 B (0%) Total Size: 1.03 MB
ℹ️ View Unchanged
|
|
I tried this and found the Button block doubles up on units. Simple changes in the Buttons block can have it working. I'm not sure that there'd be no migration concern though. Otherwise, this worked well for me though I could only test it with the group block. For some reason, opting-in to support on the Post Tags block didn't work on my end. Incidentally, it looks like this PR would resolve #29210. |
|
Thanks for testing @stokesman
The button block does skip the usual serialization of the block support styles for border radius. When it applies the radius manually it appended the
You are correct it is just a simple change to that block. I'll update that as part of this PR. Regarding the migration concerns, I believe the issue might only be for dynamic blocks. In static blocks, when the block support style was applied, it was just set to the number value and the px unit was applied automatically, so was in the saved content. Static blocks that had a border radius value without a specified unit would continue to function as they were. If the radius was adjusted the attribute value would then have a unit, which would be then saved into the content as expected. I'll test and confirm all of this. Dynamic blocks had the Also, looking at other blocks that have skipped serialization of the block support style for border radius, the search block might be a concern. Depending on its layout, button inside vs outside, it will apply the border radius differently. It relied on the fact the value was only in pixels to calculate the adjustment. It might be better to address that via a separate PR once non-pixel values are allowed for the border radius. |
|
The fixes for the button and search blocks were relatively straightforward so I've ended up including them in this PR so there is no gap between this landing and those blocks working with non-pixel units. The search block solution required inlining a style using I've searched for other blocks opting into border support and of those they all work for me without validation errors when loading a deprecated version in the editor or frontend. These blocks include; button, search, table, image & group. |
|
I tested this and from a functional prespective everything worked as advertised: I will leave it to someone with more experience in the block supports to give it s full code review signoff. |
|
I would like to comment on this topic once more and express that, in the border drop-down menu there should be a NONE above solid. |
|
Thanks for the feedback @cgaits 👍 This PR is only concerned with being able to allow custom CSS units for the border width and border radius block support. There has been some discussion regarding a "none" option for the border style support on the issues relating to refining the border block support UI. #31337 & #31333. The most recent proposal is that the border width be responsible for disabling the border i.e. a border width of This will be addressed as part of #31585 |
|
Also, in case it interests @cgaits, the original UI (which is the version in this PR) does actually have a "none" option in the border style dropdown above "solid".
|
|
I linked #29210 since this PR happens to resolve it and want to suggest that the behavior (defaulting to empty radius value) is correct and should be maintained here. |
|
Tested according to instructions (on web only) in FF, Chrome and Safari. Group block border controls appear as expected in both editor and fronted for all units, and in the absence of a value. Similarly, the search block border radius functions as expected.
Not a requirement for this PR, but just jotting it down for reference: now that the range control is absent, and there's no UI to indicate how one is able to adjust controls without the keyboard, would a cheap way be to add a cursor? E.g., using |
|
Thanks for testing @ramonjd !
I do see a cursor but only after I commence dragging up/down on the control. I'm not sure how best we can improve that UX. Changing the default cursor would obscure that the field is keyboard editable. Either way, this is specific to the |
Button block previously appended the `px` unit when manually applying the border radius style. This change removes that as the unit will be included in the block support attribute for border radius.
aaronrobertshaw
force-pushed
the
update/border-support-for-custom-units
branch
from
e4b68e8
to
41acd39
Compare
Part of: #31337
Related: #31641, #31585
Description
Given border block support is still
__experimentalno deprecation has been added for the moment.How has this been tested?
Manually.
experimental-theme.json. To save time enabling it for multiple block contexts, just update thesettings.defaults.borderflags.UnitControl.block.jsonfile.Screenshots
Types of changes
New feature.
Breaking Change (Border block support is still experimental however)
Checklist:
*.native.jsfiles for terms that need renaming or removal).