From ac0fd7264af5b2ab56a50cc9a9891113dcd1cece Mon Sep 17 00:00:00 2001 From: Jan Six Date: Thu, 5 Sep 2024 09:32:58 +0200 Subject: [PATCH 1/4] fix color creation --- .changeset/hot-bears-compete.md | 5 +++++ .../src/plugin/setColorValuesOnTarget.ts | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 .changeset/hot-bears-compete.md diff --git a/.changeset/hot-bears-compete.md b/.changeset/hot-bears-compete.md new file mode 100644 index 000000000..9f2df4a58 --- /dev/null +++ b/.changeset/hot-bears-compete.md @@ -0,0 +1,5 @@ +--- +"@tokens-studio/figma-plugin": patch +--- + +Fixes variable creation of color token was using a modifier and using a reference. We now correctly create a raw hex value as Figma doesn't have modifiers. Before we falsely used a reference without the modifier applied diff --git a/packages/tokens-studio-for-figma/src/plugin/setColorValuesOnTarget.ts b/packages/tokens-studio-for-figma/src/plugin/setColorValuesOnTarget.ts index edd1f593e..c2269ce81 100644 --- a/packages/tokens-studio-for-figma/src/plugin/setColorValuesOnTarget.ts +++ b/packages/tokens-studio-for-figma/src/plugin/setColorValuesOnTarget.ts @@ -5,6 +5,11 @@ import { defaultTokenValueRetriever } from './TokenValueRetriever'; import { ColorPaintType, tryApplyColorVariableId } from '@/utils/tryApplyColorVariableId'; import { unbindVariableFromTarget } from './unbindVariableFromTarget'; import { getReferenceTokensFromGradient } from '@/utils/color'; +import { SingleToken } from '@/types/tokens'; + +function hasModifier(token: SingleToken) { + return token.$extensions?.['studio.tokens']?.modify; +} export default async function setColorValuesOnTarget({ target, token, key, givenValue, @@ -79,7 +84,7 @@ export default async function setColorValuesOnTarget({ const containsReferenceVariable = resolvedValue.toString().startsWith('{') && resolvedValue.toString().endsWith('}'); const referenceVariableExists = await defaultTokenValueRetriever.getVariableReference(resolvedValue.slice(1, -1)); - if (containsReferenceVariable && referenceVariableExists && shouldCreateStylesWithVariables) { + if (containsReferenceVariable && referenceVariableExists && shouldCreateStylesWithVariables && !hasModifier(resolvedToken)) { try { successfullyAppliedVariable = await tryApplyColorVariableId(target, resolvedValue.slice(1, -1), ColorPaintType.PAINTS); } catch (e) { From 61027be2c14d5fb2f3cb1e4c8cf7ca0bfe59dc26 Mon Sep 17 00:00:00 2001 From: Jan Six Date: Thu, 5 Sep 2024 09:34:04 +0200 Subject: [PATCH 2/4] Fixed variable creation around font weights --- .changeset/chatty-buckets-share.md | 5 +++ .../tryApplyTypographyCompositeVariable.ts | 44 ++++++++++++++----- 2 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 .changeset/chatty-buckets-share.md diff --git a/.changeset/chatty-buckets-share.md b/.changeset/chatty-buckets-share.md new file mode 100644 index 000000000..f92e09cfb --- /dev/null +++ b/.changeset/chatty-buckets-share.md @@ -0,0 +1,5 @@ +--- +"@tokens-studio/figma-plugin": patch +--- + +Fixed an issue around variable creation where if numerical weights were used we'd display an error that we're unable to apply the font. We now changed this to properly load all weights of the font family and then create styles correctly with variable references to the numerical weight variable diff --git a/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.ts b/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.ts index 29d92eed9..48f92e1fb 100644 --- a/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.ts +++ b/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.ts @@ -24,29 +24,49 @@ export async function tryApplyTypographyCompositeVariable({ if (typeof value === 'string') return; try { - // First we set font family and weight without variables, we do this because to apply those values we need their combination - if ('fontName' in target && ('fontWeight' in value || 'fontFamily' in value)) { - setFontStyleOnTarget({ target, value: { fontFamily: value.fontFamily, fontWeight: value.fontWeight }, baseFontSize }); - } - // Then we iterate over all keys of the typography object and apply variables if available, otherwise we apply the value directly + // We iterate over all keys of the typography object and apply variables if available, otherwise we apply the value directly for (const [originalKey, val] of Object.entries(value).filter(([_, keyValue]) => typeof keyValue !== 'undefined')) { if (typeof val === 'undefined') return; let successfullyAppliedVariable = false; if (resolvedValue[originalKey].toString().startsWith('{') && resolvedValue[originalKey].toString().endsWith('}') && shouldCreateStylesWithVariables) { const variableToApply = await defaultTokenValueRetriever.getVariableReference(resolvedValue[originalKey].toString().slice(1, -1)); const key = transformTypographyKeyToFigmaVariable(originalKey, variableToApply); + // If we're dealing with a variable, we fetch all available font weights for the current font and load them. + // This is needed because we have numerical weights, but we need to apply the string ones. We dont know them from Figma, so we need to load all. + // e.g. font weight = 600, we dont know that we need to load "Bold". + if (key === 'fontFamily') { + const firstVariableValue = Object.values(variableToApply.valuesByMode)[0]; + if (firstVariableValue) { + const fontsMatching = (await figma.listAvailableFontsAsync() || []).filter((font) => font.fontName.family === firstVariableValue); + for (const font of fontsMatching) { + await figma.loadFontAsync(font.fontName); + } + } + } if (variableToApply) { if (target.fontName !== figma.mixed) await figma.loadFontAsync(target.fontName); - target.setBoundVariable(key, variableToApply); - successfullyAppliedVariable = true; + try { + target.setBoundVariable(key, variableToApply); + successfullyAppliedVariable = true; + } catch (e) { + // eslint-disable-next-line no-console + console.error('unable to apply variable', key, variableToApply, e); + } } } // If there's no variable we apply the value directly - if (!successfullyAppliedVariable && originalKey !== 'fontFamily' && originalKey !== 'fontWeight') { - if (target.fontName !== figma.mixed) await figma.loadFontAsync(target.fontName); - const transformedValue = transformValue(value[originalKey], originalKey, baseFontSize); - if (transformedValue !== null) { - target[originalKey] = transformedValue; + if (!successfullyAppliedVariable) { + // First we set font family and weight without variables, we do this because to apply those values we need their combination + if (originalKey === 'fontFamily' || originalKey === 'fontWeight') { + if ('fontName' in target && ('fontWeight' in value || 'fontFamily' in value)) { + setFontStyleOnTarget({ target, value: { fontFamily: value.fontFamily, fontWeight: value.fontWeight }, baseFontSize }); + } + } else { + if (target.fontName !== figma.mixed) await figma.loadFontAsync(target.fontName); + const transformedValue = transformValue(value[originalKey], originalKey, baseFontSize); + if (transformedValue !== null) { + target[originalKey] = transformedValue; + } } } } From ff6725a7a70e78ff87ed532f92dbe20df09b5db2 Mon Sep 17 00:00:00 2001 From: Jan Six Date: Fri, 6 Sep 2024 08:42:40 +0200 Subject: [PATCH 3/4] fix tests --- .../tryApplyTypographyCompositeVariable.test.ts | 14 +++++++++----- .../plugin/tryApplyTypographyCompositeVariable.ts | 4 ++-- .../tests/__mocks__/figmaMock.js | 4 +++- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.test.ts b/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.test.ts index 50d57d330..545238b7c 100644 --- a/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.test.ts +++ b/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.test.ts @@ -53,18 +53,20 @@ describe('tryApplyTypographyCompositeVariable', () => { fontFamily: '{fontFamily.default}', fontWeight: '{fontWeight.default}', }; - defaultTokenValueRetriever.getVariableReference = jest.fn().mockResolvedValue('Roboto'); + const familyVariable = { valuesByMode: { default: ['Roboto'] } }; + const weightVariable = { valuesByMode: { default: ['Bold'] } }; + defaultTokenValueRetriever.getVariableReference = jest.fn().mockResolvedValue(familyVariable); defaultTokenValueRetriever.getVariableReference = jest.fn() - .mockResolvedValueOnce('Roboto') - .mockResolvedValueOnce('Bold'); + .mockResolvedValueOnce(familyVariable) + .mockResolvedValueOnce(weightVariable); await tryApplyTypographyCompositeVariable({ target, value, resolvedValue, baseFontSize, }); expect(target.setBoundVariable).toHaveBeenCalledTimes(2); - expect(target.setBoundVariable).toHaveBeenNthCalledWith(1, 'fontFamily', 'Roboto'); - expect(target.setBoundVariable).toHaveBeenNthCalledWith(2, 'fontStyle', 'Bold'); + expect(target.setBoundVariable).toHaveBeenNthCalledWith(1, 'fontFamily', familyVariable); + expect(target.setBoundVariable).toHaveBeenNthCalledWith(2, 'fontStyle', weightVariable); }); it('should apply values directly if no variables are available', async () => { @@ -87,6 +89,8 @@ describe('tryApplyTypographyCompositeVariable', () => { lineHeight: '1.5', }; + defaultTokenValueRetriever.getVariableReference = jest.fn().mockResolvedValue(undefined); + await tryApplyTypographyCompositeVariable({ target, value, resolvedValue, baseFontSize, }); diff --git a/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.ts b/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.ts index 48f92e1fb..73daf5e72 100644 --- a/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.ts +++ b/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.ts @@ -28,13 +28,13 @@ export async function tryApplyTypographyCompositeVariable({ for (const [originalKey, val] of Object.entries(value).filter(([_, keyValue]) => typeof keyValue !== 'undefined')) { if (typeof val === 'undefined') return; let successfullyAppliedVariable = false; - if (resolvedValue[originalKey].toString().startsWith('{') && resolvedValue[originalKey].toString().endsWith('}') && shouldCreateStylesWithVariables) { + if (resolvedValue[originalKey]?.toString().startsWith('{') && resolvedValue[originalKey].toString().endsWith('}') && shouldCreateStylesWithVariables) { const variableToApply = await defaultTokenValueRetriever.getVariableReference(resolvedValue[originalKey].toString().slice(1, -1)); const key = transformTypographyKeyToFigmaVariable(originalKey, variableToApply); // If we're dealing with a variable, we fetch all available font weights for the current font and load them. // This is needed because we have numerical weights, but we need to apply the string ones. We dont know them from Figma, so we need to load all. // e.g. font weight = 600, we dont know that we need to load "Bold". - if (key === 'fontFamily') { + if (key === 'fontFamily' && variableToApply) { const firstVariableValue = Object.values(variableToApply.valuesByMode)[0]; if (firstVariableValue) { const fontsMatching = (await figma.listAvailableFontsAsync() || []).filter((font) => font.fontName.family === firstVariableValue); diff --git a/packages/tokens-studio-for-figma/tests/__mocks__/figmaMock.js b/packages/tokens-studio-for-figma/tests/__mocks__/figmaMock.js index 07ef4a93f..5d7e0311c 100644 --- a/packages/tokens-studio-for-figma/tests/__mocks__/figmaMock.js +++ b/packages/tokens-studio-for-figma/tests/__mocks__/figmaMock.js @@ -82,6 +82,7 @@ module.exports.mockImportVariableByKeyAsync = jest.fn(); module.exports.mockGetVariableById = jest.fn(); module.exports.mockSetValueForMode = jest.fn(); module.exports.mockSetBoundVariableForPaint = jest.fn(); +module.exports.mockListAvailableFontsAsync = jest.fn(() => Promise.resolve([{fontName: {family: 'Roboto', style: 'Bold'}}, {fontName: {family: 'Inter', style: 'Regular'}}])); module.exports.figma = { showUI: module.exports.mockShowUI, @@ -136,7 +137,8 @@ module.exports.figma = { createEffectStyle: module.exports.mockCreateEffectStyle, importStyleByKeyAsync: module.exports.mockImportStyleByKeyAsync, getNodeById: module.exports.mockGetNodeById, - createImage: module.exports.mockCreateImage + createImage: module.exports.mockCreateImage, + listAvailableFontsAsync: module.exports.mockListAvailableFontsAsync, }; parent.postMessage = module.exports.mockParentPostMessage; From 52b584b5391b73cb8336a5677a8e1867b8cf366c Mon Sep 17 00:00:00 2001 From: Jan Six Date: Fri, 6 Sep 2024 08:48:28 +0200 Subject: [PATCH 4/4] adjust tests --- ...ryApplyTypographyCompositeVariable.test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.test.ts b/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.test.ts index 545238b7c..bef99df06 100644 --- a/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.test.ts +++ b/packages/tokens-studio-for-figma/src/plugin/tryApplyTypographyCompositeVariable.test.ts @@ -177,4 +177,26 @@ describe('tryApplyTypographyCompositeVariable', () => { target, value, resolvedValue, baseFontSize, }); }); + + it('should apply transformed value when no variable is available', async () => { + target = { + fontName: { + family: 'Arial', + style: 'Regular', + }, + } as TextNode | TextStyle; + value = { + letterSpacing: '0.5em', + }; + resolvedValue = { + letterSpacing: '0.5em', + }; + defaultTokenValueRetriever.getVariableReference = jest.fn().mockResolvedValue(undefined); + + await tryApplyTypographyCompositeVariable({ + target, value, resolvedValue, baseFontSize, + }); + + expect(target.letterSpacing).toEqual({ unit: 'PIXELS', value: 8 }); + }); });