mirror of
https://github.com/apache/superset.git
synced 2026-05-06 16:34:32 +00:00
fix(DynamicEditableTitle): gate onSave on user input to prevent phantom revert
Track a dirtyRef that flips true only when the user actually types. handleBlur now skips onSave when no edit happened, so passive focus followed by a parent-driven title change followed by blur no longer silently reverts the parent's update with our stale local value. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -87,4 +87,29 @@ test('prop changes mid-edit do not clobber unsaved typing', async () => {
|
||||
expect(input.value).toBe('FooX');
|
||||
rerender(<DynamicEditableTitle {...props} title="Bar" />);
|
||||
expect(input.value).toBe('FooX');
|
||||
// Locks in commit semantics: blur after a real edit must persist the
|
||||
// user's typed value, even when a competing parent-driven title arrived
|
||||
// mid-edit.
|
||||
fireEvent.blur(input);
|
||||
expect(onSave).toHaveBeenCalledWith('FooX');
|
||||
});
|
||||
|
||||
test('passive focus then parent-driven title change then blur does not revert', () => {
|
||||
// Phantom-revert scenario: user clicks the input but does not type, the
|
||||
// parent autosaves a new title from elsewhere, then the user blurs. The
|
||||
// component must NOT call onSave with the stale local value, otherwise it
|
||||
// would silently overwrite the parent's update.
|
||||
const onSave = jest.fn();
|
||||
const props = {
|
||||
placeholder: 'placeholder',
|
||||
canEdit: true,
|
||||
label: 'Title',
|
||||
onSave,
|
||||
};
|
||||
const { rerender } = render(<DynamicEditableTitle {...props} title="Foo" />);
|
||||
const input = screen.getByRole('textbox') as HTMLInputElement;
|
||||
userEvent.click(input);
|
||||
rerender(<DynamicEditableTitle {...props} title="Bar" />);
|
||||
fireEvent.blur(input);
|
||||
expect(onSave).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -81,6 +81,11 @@ export const DynamicEditableTitle = memo(
|
||||
|
||||
const sizerRef = useRef<HTMLSpanElement>(null);
|
||||
const inputRef = useRef<InputRef>(null);
|
||||
// Tracks whether the user has actually typed since entering edit mode.
|
||||
// Gates onSave so that passive focus (click without typing) followed by a
|
||||
// parent-driven title change and blur does not silently revert the
|
||||
// parent's update with our stale currentTitle.
|
||||
const dirtyRef = useRef(false);
|
||||
const { width: containerWidth, ref: containerRef } = useResizeDetector({
|
||||
refreshMode: 'debounce',
|
||||
});
|
||||
@@ -146,10 +151,19 @@ export const DynamicEditableTitle = memo(
|
||||
return;
|
||||
}
|
||||
const formattedTitle = currentTitle.trim();
|
||||
setCurrentTitle(formattedTitle);
|
||||
if (title !== formattedTitle) {
|
||||
// Only commit when the user actually typed. Passive focus must not
|
||||
// overwrite a parent-driven title change that landed mid-edit.
|
||||
if (dirtyRef.current && title !== formattedTitle) {
|
||||
setCurrentTitle(formattedTitle);
|
||||
onSave(formattedTitle);
|
||||
} else if (!dirtyRef.current) {
|
||||
// Drop any stale local state and resync to the latest title prop so a
|
||||
// subsequent edit starts from the current parent value.
|
||||
setCurrentTitle(title);
|
||||
} else {
|
||||
setCurrentTitle(formattedTitle);
|
||||
}
|
||||
dirtyRef.current = false;
|
||||
setIsEditing(false);
|
||||
}, [canEdit, currentTitle, onSave, title]);
|
||||
|
||||
@@ -166,6 +180,7 @@ export const DynamicEditableTitle = memo(
|
||||
if (!isEditing) {
|
||||
setIsEditing(true);
|
||||
}
|
||||
dirtyRef.current = true;
|
||||
setCurrentTitle(ev.target.value);
|
||||
},
|
||||
[canEdit, isEditing],
|
||||
|
||||
Reference in New Issue
Block a user