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 variable setting for various cases #3219

Merged
merged 2 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions .changeset/seven-keys-tease.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@tokens-studio/figma-plugin": patch
---

Fixes an issue when updating a token's value from a reference to a hard value, the check we introduced in the last release caused those to not be updated (and only worked when changing to another reference).
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import setBooleanValuesOnVariable from './setBooleanValuesOnVariable';
import { isVariableWithAliasReference } from '@/utils/isAliasReference';

jest.mock('@/utils/isAliasReference');

describe('setBooleanValuesOnVariable', () => {
let mockVariable: Variable;
const mockMode = 'light';

beforeEach(() => {
mockVariable = {
valuesByMode: {
light: false,
},
setValueForMode: jest.fn(),
} as unknown as Variable;

(isVariableWithAliasReference as unknown as jest.Mock).mockReset();
});

it('should set new boolean value when different from existing value', () => {
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false);
setBooleanValuesOnVariable(mockVariable, mockMode, 'true');
expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, true);
});

it('should not set value when new value equals existing value', () => {
mockVariable.valuesByMode.light = false;
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false);
setBooleanValuesOnVariable(mockVariable, mockMode, 'false');
expect(mockVariable.setValueForMode).not.toHaveBeenCalled();
});

it('should handle alias references', () => {
mockVariable.valuesByMode.light = '{bool.primary}';
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(true);
setBooleanValuesOnVariable(mockVariable, mockMode, 'true');
expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, true);
});

