Apply radius scale in the editor #64930
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
jameskoster
added
General Interface
jameskoster
requested review from
ellatrix,
getdave,
artemiomorales,
geriux and
ajitbohra
as code owners
|
Warning: Type of PR label mismatch 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. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
1 similar comment
|
Warning: Type of PR label mismatch 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. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
|
Size Change: -112 B (-0.01%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
tyxla
approved these changes
Sep 2, 2024
| @@ -114,12 +114,12 @@ ul.dataviews-view-list { | |||
| &::before { | |||
| position: absolute; | |||
| content: ""; | |||
| top: calc(var(--wp-admin-border-width-focus) + 1px); | |||
| top: var(--wp-admin-border-width-focus); | |||
There was a problem hiding this comment.
🤔 Was this change intended for another PR?
There was a problem hiding this comment.
This one is tiny visual bug I observed while testing. Figured it was okay to include here, but can also separate it out if y'all prefer.
There was a problem hiding this comment.
Up to you, really. I'd prefer a separate PR, but no strong feelings either way, so leaving to your judgment.
| @@ -200,7 +200,7 @@ html.canvas-mode-edit-transition::view-transition-group(toggle) { | |||
| right: 9px; | |||
| bottom: 9px; | |||
| left: 9px; | |||
| border-radius: $radius-block-ui + $border-width + $border-width; | |||
| border-radius: $radius-medium; | |||
There was a problem hiding this comment.
Wow, that was akward; thanks for fixing!
| @@ -120,7 +120,7 @@ | |||
| overflow: hidden; | |||
|
|
|||
| .edit-site-layout:not(.is-full-canvas) & { | |||
| border-radius: $radius-block-ui * 4; | |||
| border-radius: $radius-large; | |||
| @@ -16,7 +16,7 @@ | |||
| cursor: ew-resize; | |||
| outline: none; | |||
| background: none; | |||
| border-radius: $radius-block-ui; | |||
| border-radius: $radius-full; | |||
There was a problem hiding this comment.
I'm assuming we're intentionally going with radius-full here?
There was a problem hiding this comment.
Yes, this is consistent with similar elements in the components package that we want to be fully rounded. E.g: https://github.com/WordPress/gutenberg/blob/trunk/packages/components/src/progress-bar/styles.ts#L43
| @@ -3,7 +3,7 @@ | |||
| } | |||
|
|
|||
| .editor-post-locked-modal__avatar { | |||
| border-radius: $radius-block-ui; | |||
| border-radius: $radius-round; | |||
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
I'm happy to land this as an improvement. The corners of segmented control active items get a bit sharp though, at 1px, and they diverge from the 2px that are applied to toggle buttons (i.e. bold, etc). I recognize there's a separate conversation about the scale at play there, though. |
|
The toggle button active state has a 1px radius for neater visual nesting. We can increase/decrease it of course, but that will mean tweaking other values in the scale. Once this PR is merged we can open a new one to try that very quickly; it's just two files to edit now rather than hundreds as before.
The block toolbar needs updating, but as noted in the OP that particular UI should be tackled independently in a separate PR. |
|
I'm going to merge this one. There's an enormous amount of tidy up included, and the earlier we merge before beta the more likely we are to spot any outlying problems. |
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.
What?
This PR applies the radius scale to the block editor UI.
Note
As it is something of a branding element I've intentionally left the block toolbar out of this work. It warrants isolated consideration.
The changes typically fall into one of the following categories...
Adopting the scale / visual changes
Where relevant, elements have been modified to adopt the radius scale. In terms of visual changes there weren't too many cases in this category because the components package already adopted the new scale in #64368. One example is anchors in list view:
Otherwise the vast majority of updates here involve a straight swap from
$radius-block-uitoradius-small.Removing obsolete style declarations
Working through the code I found a few instances of
border-radiusbeing applied to elements which did not visually appear in the UI, most likely because the design has changed since the original implementation. Instead of leaving the obsolete code in place I removed it.Similarly, there were numerous instances where
border-radiuswas unnecessarily duplicated as a style override, even though the underlying component already provided the required styling. Buttons and Inputs were the main offenders here. Again, instead of leaving the obsolete code I removed it.Miscellaneous visual changes
In #60757 block outlines were updated to remove rounded corners. However, a number of on-canvas outlines remained rounded which felt inconsistent. One example is the dashed outline one finds in empty Row/Stack blocks. In the interest of consistency I removed these radii.
Testing Instructions