Skip to content

Commit ef3fc03

Browse files
Copilothotlong
andcommitted
fix: propagate undo/redo state to parent onChange, remove redundant wrappers
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent 4044a4c commit ef3fc03

File tree

2 files changed

+26
-17
lines changed

2 files changed

+26
-17
lines changed

packages/plugin-designer/src/BrandingEditor.tsx

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* Outputs to BrandingConfig (AppSchema.branding) and ThemeSchema protocol.
1818
*/
1919

20-
import React, { useState, useCallback } from 'react';
20+
import React, { useState, useCallback, useRef, useEffect } from 'react';
2121
import type { BrandingConfig } from '@object-ui/types';
2222
import {
2323
Download,
@@ -115,24 +115,31 @@ export function BrandingEditor({
115115
const [previewMode, setPreviewMode] = useState<'light' | 'dark'>('light');
116116
const { current: branding, push, canUndo, canRedo, undo, redo } = useUndoRedo(initialBranding);
117117

118+
// Flag to skip onChange when updateBranding already called it
119+
const skipOnChangeRef = useRef(true); // Start true to skip initial render
120+
121+
// Sync undo/redo state changes back to parent
122+
useEffect(() => {
123+
if (skipOnChangeRef.current) {
124+
skipOnChangeRef.current = false;
125+
return;
126+
}
127+
// On undo/redo the current state changes without going through updateBranding
128+
onChange(branding);
129+
// eslint-disable-next-line react-hooks/exhaustive-deps
130+
}, [branding]);
131+
118132
// Sync changes to parent
119133
const updateBranding = useCallback(
120134
(partial: Partial<BrandingConfig>) => {
121135
const next = { ...branding, ...partial };
136+
skipOnChangeRef.current = true;
122137
push(next);
123138
onChange(next);
124139
},
125140
[branding, push, onChange],
126141
);
127142

128-
const handleUndo = useCallback(() => {
129-
undo();
130-
}, [undo]);
131-
132-
const handleRedo = useCallback(() => {
133-
redo();
134-
}, [redo]);
135-
136143
// Export JSON
137144
const handleExport = useCallback(() => {
138145
if (onExport) {
@@ -186,17 +193,17 @@ export function BrandingEditor({
186193
if ((e.ctrlKey || e.metaKey) && e.key === 'z') {
187194
e.preventDefault();
188195
if (e.shiftKey) {
189-
handleRedo();
196+
redo();
190197
} else {
191-
handleUndo();
198+
undo();
192199
}
193200
}
194201
if ((e.ctrlKey || e.metaKey) && e.key === 'y') {
195202
e.preventDefault();
196-
handleRedo();
203+
redo();
197204
}
198205
},
199-
[readOnly, handleUndo, handleRedo],
206+
[readOnly, undo, redo],
200207
);
201208

202209
const primaryColor = branding.primaryColor || '#3b82f6';
@@ -222,7 +229,7 @@ export function BrandingEditor({
222229
<button
223230
data-testid="branding-undo"
224231
type="button"
225-
onClick={handleUndo}
232+
onClick={undo}
226233
disabled={!canUndo || readOnly}
227234
className={cn(
228235
'inline-flex items-center gap-1 rounded-md border px-2.5 py-1.5 text-xs font-medium shadow-sm transition-colors',
@@ -239,7 +246,7 @@ export function BrandingEditor({
239246
<button
240247
data-testid="branding-redo"
241248
type="button"
242-
onClick={handleRedo}
249+
onClick={redo}
243250
disabled={!canRedo || readOnly}
244251
className={cn(
245252
'inline-flex items-center gap-1 rounded-md border px-2.5 py-1.5 text-xs font-medium shadow-sm transition-colors',

packages/plugin-designer/src/__tests__/BrandingEditor.test.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,8 @@ describe('BrandingEditor', () => {
222222
it('should not update on palette swatch click when readOnly', () => {
223223
const onChange = vi.fn();
224224
render(<BrandingEditor branding={EMPTY_BRANDING} onChange={onChange} readOnly />);
225+
// Clear initial useEffect call
226+
onChange.mockClear();
225227
fireEvent.click(screen.getByTestId('branding-swatch-ef4444'));
226228
expect(onChange).not.toHaveBeenCalled();
227229
});
@@ -302,8 +304,8 @@ describe('BrandingEditor', () => {
302304
key: 'z',
303305
ctrlKey: true,
304306
});
305-
// Should have called onChange twice (once for edit, once for undo)
306-
expect(onChange).toHaveBeenCalledTimes(1);
307+
// Should have called onChange twice (once for edit, once for undo via useEffect)
308+
expect(onChange).toHaveBeenCalledTimes(2);
307309
});
308310
});
309311

0 commit comments

Comments
 (0)