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

fix: apply token and script fixes to sync with design #2142

Merged
merged 7 commits into from
Mar 13, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .storybook/components/Docs/Guidelines/Theming.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ EDS comes with some optional tooling to allow easy transfer of theme data from F

- `eds-init-theme` - This command creates the initial file(s) for theming your application
- `eds-apply-theme` - This command parses the local config file to generate the tokens used by EDS components and tools
- `eds-import-from-figma-api` - This command reads from figma, and applies token values to the theme in the app

Each of these tools reads config to figure out where to read/write files.

Expand Down Expand Up @@ -171,11 +172,14 @@ When using a [Figma Enterprise account][figma-enterprise], core design system fi
Example:

```
npx eds-import-from-figma file <file_id> --token <personal_access_token>
npx eds-import-from-figma-api file <file_id> --token <personal_access_token>
```

**NOTE**: when using this method, it requires both the figma file ID (e.g., `figma.com/design/<file_id>/`), and an API token. Instructions from Figma [are here][figma-access-token-docs].

[figma-enterprise]: https://www.figma.com/enterprise/
[figma-api-docs]: https://www.figma.com/developers/api
[figma-access-token-docs]: https://help.figma.com/hc/en-us/articles/8085703771159-Manage-personal-access-tokens

## Custom Theming and Tailwind

Expand Down
31 changes: 31 additions & 0 deletions .storybook/data/tokens.json
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@
"eds-theme-border-radius-objects-sm": "2",
"eds-theme-border-radius-objects-xs": "0",
"eds-theme-border-radius-tab-underline": "50",
"eds-theme-border-radius-tab-underline-default": "9999",
"eds-theme-border-radius-tab-underline-sticky": "9999",
"eds-theme-border-width": "1",
"eds-theme-box-button-border-radius": "4",
"eds-theme-box-focus-ring-outline-width": "1",
Expand Down Expand Up @@ -242,9 +244,15 @@
"eds-theme-color-background-utility-warning-low-emphasis": "#FDF3D3",
"eds-theme-color-background-utility-warning-low-emphasis-hover": "#FBE8AB",
"eds-theme-color-background-utility-warning-low-emphasis-active": "#F9DA78",
"eds-theme-color-background-utility-warning-high-emphasis": "#FDF3D3",
"eds-theme-color-background-utility-warning-high-emphasis-hover": "#FBE8AB",
"eds-theme-color-background-utility-warning-high-emphasis-active": "#F9DA78",
"eds-theme-color-background-utility-information-low-emphasis": "#EAF4FF",
"eds-theme-color-background-utility-information-low-emphasis-hover": "#CEE6FF",
"eds-theme-color-background-utility-information-low-emphasis-active": "#B5DAFF",
"eds-theme-color-background-utility-informational-low-emphasis": "#EAF4FF",
"eds-theme-color-background-utility-informational-low-emphasis-hover": "#CEE6FF",
"eds-theme-color-background-utility-informational-low-emphasis-active": "#B5DAFF",
"eds-theme-color-background-utility-disabled-no-emphasis": "transparent",
"eds-theme-color-background-utility-disabled-low-emphasis": "#EEE7E4",
"eds-theme-color-background-utility-disabled-medium-emphasis": "#CFC9C7",
Expand Down Expand Up @@ -519,6 +527,7 @@
"eds-theme-color-icon-utility-disabled-secondary": "#CFC9C7",
"eds-theme-color-icon-utility-inverse": "rgb(var(--eds-color-white) / 1)",
"eds-theme-color-icon-utility-inverse-disabled": "rgb(var(--eds-color-white) / 0.5)",
"eds-theme-color-icon-utility-inverse-interactive-visited": "rgb(var(--eds-color-white) / 1)",
"eds-theme-color-icon-utility-placeholder": "#6C6967",
"eds-theme-color-icon-utility-success": "#00A56A",
"eds-theme-color-icon-utility-success-hover": "#008656",
Expand Down Expand Up @@ -820,6 +829,28 @@
"eds-size-quarter": "2",
"eds-size-1-and-half": "12",
"eds-size-2-and-half": "20",
"eds-spacing-size-1": "8",
"eds-spacing-size-2": "16",
"eds-spacing-size-3": "24",
"eds-spacing-size-4": "32",
"eds-spacing-size-5": "40",
"eds-spacing-size-6": "48",
"eds-spacing-size-7": "56",
"eds-spacing-size-8": "64",
"eds-spacing-size-9": "72",
"eds-spacing-size-10": "80",
"eds-spacing-size-11": "88",
"eds-spacing-size-12": "96",
"eds-spacing-size-20": "160",
"eds-spacing-size-24": "192",
"eds-spacing-size-32": "256",
"eds-spacing-size-34": "272",
"eds-spacing-size-40": "320",
"eds-spacing-size-base-unit": "8",
"eds-spacing-size-half": "4",
"eds-spacing-size-quarter": "2",
"eds-spacing-size-1-and-half": "12",
"eds-spacing-size-2-and-half": "20",
"eds-z-index-0": "0",
"eds-z-index-100": "100",
"eds-z-index-200": "200",
Expand Down
83 changes: 50 additions & 33 deletions bin/_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,14 +344,18 @@ module.exports = {
* @returns Variable
*/
retrieveVariable(variableId) {
return Object.values(this._jsonData.variables).find((variable) => {
return variable.id === variableId;
});
const retrieved = Object.values(this._jsonData.variables).find(
(variable) => {
return variable.id === variableId;
},
);
return retrieved;
}
},

