aminghadersohi commented on code in PR #44878:
URL: https://github.com/apache/superset/pull/44878#discussion_r4413951728
##########
superset-frontend/src/explore/components/controls/TextControl/TextControl.test.tsx:
##########
@@ -118,3 +121,132 @@ test('should keep showing an externally updated value
across later re-renders',
rerender(<TextControl {...mockedProps} value="200" label="Row limit" />);
expect(screen.getByDisplayValue('200')).toBeInTheDocument();
});
+
+test('should not call onChange on mount', () => {
+ const onChange = jest.fn();
+ render(<TextControl {...mockedProps} onChange={onChange} />);
+ expect(onChange).not.toHaveBeenCalled();
+});
+
+test('should not call onChange again when a parent re-render hands down a new
onChange identity', async () => {
+ const onChangeA = jest.fn();
+ const onChangeB = jest.fn();
+ const { rerender } = render(
+ <TextControl {...mockedProps} value="1" onChange={onChangeA} />,
+ );
+ await userEvent.type(screen.getByRole('textbox'), '2');
+ await waitFor(() => expect(onChangeA).toHaveBeenCalledTimes(1));
+
+ // a parent re-rendering with a fresh inline callback (common in control
+ // panels) must not, by itself, re-fire the deferred-value effect
+ rerender(<TextControl {...mockedProps} value="1" onChange={onChangeB} />);
+ expect(onChangeA).toHaveBeenCalledTimes(1);
+ expect(onChangeB).not.toHaveBeenCalled();
+
+ // the next edit must reach the latest handler, not the stale one
+ await userEvent.type(screen.getByRole('textbox'), '3');
+ await waitFor(() => expect(onChangeB).toHaveBeenCalledWith('123', []));
+ expect(onChangeA).toHaveBeenCalledTimes(1);
+});
+
+test('should report a repeated identical edit after an external reset', async
() => {
+ const onChange = jest.fn();
+ const { rerender } = render(
+ <TextControl {...mockedProps} value="75" onChange={onChange} />,
+ );
+ const input = screen.getByRole('textbox');
+ await userEvent.clear(input);
+ await waitFor(() => expect(onChange).toHaveBeenCalledTimes(1));
+ rerender(<TextControl {...mockedProps} value="" onChange={onChange} />);
+ rerender(<TextControl {...mockedProps} value="75" onChange={onChange} />);
+ await userEvent.clear(input);
+ await waitFor(() => expect(onChange).toHaveBeenCalledTimes(2));
+ expect(onChange).toHaveBeenLastCalledWith('', []);
+});
+
+test('should coalesce a burst of keystrokes into one onChange with the settled
value', async () => {
+ const onChange = jest.fn();
+ render(<TextControl {...mockedProps} value="" onChange={onChange} />);
+ await userEvent.type(screen.getByRole('textbox'), '120');
+ await waitFor(() => expect(onChange).toHaveBeenCalled());
+ expect(onChange).toHaveBeenCalledTimes(1);
+ expect(onChange).toHaveBeenCalledWith('120', []);
+});
+
+test('should flush the latest typed value on blur before the debounce fires',
() => {
+ jest.useFakeTimers();
+ try {
+ const onChange = jest.fn();
+ render(<TextControl {...mockedProps} value="" onChange={onChange} />);
+ const input = screen.getByRole('textbox');
+ fireEvent.change(input, { target: { value: 'abc' } });
+ expect(onChange).not.toHaveBeenCalled();
+ fireEvent.blur(input);
+ expect(onChange).toHaveBeenCalledTimes(1);
+ expect(onChange).toHaveBeenCalledWith('abc', []);
+ act(() => {
+ jest.advanceTimersByTime(Constants.FAST_DEBOUNCE * 2);
+ });
+ expect(onChange).toHaveBeenCalledTimes(1);
+ } finally {
+ jest.useRealTimers();
+ }
+});
+
+test('should drop a pending edit when the value is reset externally', () => {
+ jest.useFakeTimers();
+ try {
+ const onChange = jest.fn();
+ const { rerender } = render(
+ <TextControl {...mockedProps} value="1" onChange={onChange} />,
+ );
+ fireEvent.change(screen.getByRole('textbox'), { target: { value: '12' } });
+ rerender(<TextControl {...mockedProps} value="9" onChange={onChange} />);
+ act(() => {
+ jest.advanceTimersByTime(Constants.FAST_DEBOUNCE * 2);
+ });
+ expect(onChange).not.toHaveBeenCalled();
+ expect(screen.getByDisplayValue('9')).toBeInTheDocument();
+ } finally {
+ jest.useRealTimers();
+ }
+});
+
+test('should not flush a stale edit on blur after an external reset', () => {
+ jest.useFakeTimers();
+ try {
+ const onChange = jest.fn();
+ const { rerender } = render(
+ <TextControl {...mockedProps} value="1" onChange={onChange} />,
+ );
+ const input = screen.getByRole('textbox');
+ fireEvent.change(input, { target: { value: '12' } });
+ rerender(<TextControl {...mockedProps} value="9" onChange={onChange} />);
Review Comment:
The reset rerender lets the deferred-null effect cancel the queued edit
before blur, so this test passes even without the new blur guard. Hold the
deferred value at '12' across the reset so blur must cancel the stale
invocation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]