Skip to content

fix: color picker indicator not updating on hex change with delayed updates #706

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
6 changes: 5 additions & 1 deletion packages/graph-editor/src/components/colorPicker/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 (
<InputPopover
defaultOpen={defaultOpen}
Expand All @@ -44,7 +48,7 @@ export function ColorPickerPopover({
>
<ColorPicker value={value} onChange={onChange} />
<TextInput
value={value}
value={inputValue}
onChange={(event) => onChange(event.target.value)}
/>
{showRemoveButton && (
Expand Down
151 changes: 151 additions & 0 deletions packages/graph-editor/src/components/controls/color.test.tsx
Original file line number Diff line number Diff line change
@@ -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 }) => (
<div data-testid="color-picker-popover">
<button
data-testid="color-ball"
style={{ background: value }}
aria-label={`Color ball with value ${value}`}
/>
<input
data-testid="color-input"
value={textValue || value}
onChange={(e) => onChange(e.target.value)}
/>
</div>
),
}));

// Mock Redux selectors
jest.mock('@/redux/selectors/index.js', () => ({
delayedUpdateSelector: (state) => state.ui.useDelayed,
}));

// Mock icons
jest.mock('@tokens-studio/icons/FloppyDisk.js', () => () => <div>Save</div>);

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(
<Provider store={store}>
<ColorField port={port} readOnly={false} />
</Provider>
);

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(
<Provider store={store}>
<ColorField port={port} readOnly={false} />
</Provider>
);

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(
<Provider store={store}>
<ColorField port={port} readOnly={false} />
</Provider>
);

expect(screen.getByText('Save')).toBeInTheDocument();
});

it('should not show save button when delayed updates are disabled', () => {
const store = createMockStore(false);
const port = createMockPort();

render(
<Provider store={store}>
<ColorField port={port} readOnly={false} />
</Provider>
);

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(
<Provider store={store}>
<ColorField port={port} readOnly={false} />
</Provider>
);

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] });
});
});
14 changes: 12 additions & 2 deletions packages/graph-editor/src/components/controls/color.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Stack direction="row" justify="between" align="center" gap={2}>
<ColorPickerPopover value={val} onChange={onChange} />
<ColorPickerPopover value={colorBallValue} onChange={onChange} textValue={val} />
<Text muted>{val}</Text>
{useDelayed && (
<IconButton
icon={<FloppyDisk />}
onClick={() => (port as Input).setValue(val)}
onClick={() => (port as Input).setValue(hexToColor(val))}
/>
)}
</Stack>
Expand Down
Loading