Skip to content
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

[WIP] JSON theme #4443

Draft
wants to merge 35 commits into
base: main
Choose a base branch
from
Draft

[WIP] JSON theme #4443

wants to merge 35 commits into from

Conversation

origami-z
Copy link
Contributor

@origami-z origami-z commented Nov 25, 2024

MVP - Make the script to generate CSS for theme-next, so there is no visual regression with minimal output change (e.g., variable syntax stays the same, CSS rule / selector stays the same, order of CSS variable may change)

The main goal of this PR is to extract tokens from Salt (Next) Figma libraries and write some custom Style Dictionary rules to generate CSS and then can be compared with existing /theme/css files.

Once discussions are settled, can merge 1 CSS a time (next theme first), starting from foundations to palette ... characteristics

#4041



Tokens

  1. Run "Export Color Styles" -> "Export Variables" -> "Json (Experimental)" from PR#51
  2. Select Collection and Mode on plugin UI
    1. Add "Additional Root Key (Optional)"
      • Colors library: 'color'
      • Palette library: 'palette'
  3. Copy output to corresponding *.tokens.json file
  4. Modify color tokens
    1. remove categorical. from , e.g. replace "{color.categorical to "{color
    2. added background. to a few background color raw tokens

TODOs

  • Custom value tranformer for foundation/alpha-next.css tokens, to achieve rgba syntax from packages/theme/css/foundations/alpha-next.css? Or this may not be needed given we generate these values
  • Better alpha foundation color generation, maybe use custom opacity modifier instead. Example of custom darken format
    • Can follow style-dictionary, use setAlpha from tinycolor2
  • Correct token types for non-colors, they're currently all "number". Refer to DTCG format types, e.g. "$type": "dimension"

Conversation Logs

Figma issue (likely)

  • gray/600/10a not same rgb as gray/600/40a
  • Not in Figma --salt-palette-accent-strongest
  • --salt-palette-background-secondary-disabled referencing charcoal not granite (compared between disabled/default)

Code issue

Shared discussion

  • Code doesn't have color 0a variables but has been using transparent in palette layer. e.g. --salt-color-blue-500-0a
  • Charcoal color not in code (next theme). What's correct for secondary bg (Charcoal vs Granite in code)
  • A couple of weakest token, --salt-palette-info-weakest (in Figma) vs --salt-palette-info-weak (in code), likely a breaking change needed
  • --salt-palette-neutral-weak-readonly (in Figma) vs --salt-palette-neutral-weaker-readonly (in code)
  • Logo brown should be in code or not?

Copy link

changeset-bot bot commented Nov 25, 2024

⚠️ No Changeset found

Latest commit: 4cc2d7f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Nov 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
saltdesignsystem ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 6, 2024 2:20pm

--salt-palette-accent-strong: var(--salt-color-blue-600);
--salt-palette-accent-strong-disabled: var(--salt-color-blue-600-40a);
--salt-palette-accent-stronger: var(--salt-color-blue-700);
--salt-palette-accent-stronger-disabled: var(--salt-color-blue-700-40a);
--salt-palette-accent-strongest: var(--salt-color-blue-800);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does not exist in Figma

--salt-palette-accent-disabled: var(--salt-color-blue-500-40a);
--salt-palette-accent-none: var(--salt-color-blue-500-0a);
Copy link
Contributor Author

@origami-z origami-z Nov 29, 2024

Choose a reason for hiding this comment

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

This is new in Figma, not in code

--salt-color-blue-500-0a didn't exist in code

.salt-theme.salt-theme-next[data-mode="light"] {
--salt-palette-alpha: var(--salt-color-black-30a);
--salt-palette-alpha-backdrop: var(--salt-color-white-70a);
--salt-palette-alpha-none: var(--salt-color-black-0a);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

--salt-color-black-0a vs transparent

--salt-color-background-snow-rgb: 255, 255, 255;
--salt-color-background-marble-rgb: 245, 247, 248;
--salt-color-background-limestone-rgb: 250, 248, 242;
--salt-color-background-gradientlight-rgb: 226, 228, 229;
Copy link
Contributor Author

@origami-z origami-z Nov 29, 2024

Choose a reason for hiding this comment

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

This color was fundamentally wrong #4463

--salt-color-background-marble: rgb(245, 247, 248);
--salt-color-background-limestone: rgb(250, 248, 242);
--salt-color-background-titanium: rgb(226, 228, 229);
--salt-color-background-charcoal: rgb(71, 76, 80);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Charcoal not in code

.salt-theme.salt-theme-next[data-mode="dark"] {
--salt-palette-background-primary: var(--salt-color-background-jet);
--salt-palette-background-primary-disabled: var(--salt-color-background-jet-40a);
--salt-palette-background-secondary: var(--salt-color-background-granite);
--salt-palette-background-secondary-disabled: var(--salt-color-background-granite-40a);
--salt-palette-background-secondary-disabled: var(--salt-color-background-charcoal-40a);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Likely Figma is wrong, default is granite the line above

.salt-theme.salt-theme-next[data-mode="light"] {
--salt-palette-info: var(--salt-color-blue-500);
--salt-palette-info-strong: var(--salt-color-blue-600);
--salt-palette-info-weak: var(--salt-color-blue-100);
--salt-palette-info-weakest: var(--salt-color-blue-100);
Copy link
Contributor Author

@origami-z origami-z Nov 29, 2024

Choose a reason for hiding this comment

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

Code is likely wrong, need to fix with deprecation. This can't be easily deprecate and fix, given weak may be needed afterwards. Should be a breaking change action. Noted in #1107

.salt-theme.salt-theme-next[data-mode="light"] {
--salt-palette-negative: var(--salt-color-red-500);
--salt-palette-negative-strong: var(--salt-color-red-600);
--salt-palette-negative-strong-disabled: var(--salt-color-red-600-40a);
--salt-palette-negative-weak: var(--salt-color-red-100);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Name weak - weakest

--salt-palette-neutral-weak: var(--salt-color-gray-400);
--salt-palette-neutral-weak-disabled: var(--salt-color-gray-400-40a);
--salt-palette-neutral-weak-readonly: var(--salt-color-gray-400-10a);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Net new in Figma, code only has --salt-palette-neutral-weaker-readonly, neither used by characteristics?

.salt-theme.salt-theme-next[data-mode="light"] {
--salt-palette-positive: var(--salt-color-green-500);
--salt-palette-positive-strong: var(--salt-color-green-600);
--salt-palette-positive-strong-disabled: var(--salt-color-green-600-40a);
--salt-palette-positive-weak: var(--salt-color-green-100);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

weak -> weakest

--salt-palette-warning-disabled: var(--salt-color-orange-500-40a);
--salt-palette-warning-strong: var(--salt-color-orange-600);
--salt-palette-warning-strong-disabled: var(--salt-color-orange-600-40a);
--salt-palette-warning-weak: var(--salt-color-orange-100);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

weak -> weakest

--salt-color-gray-500-10a: rgba(114, 119, 125, 0.1);
--salt-color-gray-500-40a: rgba(114, 119, 125, 0.4);
--salt-color-gray-600-10a: rgba(95, 100, 106, 0.1);
--salt-color-gray-600-40a: rgba(97, 102, 108, 0.4);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

??? Why gray 600 10a/40a not same RGB

@@ -4,6 +4,7 @@
"main": "./src/index.ts",
"version": "1.0.0",
"dependencies": {
"slash": "^4.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix typecheck error, different version is resolved without this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant