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 1 commit
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
3 changes: 3 additions & 0 deletions .storybook/components/Docs/Guidelines/Theming.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,11 @@ Example:
npx eds-import-from-figma 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. Instrustions 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
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
/* NOTE: by specifying no default color for borders, they inherit from color below */
&.inline-notification--status-informational {
color: var(--eds-theme-color-text-utility-informational);
background-color: var(--eds-theme-color-background-utility-information-low-emphasis);
background-color: var(--eds-theme-color-background-utility-informational-low-emphasis);
}

&.inline-notification--status-critical {
Expand All @@ -47,4 +47,4 @@

.inline-notification__body {
flex-grow: 2;
}
}
Loading