-
Notifications
You must be signed in to change notification settings - Fork 79
fix(tokens): restore --calcite-color-focus
#11960
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
--calcite-color-focus
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions. |
Belated update, taking another look at this once since the current updates will reintroduce a regression since it won't allow component-level overrides to take place. |
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions. |
One way we could approach this is to:
We could alternatively include it in global/semantic output, but we would need to avoid adding it to Thoughts? cc @matgalla @macandcheese @driskull @alisonailea |
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions. |
@jcfranco The proposed solution seems okay to me, just maybe a bit odd to have a file dedicated to a single token. Are there any other tokens that would potentially fit into that file eventually? And we don't need the focus color variable from a design/Figma standpoint, so no need to go that route. |
@matgalla Yes, sorry for not adding that detail earlier. The The main issue is that the CSS output defines most custom CSS props on
Definitely an option. The main purpose was to restore the token representing the design decision for focus to use the brand color. |
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions. |
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.
👍
"outline-color": "transparent", | ||
}, | ||
".focus-normal": { | ||
outline: "2px solid var(--calcite-internal-color-focus)", |
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.
should we use a var for 2px
? Maybe the border width var? var(--calcite-spacing-base)
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.
Good idea. I'll do that in a follow-up since I'll also have to update the preset utils.
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 submitted! #12348
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.
🏅
Related Issue: #11713 #11711
Summary
Restores the
--calcite-color-focus
token in both light and dark semantic color JSON files.Noteworthy changes
index.(s)css
components.css
design tokens output for inclusion incalcite-components
shadow DOM (host)--calcite-internal-color-focus
)invert
util fromcalcite-tailwind-preset
(internal) – can be moved to internal shared utilscalcite-components
– avoids depending on internal CSS props when using the presetts/color/css/hexrgba
transform override to support focus color token value structurecalcite-design-tokens
format outputsBEGIN_COMMIT_OVERRIDE
omitted from changelog
END_COMMIT_OVERRIDE