Block Support: Update border support UI #31585
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aaronrobertshaw
added
[Type] Enhancement
7 tasks
aaronrobertshaw
force-pushed
the
update/border-block-support-ui
branch
from
58adbc4
to
5f56717
Compare
|
Size Change: +13.7 kB (+1%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
aaronrobertshaw
changed the base branch from
update/border-support-for-custom-units
to
update/border-radius-support-custom-corners
aaronrobertshaw
force-pushed
the
update/border-block-support-ui
branch
from
cbbc3d1
to
975097b
Compare
This was referenced May 12, 2021
|
The |
This was referenced May 21, 2021
|
Sorry for missing this one. Based on the GIF, this is a great improvement. Thanks for working on this. The ability to have per-corner custom radii is a very compelling feature. |
Not a problem at all. I deliberately left this as only a WIP draft as I wasn't sure whether the two PRs it depends on would force any changes. It seemed a little premature to flag it for full review until the custom units and per corner radii changes were merged. Hopefully those will be ready to merge in the near future. |
|
Loving this so far. Editor and frontend look as expected. Inline styles on the frontend checkout.
|
15 tasks
aaronrobertshaw
force-pushed
the
update/border-radius-support-custom-corners
branch
from
677a9a8
to
6408a75
Compare
aaronrobertshaw
force-pushed
the
update/border-block-support-ui
branch
from
6428068
to
41cb528
Compare
7 tasks
aaronrobertshaw
force-pushed
the
update/border-radius-support-custom-corners
branch
from
706321b
to
9b8f2c7
Compare
aaronrobertshaw
force-pushed
the
update/border-block-support-ui
branch
from
41cb528
to
f276e8c
Compare
aaronrobertshaw
changed the title
|
I've rebased and re-tested this PR after recent changes to #31641. It still works for me. |
Base automatically changed from
update/border-radius-support-custom-corners
to
trunk
June 18, 2021 08:50
aaronrobertshaw
force-pushed
the
update/border-block-support-ui
branch
from
f276e8c
to
1dc8a7e
Compare
aaronrobertshaw
force-pushed
the
update/border-block-support-ui
branch
from
c79a569
to
67e6dc8
Compare
glendaviesnz
approved these changes
Jul 7, 2021
ramonjd
reviewed
Jul 7, 2021
There was a problem hiding this comment.
I took the UI for a run in Chrome/FF/Safari, and the controls work and display as expected in the block editor and frontend 👍
I don't know if this PR should be concerned or whether it's a theme responsibility, but I checked in a couple of themes, and in Twenty Nineteen at least, the theme styles are overriding the primary and secondary border colors (e.g., .entry .entry-content .has-primary-background-color):
Editor
Frontend
packages/block-editor/src/components/border-radius-control/utils.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/border-radius-control/utils.js
Outdated
Show resolved
Hide resolved
|
Thank you @glendaviesnz & @ramonjd for the reviews 👍
This PR is primarily concerned with the UI around the existing border block support. My thinking is that it will be best to keep the border block support consistent with other block support features in terms of style specificity. If there is a particular reason the TwentyNineteen theme can't be updated to address its overwriting of border color styles, we can address it in a separate PR. I've made the requested tweaks as well as updated the border width control to use the new default step values for each unit. Let me know if I've missed anything. |
ramonjd
approved these changes
Jul 7, 2021
|
After merging this is working well for existing blocks that leverage border support. BorderRadius-ShorthandVsLonghandStyles.mp4 |
oandregal
reviewed
Jul 8, 2021
| ! hasDefinedValues( values ) || ! hasMixedValues( values ) | ||
| ); | ||
|
|
||
| const units = useCustomUnits( { availableUnits: [ 'px', 'em', 'rem' ] } ); |
There was a problem hiding this comment.
Does this need to use the spacing.units or layout.units available via theme.json? I've noticed it's one of the few places where these aren't used so are not configurable by the theme.
There was a problem hiding this comment.
Good point.
I can see theme developers wishing to allow different units for borders to those of spacing or layouts as well. A PR allowing configuration of border units can be found in #33315.
sarayourfriend
pushed a commit
that referenced
this pull request
Jul 15, 2021
Refine the controls provided via the border block support feature.
12 tasks
7 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of: #31337
Related: #32080
Depends on: #31483 & #31641Description
This PR refines the border block support UI in line with #31337.
These refinements are separate to migrating the border block support to use the new progressive display panel proposed in #32392. Converting to this new panel will be best handled separately.
The condensed color control proposed in the G2 components still requires some additional dependencies being imported into Gutenberg so will also be addressed in a future PR as soon as the control is available.
How has this been tested?
Test instructions
Screenshots
Types of changes
New feature.
Checklist:
*.native.jsfiles for terms that need renaming or removal).