-
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
Extract CSS resets to reusable mixins #14509
Conversation
box-sizing: inherit; | ||
} | ||
|
||
select { |
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.
Ideally for reusable components using inputs/selects... we shouldn't rely on the existence of these resets because this "reset" mixins are not included in the styles of these reusable components which means if you use a given component outside of the Gutenberg context, it may not be shown properly.
This is a separate task though that should be done step by step.
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 works well in my testing! Just left a couple super-minor comments inline. Once we double check that post title stuff, this should be good to go. 👍
@@ -24,6 +24,7 @@ | |||
border: $border-width solid transparent; | |||
border-left-width: 0; | |||
border-right-width: 0; | |||
border-radius: 0; |
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.
In my testing (FF, Chrome, Safari for Mac), this border-radius
rule seems to be the only change that's needed in this file: if I remove the changes on L10
& L53-64
, this still seems to work just fine. Are we sure we need those updates?
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.
L10 is probably not needed, I'll check but the focus style is needed in the "Spotlight Mode"
} | ||
} | ||
|
||
input[type="radio"] { |
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.
Can I get a ✅ here :) |
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.
LGTM! Thanks, @youknowriad.
Thanks for the review @kjellr |
// Stack borders on mobile. | ||
border: $border-width solid transparent; | ||
border-left-width: 0; | ||
border-right-width: 0; |
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.
👍 PR to fix it #14572
input[type="month"], | ||
input[type="number"], | ||
select, | ||
textarea { |
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 think this may have caused a regression on the appearance of the "Code Editor" textarea, where the reset of .block-editor textarea
in assigning font-family
takes specificity priority over the code styling applied by the .editor-post-text-editor
selector.
gutenberg/packages/edit-post/src/style.scss
Lines 64 to 68 in 70cc953
.block-editor, | |
// The modals are shown outside the .block-editor wrapper, they need these styles | |
.components-modal__frame { | |
@include reset; | |
} |
.editor-post-text-editor { |
(011
vs. 010
)
Before | After |
---|---|
Aside: Should .block-editor
styles be applied at edit-post/src/style.scss
?
Noting that with this change, all the rules related to inputs now apply also to the metaboxes area and to the custom fields area, see for example Yoast/wordpress-seo#13316 The reason is simple:
In other words, this change breaks UIs provided by plugins. It should either be communicated to plugin authors well in advance so they can adjust their CSS or (preferably) the new mixin should be better scoped. |
Global CSS resets are a bad idea and the more we move away from them, the better is for reusability.
That said, the current resets we have are not very well organized and can't be reused across pages.
With the new widgets page, we need to share these resets between the editor and the widgets page. This PR extract these resets (not all of them) to two reusable mixins:
Testing instructions