UI Components: Add theme support for FormTokenField and ComboboxControl
#69638
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. |
|
Hi @ciampo, following our previous discussions, here's a PR adding theming support to both |
| @@ -12,6 +12,8 @@ input.components-combobox-control__input[type="text"] { | |||
| margin: 0; | |||
| line-height: inherit; | |||
| min-height: auto; | |||
| background: inherit; | |||
There was a problem hiding this comment.
I wonder if we should use $components-color-background instead?
| border-color: $components-color-accent; | ||
| box-shadow: 0 0 0 0.5px $components-color-accent; | ||
| outline: 2px solid transparent; | ||
| outline-offset: -2px; |
There was a problem hiding this comment.
Moving away from shared mixins is not great (especially since we're using the input-control mixing just a few lines above) but I understand that input-style__focus() doesn't support components theme colors.
A few ideas to consider:
- re-define the
--wp-admin-theme-colorCSS variable only in the scope of this specific CSS class, and re-use the mixin; - tweak existing mixins to support custom accent colors
Otherwise, if really need to resort to custom code, let's at least add comments:
- explaining that this code mirrors the
input-style__focus()mixin; - explaining the reason for the
0.5pxbox shadow - explaining that the invisible outline is for Windows High Contrast mode
There was a problem hiding this comment.
Thanks for the review, @ciampo.
tweak existing mixins to support custom accent colors
It made more sense to go with this approach, and I’ve implemented it in the latest commit. Thanks!
a94bb84 to
c5eb443
Compare
| @@ -151,10 +151,10 @@ | |||
| } | |||
|
|
|||
|
|
|||
| @mixin input-style__focus() { | |||
| border-color: var(--wp-admin-theme-color); | |||
| @mixin input-style__focus($accent-color: var(--wp-admin-theme-color)) { | |||
There was a problem hiding this comment.
I wonder if we should expose the same $accent-color argument in the input-control mixin too (in the same file) (also cc @mirka for a quick check about this idea making sense)
There was a problem hiding this comment.
I'm a bit conflicted, now that you bring it up. I'm looking at how these generic mixins are used in the repo, and it's mostly just the components package. Almost as if these shared styles should be internal to the components package now and deprecated from base-styles — I assume they were once used more in other packages, and we've been able to remove them as we've consolidated our component usage better.
However, I like what @yogeshbhutkar is doing here, and I think it's fine to do it in the input-control mixin too, like you suggest. I'm just not sure if that scales to "let's do it for all the base-style mixins with accent colors".
I don't have a strong preference either way, given that I want to eventually overhaul these bits.
There was a problem hiding this comment.
Agreed. In a perfect world, @wordpress/component would offer a set of input components that can satisfy all usecases in Gutenberg, allowing us to move the mixing to the component package.
At the same, we need to consider whether the base-style package should exist as a lower-level (even lower that the components package) for literally any UI build in Gutenberg, even when the components from the @wordpress/components package can't be used
84a459a to
4c9319b
Compare
…ntrol` (WordPress#69638) * feat: add dark theme support for `form-token-field` * feat: add dark theme support for `combobox-control` * chore: add CHANGELOG * chore: use consistent spacing * refactor: update `input-style__focus` to support custom accents * feat: enhance `input-control` mixin to accept custom accent colors * chore: move `CHANGELOG` to unreleased section --- Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: mirka <0mirka00@git.wordpress.org>
What?
Part of #69646
Follow up to #69621
Related: #69593 (comment)
This PR introduces theming support for the
FormTokenFieldandComboboxControlcomponents.Why?
Previously, these components lacked dynamic theming, causing visibility issues in dark mode. This update ensures proper theme adaptation for improved readability and user experience.
How?
The corresponding SCSS files have been updated to use theming.
Testing Instructions
npm run storybook:dev)FormTokenFieldcomponent.ComboboxControlcomponent, and repeat step 3.Testing Instructions for Keyboard
Same
Screenshots
Combobox
FormTokenField