From cf4411db3ed8e477f04da6fcc5b70e1b380183f3 Mon Sep 17 00:00:00 2001 From: Marco Christian Krenn Date: Fri, 23 May 2025 22:07:15 +0200 Subject: [PATCH 1/3] fix: update ColorField to handle delayed updates properly --- .../graph-editor/src/components/controls/color.tsx | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/graph-editor/src/components/controls/color.tsx b/packages/graph-editor/src/components/controls/color.tsx index 20f4815d..780856c4 100644 --- a/packages/graph-editor/src/components/controls/color.tsx +++ b/packages/graph-editor/src/components/controls/color.tsx @@ -58,14 +58,24 @@ export const ColorField = observer(({ port, readOnly }: IField) => { ); } + // When delayed updates are enabled, show the actual port value for the color ball + // but allow the text input to show the temporary value + const colorBallValue = useDelayed ? (() => { + try { + return toHex(toColor(port.value)); + } catch { + return val; + } + })() : val; + return ( - + {val} {useDelayed && ( } - onClick={() => (port as Input).setValue(val)} + onClick={() => (port as Input).setValue(hexToColor(val))} /> )} From b725564c36779446ae1dc562dded92cd917b4233 Mon Sep 17 00:00:00 2001 From: Marco Christian Krenn Date: Fri, 23 May 2025 22:08:04 +0200 Subject: [PATCH 2/3] fix: add textValue prop to ColorPickerPopover for delayed updates --- packages/graph-editor/src/components/colorPicker/index.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/graph-editor/src/components/colorPicker/index.tsx b/packages/graph-editor/src/components/colorPicker/index.tsx index 209af067..ae754622 100644 --- a/packages/graph-editor/src/components/colorPicker/index.tsx +++ b/packages/graph-editor/src/components/colorPicker/index.tsx @@ -17,6 +17,7 @@ export function ColorPicker({ value, onChange }) { type ColorPickerPopoverProps = { value: string; + textValue?: string; // Optional separate value for text input defaultOpen?: boolean; onChange: (value: string) => void; showRemoveButton?: boolean; @@ -25,11 +26,14 @@ type ColorPickerPopoverProps = { export function ColorPickerPopover({ value, + textValue, defaultOpen = false, onChange, showRemoveButton = false, onRemove, }: ColorPickerPopoverProps) { + // Use textValue for the input if provided, otherwise use value + const inputValue = textValue ?? value; return ( onChange(event.target.value)} /> {showRemoveButton && ( From aa4b41afa04e66628875d129891319035f925250 Mon Sep 17 00:00:00 2001 From: Marco Christian Krenn Date: Fri, 23 May 2025 22:09:22 +0200 Subject: [PATCH 3/3] test: add unit tests for ColorField delayed updates fix --- .../src/components/controls/color.test.tsx | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 packages/graph-editor/src/components/controls/color.test.tsx diff --git a/packages/graph-editor/src/components/controls/color.test.tsx b/packages/graph-editor/src/components/controls/color.test.tsx new file mode 100644 index 00000000..75957f05 --- /dev/null +++ b/packages/graph-editor/src/components/controls/color.test.tsx @@ -0,0 +1,151 @@ +import React from 'react'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { Provider } from 'react-redux'; +import { configureStore } from '@reduxjs/toolkit'; +import { ColorField } from './color'; +import { Input } from '@tokens-studio/graph-engine'; + +// Mock the graph engine utilities +jest.mock('@tokens-studio/graph-engine', () => ({ + Input: jest.fn(), + toColor: jest.fn((color) => ({ to: () => ({ toString: () => color.hex || '#ff0000' }) })), + toHex: jest.fn((color) => color.hex || '#ff0000'), + hexToColor: jest.fn((hex) => ({ hex, space: 'srgb', channels: [1, 0, 0] })), +})); + +// Mock the ColorPickerPopover component +jest.mock('../colorPicker/index.js', () => ({ + ColorPickerPopover: ({ value, textValue, onChange }) => ( +
+
+ ), +})); + +// Mock Redux selectors +jest.mock('@/redux/selectors/index.js', () => ({ + delayedUpdateSelector: (state) => state.ui.useDelayed, +})); + +// Mock icons +jest.mock('@tokens-studio/icons/FloppyDisk.js', () => () =>
Save
); + +describe('ColorField', () => { + const createMockStore = (useDelayed = false) => { + return configureStore({ + reducer: { + ui: (state = { useDelayed }, action) => state, + }, + }); + }; + + const createMockPort = (initialValue = { hex: '#ff0000' }) => { + const port = { + value: initialValue, + setValue: jest.fn(), + }; + return port; + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should show the same color in ball and input when delayed updates are disabled', () => { + const store = createMockStore(false); + const port = createMockPort(); + + render( + + + + ); + + const colorBall = screen.getByTestId('color-ball'); + const colorInput = screen.getByTestId('color-input'); + + expect(colorBall.style.background).toBe('#ff0000'); + expect(colorInput.value).toBe('#ff0000'); + }); + + it('should show port value in color ball but allow different text input when delayed updates are enabled', () => { + const store = createMockStore(true); + const port = createMockPort({ hex: '#ff0000' }); + + render( + + + + ); + + const colorBall = screen.getByTestId('color-ball'); + const colorInput = screen.getByTestId('color-input'); + + // Initially both should show the port value + expect(colorBall.style.background).toBe('#ff0000'); + expect(colorInput.value).toBe('#ff0000'); + + // When user types a new hex value + fireEvent.change(colorInput, { target: { value: '#00ff00' } }); + + // Color ball should still show the original port value (delayed update behavior) + expect(colorBall.style.background).toBe('#ff0000'); + // But input should show the new value + expect(colorInput.value).toBe('#00ff00'); + + // Port setValue should not have been called yet (delayed) + expect(port.setValue).not.toHaveBeenCalled(); + }); + + it('should show save button when delayed updates are enabled', () => { + const store = createMockStore(true); + const port = createMockPort(); + + render( + + + + ); + + expect(screen.getByText('Save')).toBeInTheDocument(); + }); + + it('should not show save button when delayed updates are disabled', () => { + const store = createMockStore(false); + const port = createMockPort(); + + render( + + + + ); + + expect(screen.queryByText('Save')).not.toBeInTheDocument(); + }); + + it('should update port value immediately when delayed updates are disabled', () => { + const store = createMockStore(false); + const port = createMockPort(); + + render( + + + + ); + + const colorInput = screen.getByTestId('color-input'); + fireEvent.change(colorInput, { target: { value: '#00ff00' } }); + + // Port setValue should be called immediately + expect(port.setValue).toHaveBeenCalledWith({ hex: '#00ff00', space: 'srgb', channels: [1, 0, 0] }); + }); +});