-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update hard-coded border-radius instances #64693
Conversation
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. |
@@ -102,7 +102,6 @@ | |||
|
|||
.block-editor-global-styles-background-panel__image-tools-panel-item { | |||
border: $border-width solid $gray-300; | |||
border-radius: 2px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The radius here is already added to the parent element.
@@ -553,7 +553,6 @@ svg { | |||
background-color: #1e1e1e; | |||
color: #fff; | |||
margin: $grid-unit-10 0 0 24px; | |||
border-radius: 2px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Radius here is inherited from the Button
component.
@@ -100,25 +100,6 @@ $components-custom-gradient-picker__padding: $grid-unit-20; // 48px container, 1 | |||
height: 20px; | |||
} | |||
|
|||
.components-custom-gradient-picker .components-custom-gradient-picker__toolbar { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class doesn't seem to be used at all.
@@ -43,7 +43,7 @@ $resize-handler-container-size: $resize-handler-size + ($grid-unit-05 * 2); // M | |||
// This is the "visible" resize handle for side handles - the line | |||
.components-resizable-box__side-handle::before { | |||
display: block; | |||
border-radius: 2px; | |||
border-radius: $radius-full; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensures the handle is fully rounded regardless of any future width/height changes.
.edit-site-layout.is-full-canvas { | ||
.edit-site-layout__view-mode-toggle.components-button { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far as I can tell, .edit-site-layout__view-mode-toggle
will never render inside .edit-site-layout.is-full-canvas
.
border-radius: 2px; | ||
|
||
.page-patterns-preview-field__button { | ||
border-radius: 2px; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any reason for the preview thumbnail to have a different radius based on layout selection.
Size Change: +218 B (+0.01%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup.
The x-small radius feels so small it's barely perceptible. I'm not sure that has value to have, but that's not a strong opinion. Happy to try this.
@@ -89,7 +89,6 @@ | |||
} | |||
|
|||
&:focus-visible { | |||
border-radius: 2px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping or removing the radius is totally a design decision, but I think that whatever we decide should likely be applied to Tabs
too (which is still private, but will ultimately replace TabPanel
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
@@ -102,7 +102,6 @@ | |||
&.is-error { | |||
.components-form-token-field__token-text { | |||
color: $alert-red; | |||
border-radius: 4px 0 0 4px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed — also, I don't see why there should be a different radius (4px) compared to the default (2px)
Thank you for the great work on style consistency 👍 In 9a71ac5 I've pushed another missed instance of |
Thanks @tyxla :) |
@jameskoster are we good to merge? Just asking while there are no conflicts 😂 |
Yes I think so 🚀 |
Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org>
What?
Update any instances of hard-coded border radii to utilise the new radius scale (#63703).
Why?
Code cleanliness, ease of maintenance.
Screenshots
There are only two visual change here;
$radius-x-small
is applied to tokens inFormTokenField