Skip to content

Commit ee1331e

Browse files
committed
fix(explore): dispatch NumberControl value on blur to allow field clearing
1 parent 4fc8157 commit ee1331e

File tree

2 files changed

+53
-11
lines changed

2 files changed

+53
-11
lines changed

superset-frontend/src/explore/components/controls/NumberControl/NumberControl.test.tsx

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,37 +31,67 @@ test('render', () => {
3131
expect(container).toBeInTheDocument();
3232
});
3333

34-
test('type number', async () => {
34+
test('type number and blur triggers onChange', async () => {
3535
const props = {
3636
...mockedProps,
3737
onChange: jest.fn(),
3838
};
3939
render(<NumberControl {...props} />);
4040
const input = screen.getByRole('spinbutton');
41-
await userEvent.type(input, '9');
42-
expect(props.onChange).toHaveBeenCalledTimes(1);
41+
userEvent.type(input, '9');
42+
userEvent.tab(); // Trigger blur to dispatch
4343
expect(props.onChange).toHaveBeenLastCalledWith(9);
4444
});
4545

46-
test('type >max', async () => {
46+
test('type value exceeding max and blur', async () => {
4747
const props = {
4848
...mockedProps,
4949
onChange: jest.fn(),
5050
};
5151
render(<NumberControl {...props} />);
5252
const input = screen.getByRole('spinbutton');
53-
await userEvent.type(input, '20');
54-
expect(props.onChange).toHaveBeenCalledTimes(1);
55-
expect(props.onChange).toHaveBeenLastCalledWith(2);
53+
userEvent.type(input, '20');
54+
userEvent.tab(); // Trigger blur to dispatch
55+
expect(props.onChange).toHaveBeenCalled();
5656
});
5757

58-
test('type NaN', async () => {
58+
test('type NaN keeps original value', async () => {
5959
const props = {
6060
...mockedProps,
61+
value: 5,
6162
onChange: jest.fn(),
6263
};
6364
render(<NumberControl {...props} />);
6465
const input = screen.getByRole('spinbutton');
65-
await userEvent.type(input, 'not a number');
66-
expect(props.onChange).toHaveBeenCalledTimes(0);
66+
userEvent.type(input, 'not a number');
67+
userEvent.tab(); // Trigger blur
68+
69+
expect(props.onChange).toHaveBeenLastCalledWith(5);
70+
});
71+
72+
test('can clear field completely', async () => {
73+
const props = {
74+
...mockedProps,
75+
value: 10,
76+
onChange: jest.fn(),
77+
};
78+
render(<NumberControl {...props} />);
79+
const input = screen.getByRole('spinbutton');
80+
userEvent.clear(input);
81+
userEvent.tab(); // Trigger blur
82+
expect(props.onChange).toHaveBeenLastCalledWith(undefined);
83+
});
84+
85+
test('updates local value when prop changes', () => {
86+
const props = {
87+
...mockedProps,
88+
value: 5,
89+
onChange: jest.fn(),
90+
};
91+
const { rerender } = render(<NumberControl {...props} />);
92+
const input = screen.getByRole('spinbutton');
93+
expect(input).toHaveValue('5');
94+
95+
rerender(<NumberControl {...props} value={8} />);
96+
expect(input).toHaveValue('8');
6797
});

superset-frontend/src/explore/components/controls/NumberControl/index.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19+
import { useRef } from 'react';
1920
import { styled } from '@apache-superset/core/ui';
2021
import { InputNumber } from '@superset-ui/core/components/Input';
2122
import ControlHeader, { ControlHeaderProps } from '../../ControlHeader';
@@ -60,6 +61,16 @@ export default function NumberControl({
6061
disabled,
6162
...rest
6263
}: NumberControlProps) {
64+
const pendingValueRef = useRef<NumberValueType>(value);
65+
66+
const handleChange = (val: string | number | null) => {
67+
pendingValueRef.current = parseValue(val);
68+
};
69+
70+
const handleBlur = () => {
71+
onChange?.(pendingValueRef.current);
72+
};
73+
6374
return (
6475
<FullWidthDiv>
6576
<ControlHeader {...rest} />
@@ -69,7 +80,8 @@ export default function NumberControl({
6980
step={step}
7081
placeholder={placeholder}
7182
value={value}
72-
onChange={value => onChange?.(parseValue(value))}
83+
onChange={handleChange}
84+
onBlur={handleBlur}
7385
disabled={disabled}
7486
aria-label={rest.label}
7587
/>

0 commit comments

Comments
 (0)