FigmaVariable: class {
static TIER_1_PREFIX = 'Render/';
static TIER_1_PREFIX = 'render/';
static VARIABLE_ALIAS = 'VARIABLE_ALIAS';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: a few of these constants will migrate to configuration once we have enough use cases to do so


constructor(json, mode, lookupDelegate) {
// TODO: throw if any private members are invalid
Expand Down Expand Up @@ -415,8 +419,27 @@ module.exports = {
}
}

/**
* Determine whether a variable is marked as orphaned (e.g., deleted from the var.s but still used in a component)
* @returns {boolean}
*/
isOrphaned() {
return !!this._figmaVariableData.deletedButReferenced;
}

/**
* @static
*
* Returns whether the value of a given figma variable and mode is a reference or not
* @param {FigmaVariable} figmaVariable API JSON data representing a figma variable instance
* @param {string} mode the collection that the variable has a value in
* @returns {boolean} whether the value for the variable is a literal or references another figma variable
*/
static isAliased(figmaVariable, mode) {
return figmaVariable.valuesByMode[mode]?.type === 'VARIABLE_ALIAS';
return (
figmaVariable.valuesByMode[mode]?.type ===
module.exports.FigmaVariable.VARIABLE_ALIAS
);
}

/**
Expand Down Expand Up @@ -499,55 +522,49 @@ module.exports = {
}
}

/**
* @property
* Unformatted (figma) name for the current figma variable
*/
get name() {
return this._figmaVariableData.name;
}

/**
* @property
* Resolved and parsed value for the given variable. Performs lookup if value is aliased, using delegate.
*/
get value() {
return this.parseResolvedValue(this.getResovledValue());
}

/**
* @property
* Resolved and parsed value reference (path) for the given variable. Performs lookup if value is aliased, using delegate.
*/
get valueRef() {
switch (this._figmaVariableData.resolvedType) {
case 'COLOR':
// TODO: (in source) transparent exists as a tier 1 token but should not
if (this.getResovledName() === 'Render/Transparent') {
return 'transparent';
}
// TODO-AH: BUG: we assume all lookups resolve to a tier 1 value
// the one that errors is b/c it doesn't eventually reference a tier 1 (icon/util/inverse-disabled)
// how to properly use .getTokenPath here?
return module.exports.FigmaVariable.isAliased(
this._figmaVariableData,
this._mode,
)
? `{${this._tokenNameToPath(
this.getResovledName()
.replace(
module.exports.FigmaVariable.TIER_1_PREFIX,
'eds.color.',
)
// TODO: (in source) remove duplicate color from structures
.replace('Neutral/Neutral', 'Neutral')
.replace('Red/Red', 'Red')
.replace('Orange/Orange', 'Orange')
.replace('Yellow/Yellow', 'Yellow')
.replace('Green/Green', 'Green')
.replace('Blue/Blue', 'Blue')
.replace('Purple/Purple', 'Purple')
.replace('Pink/Pink', 'Pink')
// TODO: (in source) lower case the names of the tier 1 color tokens
.toLowerCase(),
)}}`
? `${this._tokenNameToPath(
this.getResovledName().replace(
module.exports.FigmaVariable.TIER_1_PREFIX,
'eds.color.',
),
)}`
: this.value;
case 'FLOAT':
return module.exports.FigmaVariable.isAliased(
this._figmaVariableData,
this._mode,
)
? `{${this._tokenNameToPath(
'eds.' +
this.getResovledName()
// TODO: (in source) lower case the names of the tier 1 color tokens
.toLowerCase(),
)}}`
? `${this._tokenNameToPath('eds.' + this.getResovledName())}`
: this.value;
default:
throw new TypeError(
Expand Down
9 changes: 6 additions & 3 deletions bin/_util.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ describe('utils', function () {
variables: {
'VariableID:6348:6248': {
id: 'VariableID:6348:6248',
name: 'Render/Neutral/Neutral-050',
name: 'render/neutral-050',
remote: false,
key: '176ccd26264f7e048ae441ac585726a51fa6a819',
variableCollectionId: 'VariableCollectionId:6181:1797',
Expand Down Expand Up @@ -405,7 +405,7 @@ describe('utils', function () {
},
'VariableID:6195:914': {
id: 'VariableID:6195:914',
name: 'Render/Neutral/White',
name: 'render/neutral/white',
remote: false,
key: 'b2ed72ac00bfa71fbe0c0039404b125f7f5193e2',
variableCollectionId: 'VariableCollectionId:6181:1797',
Expand Down Expand Up @@ -546,6 +546,9 @@ describe('utils', function () {
});

describe('FigmaVariable', function () {
// TODO-AH: add test for lookup when the matching token is not tier 1
// TODO-AH: add test for orphaned token

it('allows retrieval of token path from a variable', () => {
const variable = new utils.FigmaVariable(
{
Expand Down Expand Up @@ -638,7 +641,7 @@ describe('utils', function () {
new utils.FigmaAPIReader(mockData),
);

expect(variable.valueRef).toEqual('{eds.color.neutral.white}');
expect(variable.valueRef).toEqual('eds.color.neutral.white');
});

it('allows retrieval of float value ref from a variable', () => {
Expand Down
43 changes: 34 additions & 9 deletions bin/eds-import-from-figma-api.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#!/usr/bin/env node

const { identity } = require('lodash');

// Script shell
(async function () {
const jsonfile = require('jsonfile');
Expand Down Expand Up @@ -118,22 +120,34 @@
response.modeId,
figmaApiReader,
);
let writePath = variable.getTokenPath();

// If we have a zombie entry in the variable list, skip it (flag for design)
if (variable.isOrphaned()) {
stats.skipped.push(variable);
spinner.warn(
chalk.bold(variable.name) +
': Skipped with warning (orphaned): please remove usage in figma',
);

return;
}

// mesh the token path to a matching path in the local theme file
const found = at(localTheme, writePath).filter(
let writePath = variable.getTokenPath();
const locationInLocal = at(localTheme, writePath).filter(
(entries) => typeof entries !== 'undefined',
);

if (found.length) {
// Update the write path to conform to the format used in style-dictionary
if (locationInLocal.length) {
// handle case where we should look for @ in the file, then pluck the value object properly
if (found[0]['@']?.value) {
if (locationInLocal[0]['@']?.value) {
// update the write path to mark the @ and value
writePath = writePath + '[email protected]';
}

// handle case where it's just value
if (found[0]?.value) {
if (locationInLocal[0]?.value) {
// update the write path to mark the value
writePath = writePath + '.value';
}
Expand All @@ -143,17 +157,28 @@
try {
// error when path suffix is invalid (all should end with .value)
if (!writePath.endsWith('value')) {
isVerbose && console.error(variable);
throw new Error(
`Name format violation. JSON path missing in local theme: ${writePath} (${figmaVariable.resolvedType})`,
);
}

if (
FigmaVariable.isAliased(figmaVariable, response.modeId) &&
!at(localTheme, variable.valueRef).some(identity)
) {
throw new Error(
`Name format violation. Name missing in local theme: ${writePath} (${figmaVariable.resolvedType})`,
`Value violation. Trying to write a reference value path not in local theme: ${writePath} => {${variable.valueRef}}`,
);
}

// Write the defined value to the specified location by json path
canWrite &&
args.local &&
set(localTheme, writePath, variable.valueRef);
set(localTheme, writePath, `{${variable.valueRef}}`);
canWrite && !args.local && set(localTheme, writePath, variable.value);

// write the value using the calculated path and parsed value
// log if in a loggable mode (verbose / dry-run)
if (isVerbose || !canWrite) {
spinner.succeed(
`Write: ${
Expand All @@ -166,7 +191,7 @@
stats.updated.push(variable);
} catch (e) {
// We couldn't parse the resolved value, so skip and add to errors
spinner.fail(chalk.bold(variable.name) + ': Error (' + e + ')');
spinner.fail(chalk.bold(variable.name) + ': ' + e);
stats.errored.push(variable);
}
} else {
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"bin": {
"eds-apply-theme": "bin/eds-apply-theme.js",
"eds-import-from-figma": "bin/eds-import-from-figma.js",
"eds-import-from-figma-api": "bin/eds-import-from-figma-api.js",
"eds-init-theme": "bin/eds-init.js",
"eds-migrate": "bin/eds-migrate.js"
},
Expand Down
2 changes: 1 addition & 1 deletion src/components/Button/Button.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

.button {
position: relative;
border-radius: calc(var(--eds-border-radius-full) * 1px);
border-radius: calc(var(--eds-theme-border-radius-actions) * 1px);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes a bug identified during tool demo ++

border: calc(var(--eds-border-width-sm) * 1px) solid;
overflow: hidden;
display: flex;
Expand Down
8 changes: 4 additions & 4 deletions src/components/Card/Card.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,16 @@
}

.card--container-color-call-out {
background-color: var(--eds-theme-color-background-utility-information-low-emphasis);
background-color: var(--eds-theme-color-background-utility-informational-low-emphasis);
border-color: var(--eds-theme-color-border-utility-informational);

&.card--is-interactive {
&:hover {
background-color: var(--eds-theme-color-background-utility-information-low-emphasis-hover);
background-color: var(--eds-theme-color-background-utility-informational-low-emphasis-hover);
}

&:active {
background-color: var(--eds-theme-color-background-utility-information-low-emphasis-active);
background-color: var(--eds-theme-color-background-utility-informational-low-emphasis-active);
}
}
}
Expand All @@ -87,7 +87,7 @@
& .header__eyebrow {
margin-bottom: calc(var(--eds-size-1) / 16 * 1rem);
}

&.header--size-sm {
gap: calc(var(--eds-size-1) / 16 * 1rem)
}
Expand Down
Loading