it('should return early if existing value is invalid', () => {
mockVariable.valuesByMode.light = 'invalid' as any;
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false);
setBooleanValuesOnVariable(mockVariable, mockMode, 'true');
expect(mockVariable.setValueForMode).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import { isVariableWithAliasReference } from '@/utils/isAliasReference';

export default function setBooleanValuesOnVariable(variable: Variable, mode: string, value: string) {
try {
const existingVariableValue = variable.valuesByMode[mode];
if (typeof existingVariableValue !== 'boolean') return;
if (existingVariableValue === undefined || !(typeof existingVariableValue === 'boolean' || isVariableWithAliasReference(existingVariableValue))) return;

const newValue = value === 'true';

const existingValueString = existingVariableValue.toString();
if (existingValueString !== value) {
if (value === 'true') {
variable.setValueForMode(mode, value === 'true');
} else {
variable.setValueForMode(mode, false);
}
if (existingVariableValue !== newValue) {
variable.setValueForMode(mode, newValue);
}
} catch (e) {
console.error('Error setting booleanVariable', e);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import setColorValuesOnVariable, { normalizeFigmaColor } from './setColorValuesOnVariable';

describe('normalizeFigmaColor', () => {
it('should round color values to 6 decimal places', () => {
const input = {
r: 0.12345678,
g: 0.23456789,
b: 0.34567890,
a: 0.45678901,
};

const result = normalizeFigmaColor(input);

expect(result).toEqual({
r: 0.123457,
g: 0.234568,
b: 0.345679,
a: 0.456789,
});
});
});

describe('setColorValuesOnVariable', () => {
let mockVariable: Variable;
const mockMode = 'light';

beforeEach(() => {
mockVariable = {
valuesByMode: {},
setValueForMode: jest.fn(),
} as unknown as Variable;
});

it('should set new color value when values are different', () => {
mockVariable.valuesByMode[mockMode] = {
r: 0, g: 0, b: 0, a: 1,
};

setColorValuesOnVariable(mockVariable, mockMode, '#FF0000');

expect(mockVariable.setValueForMode).toHaveBeenCalledWith(
mockMode,
expect.objectContaining({
r: 1, g: 0, b: 0, a: 1,
}),
);
});

it('should not set value when colors are identical', () => {
mockVariable.valuesByMode[mockMode] = {
r: 1, g: 0, b: 0, a: 1,
};

setColorValuesOnVariable(mockVariable, mockMode, '#FF0000');

expect(mockVariable.setValueForMode).not.toHaveBeenCalled();
});

it('should handle invalid existing variable values', () => {
mockVariable.valuesByMode[mockMode] = 'invalid' as any;

setColorValuesOnVariable(mockVariable, mockMode, '#FF0000');

expect(mockVariable.setValueForMode).not.toHaveBeenCalled();
});

it('should handle errors gracefully', () => {
const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});

setColorValuesOnVariable(mockVariable, mockMode, 'invalid-color');

expect(consoleSpy).toHaveBeenCalledWith(
'Error setting colorVariable',
expect.any(Error),
);
consoleSpy.mockRestore();
});
});
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isVariableWithAliasReference } from '@/utils/isAliasReference';
import { convertToFigmaColor } from './figmaTransforms/colors';

export function normalizeFigmaColor({
Expand All @@ -22,16 +23,22 @@ export default function setColorValuesOnVariable(variable: Variable, mode: strin
try {
const { color, opacity } = convertToFigmaColor(value);
const existingVariableValue = variable.valuesByMode[mode];
if (!existingVariableValue || !isFigmaColorObject(existingVariableValue)) return;
const existingValue = normalizeFigmaColor(existingVariableValue as RGBA);
const newValue = normalizeFigmaColor({ ...color, a: opacity });
if (!existingVariableValue || !(isFigmaColorObject(existingVariableValue) || isVariableWithAliasReference(existingVariableValue))) return;

if ((existingValue.r !== newValue.r)
|| (existingValue.g !== newValue.g)
|| (existingValue.b !== newValue.b)
|| (existingValue.a !== newValue.a)) {
variable.setValueForMode(mode, newValue);
const existingValue = isFigmaColorObject(existingVariableValue) ? normalizeFigmaColor(existingVariableValue) : existingVariableValue;
const newValue = isFigmaColorObject(color) ? normalizeFigmaColor({ ...color, a: opacity }) : { ...color, a: opacity };

if (isFigmaColorObject(existingValue) && isFigmaColorObject(newValue)) {
if ((existingValue.r === newValue.r)
&& (existingValue.g === newValue.g)
&& (existingValue.b === newValue.b)
&& (existingValue.a === newValue.a)) {
// return if values match
return;
}
}

variable.setValueForMode(mode, newValue);
} catch (e) {
console.error('Error setting colorVariable', e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import setNumberValuesOnVariable from './setNumberValuesOnVariable';
import { isVariableWithAliasReference } from '@/utils/isAliasReference';

jest.mock('@/utils/isAliasReference');

describe('setNumberValuesOnVariable', () => {
let mockVariable: Variable;
const mockMode = 'light';

beforeEach(() => {
mockVariable = {
name: 'testVariable',
valuesByMode: {
light: 10,
},
setValueForMode: jest.fn(),
} as unknown as Variable;

(isVariableWithAliasReference as unknown as jest.Mock).mockReset();
});

it('should set new number value when different from existing value', () => {
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false);
setNumberValuesOnVariable(mockVariable, mockMode, 20);
expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, 20);
});

it('should not set value when new value equals existing value', () => {
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false);
setNumberValuesOnVariable(mockVariable, mockMode, 10);
expect(mockVariable.setValueForMode).not.toHaveBeenCalled();
});

it('should handle alias references', () => {
mockVariable.valuesByMode.light = '{number.primary}';
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(true);
setNumberValuesOnVariable(mockVariable, mockMode, 30);
expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, 30);
});

it('should handle NaN values', () => {
const consoleSpy = jest.spyOn(console, 'error');
setNumberValuesOnVariable(mockVariable, mockMode, NaN);
expect(mockVariable.setValueForMode).not.toHaveBeenCalled();
expect(consoleSpy).toHaveBeenCalled();
});

it('should handle existing value of 0', () => {
mockVariable.valuesByMode.light = 0;
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false);
setNumberValuesOnVariable(mockVariable, mockMode, 20);
expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, 20);
});

it('should handle new value of 0', () => {
mockVariable.valuesByMode.light = 20;
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false);
setNumberValuesOnVariable(mockVariable, mockMode, 0);
expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, 0);
});

it('should return early if existing value is invalid', () => {
mockVariable.valuesByMode.light = 'invalid' as any;
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false);
setNumberValuesOnVariable(mockVariable, mockMode, 20);
expect(mockVariable.setValueForMode).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import { isVariableWithAliasReference } from '@/utils/isAliasReference';

export default function setNumberValuesOnVariable(variable: Variable, mode: string, value: number) {
try {
if (isNaN(value)) {
throw new Error(`Skipping due to invalid value: ${value}`);
}
const existingValue = variable.valuesByMode[mode];
if (typeof existingValue !== 'number') return;
if (existingValue !== value) {
variable.setValueForMode(mode, value);
const existingVariableValue = variable.valuesByMode[mode];
if (existingVariableValue === undefined || !(typeof existingVariableValue === 'number' || isVariableWithAliasReference(existingVariableValue))) return;

const newValue = value;

if (existingVariableValue !== newValue) {
variable.setValueForMode(mode, newValue);
}
} catch (e) {
console.error('Error setting numberVariable on variable', variable.name, e);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import setStringValuesOnVariable from './setStringValuesOnVariable';
import { isVariableWithAliasReference } from '@/utils/isAliasReference';

jest.mock('@/utils/isAliasReference');

describe('setStringValuesOnVariable', () => {
let mockVariable: Variable;
const mockMode = 'light';

beforeEach(() => {
mockVariable = {
valuesByMode: {
light: 'oldValue',
},
setValueForMode: jest.fn(),
} as unknown as Variable;

(isVariableWithAliasReference as unknown as jest.Mock).mockReset();
});

it('should set new string value when different from existing value', () => {
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false);
setStringValuesOnVariable(mockVariable, mockMode, 'newValue');
expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, 'newValue');
});

it('should not set value when new value equals existing value', () => {
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false);
setStringValuesOnVariable(mockVariable, mockMode, 'oldValue');
expect(mockVariable.setValueForMode).not.toHaveBeenCalled();
});

it('should handle alias references', () => {
mockVariable.valuesByMode.light = '{color.primary}';
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(true);
setStringValuesOnVariable(mockVariable, mockMode, '{color.secondary}');
expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, '{color.secondary}');
});

it('should return early if existing value is invalid', () => {
mockVariable.valuesByMode.light = 123 as any;
(isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false);
setStringValuesOnVariable(mockVariable, mockMode, 'newValue');
expect(mockVariable.setValueForMode).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { isVariableWithAliasReference } from '@/utils/isAliasReference';

export default function setStringValuesOnVariable(variable: Variable, mode: string, value: string) {
try {
const existingValue = variable.valuesByMode[mode];
if (typeof existingValue !== 'string') return;
const existingVariableValue = variable.valuesByMode[mode];
if (!existingVariableValue || !(typeof existingVariableValue === 'string' || isVariableWithAliasReference(existingVariableValue))) return;

if (existingValue !== value) {
if (existingVariableValue !== value) {
variable.setValueForMode(mode, value);
}
} catch (e) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function isVariableWithAliasReference(obj: VariableValue): obj is VariableAlias {
return typeof obj === 'object' && 'id' in obj && typeof obj.id === 'string';
}
Loading