mirror of
https://github.com/apache/superset.git
synced 2026-06-12 19:19:20 +00:00
Compare commits
3 Commits
fix/chart-
...
react18-ti
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
bf8bbfe514 | ||
|
|
65aea59df1 | ||
|
|
4bb6c8e932 |
@@ -70,11 +70,46 @@ test('a change event that arrives before isEditing flips is not dropped', () =>
|
|||||||
});
|
});
|
||||||
|
|
||||||
test('prop changes mid-edit do not clobber unsaved typing', async () => {
|
test('prop changes mid-edit do not clobber unsaved typing', async () => {
|
||||||
const { rerender } = render(<Harness initialTitle="Foo" />);
|
// Rerender DynamicEditableTitle directly with a changed title prop so the
|
||||||
|
// sync effect actually runs. Going through Harness would not exercise the
|
||||||
|
// bug because Harness owns its own state and only reads initialTitle once.
|
||||||
|
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;
|
const input = screen.getByRole('textbox') as HTMLInputElement;
|
||||||
userEvent.click(input);
|
userEvent.click(input);
|
||||||
await userEvent.type(input, 'X', { delay: 1 });
|
await userEvent.type(input, 'X', { delay: 1 });
|
||||||
expect(input.value).toBe('FooX');
|
expect(input.value).toBe('FooX');
|
||||||
rerender(<Harness initialTitle="Foo" />);
|
rerender(<DynamicEditableTitle {...props} title="Bar" />);
|
||||||
expect(input.value).toBe('FooX');
|
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,12 +81,25 @@ export const DynamicEditableTitle = memo(
|
|||||||
|
|
||||||
const sizerRef = useRef<HTMLSpanElement>(null);
|
const sizerRef = useRef<HTMLSpanElement>(null);
|
||||||
const inputRef = useRef<InputRef>(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({
|
const { width: containerWidth, ref: containerRef } = useResizeDetector({
|
||||||
refreshMode: 'debounce',
|
refreshMode: 'debounce',
|
||||||
});
|
});
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
setCurrentTitle(title);
|
// Don't overwrite in-flight user input when the parent re-renders with a
|
||||||
|
// new title prop mid-edit. handleBlur already syncs currentTitle on commit;
|
||||||
|
// re-running this effect when isEditing flips would resync to a stale
|
||||||
|
// title prop, so isEditing is intentionally read via closure rather than
|
||||||
|
// listed as a dep.
|
||||||
|
if (!isEditing) {
|
||||||
|
setCurrentTitle(title);
|
||||||
|
}
|
||||||
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
}, [title]);
|
}, [title]);
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (isEditing) {
|
if (isEditing) {
|
||||||
@@ -138,10 +151,19 @@ export const DynamicEditableTitle = memo(
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const formattedTitle = currentTitle.trim();
|
const formattedTitle = currentTitle.trim();
|
||||||
setCurrentTitle(formattedTitle);
|
// Only commit when the user actually typed. Passive focus must not
|
||||||
if (title !== formattedTitle) {
|
// overwrite a parent-driven title change that landed mid-edit.
|
||||||
|
if (dirtyRef.current && title !== formattedTitle) {
|
||||||
|
setCurrentTitle(formattedTitle);
|
||||||
onSave(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);
|
setIsEditing(false);
|
||||||
}, [canEdit, currentTitle, onSave, title]);
|
}, [canEdit, currentTitle, onSave, title]);
|
||||||
|
|
||||||
@@ -158,6 +180,7 @@ export const DynamicEditableTitle = memo(
|
|||||||
if (!isEditing) {
|
if (!isEditing) {
|
||||||
setIsEditing(true);
|
setIsEditing(true);
|
||||||
}
|
}
|
||||||
|
dirtyRef.current = true;
|
||||||
setCurrentTitle(ev.target.value);
|
setCurrentTitle(ev.target.value);
|
||||||
},
|
},
|
||||||
[canEdit, isEditing],
|
[canEdit, isEditing],
|
||||||
|
|||||||
Reference in New Issue
Block a user