-
Notifications
You must be signed in to change notification settings - Fork 4.5k
TextControl: Improve theming support #70910
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
Conversation
$components-color-dark-gray-placeholder: color-mix(in srgb, $components-color-foreground, transparent 38%); | ||
$components-color-light-gray-placeholder: color-mix(in srgb, $components-color-background, transparent 35%); |
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.
Matches the CSS-in-JS equivalents:
gutenberg/packages/components/src/utils/colors-values.js
Lines 66 to 68 in a2cb1b0
// Matches @wordpress/base-styles | |
darkGrayPlaceholder: `color-mix(in srgb, ${ THEME.foreground }, transparent 38%)`, | |
lightGrayPlaceholder: `color-mix(in srgb, ${ THEME.background }, transparent 35%)`, |
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. |
Size Change: -200 B (-0.01%) Total Size: 1.89 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.
🚀
@@ -18,7 +18,12 @@ | |||
margin: 0; | |||
background: $components-color-background; | |||
color: $components-color-foreground; | |||
@include input-control( $components-color-accent); | |||
@include input-control( $components-color-accent ); | |||
border-color: $components-color-border; |
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.
Border color is already defined in the input-style__neutral
mixin included by input-control
. Should the themed color be moved to that mixin? Then it would apply to multiple components, everything that uses input-control
. Not sure how big issue that is.
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 themed variables are experimental and live at the @wordpress/components
level, so we don't want to expose them at the @wordpress/base-styles
level.
|
||
&::placeholder { | ||
color: $components-color-dark-gray-placeholder; | ||
} |
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.
Is there an existing style that styles the ::placeholder
? I see some non-standard pseudo-selectors like ::-webkit-input-placeholder
, I wonder why we don't use the standard one yet.
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 believe it's just old code and can be replaced. I guess the repo doesn't have the property-no-vendor-prefix
lint?
* TextControl: Improve theming support * Add changelog
What?
Makes the border and placeholder colors of
TextControl
themable.Why?
To expand themability of our basic components.
Testing Instructions
npm run storybook:dev
and go to the story forTextControl
. Use the theme switcher in the top toolbar to try the dark theme.Screenshots or screencast