Skip to content

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

Merged
merged 9 commits into from
Jun 17, 2025
Merged

Conversation

matgalla
Copy link
Contributor

@matgalla matgalla commented Apr 15, 2025

Related Issue: #11713 #11711

Summary

Restores the --calcite-color-focus token in both light and dark semantic color JSON files.

Noteworthy changes

  • includes the new focus token in all outputs except index.(s)css
  • adds new components.css design tokens output for inclusion in calcite-components shadow DOM (host)
    • defines internal custom CSS property for use by focus utility styles (--calcite-internal-color-focus)
  • exports invert util from calcite-tailwind-preset (internal) – can be moved to internal shared utils
  • overrides Tailwind focus utilities in calcite-components – avoids depending on internal CSS props when using the preset
  • restores ts/color/css/hexrgba transform override to support focus color token value structure
  • extracts shared utilities across calcite-design-tokens format outputs
  • enhances dictionary utility to support filtered tokens
  • all components define the internal focus CSS prop, which might not be necessary in all cases – we can address this in Use styles object for shared component styles #12106 and only include it in components that are focusable.

BEGIN_COMMIT_OVERRIDE
omitted from changelog
END_COMMIT_OVERRIDE

@matgalla matgalla added the refactor Issues tied to code that needs to be significantly reworked. label Apr 15, 2025
@matgalla matgalla added this to the 2025-04-29 - Apr Milestone milestone Apr 15, 2025
@matgalla matgalla requested a review from jcfranco April 15, 2025 19:40
@matgalla matgalla self-assigned this Apr 15, 2025
@jcfranco jcfranco changed the title refactor(tokens): add focus color token fix(tokens): restore --calcite-color-focus Apr 15, 2025
@jcfranco jcfranco added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Apr 15, 2025
@jcfranco jcfranco marked this pull request as ready for review April 15, 2025 19:52
@jcfranco jcfranco requested a review from alisonailea as a code owner April 15, 2025 19:52
@jcfranco jcfranco added the no changelog entry Use the commit override to avoid a changelog entry label Apr 15, 2025
@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Apr 15, 2025
@jcfranco jcfranco added skip visual snapshots Pull requests that do not need visual regression testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Apr 15, 2025
Copy link
Contributor

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.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Apr 26, 2025
@github-actions github-actions bot removed the Stale Issues or pull requests that have not had recent activity. label Apr 30, 2025
@geospatialem geospatialem removed this from the 2025-04-29 - Apr Milestone milestone Apr 30, 2025
@jcfranco
Copy link
Member

jcfranco commented May 1, 2025

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.

Copy link
Contributor

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.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label May 12, 2025
@matgalla matgalla added this to the 2025-06-24 - Jun Milestone milestone May 12, 2025
@geospatialem geospatialem removed this from the 2025-06-24 - Jun Milestone milestone May 12, 2025
@jcfranco
Copy link
Member

jcfranco commented May 15, 2025

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.

One way we could approach this is to:

  1. add a new output file (e.g., component)
  2. exclude it from the components build (e.g., index or core token outputs)
  3. use it in the Tailwind preset as the fallback value for focus utils (example) via the es6 output:
    import { calciteColorFocus } from "@esri/calcite-design-tokens/dist/es6/component";
    // ...
    ".focus-normal": {
       outline: `2px solid var(--calcite-color-focus, var(--calcite-ui-focus-color, ${calciteColorFocus}))`,
     },

We could alternatively include it in global/semantic output, but we would need to avoid adding it to :root (default), which would have the same issue seen above.

Thoughts? cc @matgalla @macandcheese @driskull @alisonailea

@github-actions github-actions bot removed the Stale Issues or pull requests that have not had recent activity. label May 16, 2025
Copy link
Contributor

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.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label May 24, 2025
@matgalla
Copy link
Contributor Author

@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.

@github-actions github-actions bot removed the Stale Issues or pull requests that have not had recent activity. label May 28, 2025
@jcfranco
Copy link
Member

@matgalla Yes, sorry for not adding that detail earlier. The component output file would host semantic tokens that need to support being overridden at the component level due to fallback behavior.

The main issue is that the CSS output defines most custom CSS props on :root, which limits overriding/fallback patterns that need to be scoped to the component, like in this case.

.And we don't need the focus color variable from a design/Figma standpoint, so no need to go that route.

Definitely an option. The main purpose was to restore the token representing the design decision for focus to use the brand color.

Copy link
Contributor

github-actions bot commented Jun 5, 2025

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.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Jun 5, 2025
@github-actions github-actions bot removed the Stale Issues or pull requests that have not had recent activity. label Jun 17, 2025
@jcfranco jcfranco requested a review from benelan as a code owner June 17, 2025 07:19
@jcfranco jcfranco added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jun 17, 2025
@jcfranco jcfranco added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. skip visual snapshots Pull requests that do not need visual regression testing. labels Jun 17, 2025
@jcfranco jcfranco requested review from driskull and macandcheese and removed request for alisonailea June 17, 2025 18:16
Copy link
Member

@driskull driskull left a 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)",
Copy link
Member

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)

Copy link
Member

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR submitted! #12348

Copy link
Contributor

@macandcheese macandcheese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏅

@jcfranco jcfranco merged commit 8bc246f into dev Jun 17, 2025
16 checks passed
@jcfranco jcfranco deleted the matgalla/add-focus-color-token branch June 17, 2025 18:46
@github-actions github-actions bot added this to the 2025-06-24 - Jun Milestone milestone Jun 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug reports for broken functionality. Issues should include a reproduction of the bug. no changelog entry Use the commit override to avoid a changelog entry pr ready for visual snapshots Adding this label will run visual snapshot testing. refactor Issues tied to code that needs to be significantly reworked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants