Skip to content

Commit 91955bd

Browse files
committed
fix: implement manual clear via onChange detection instead of keyDown
- Changed approach per reviewer feedback (@zombieJ) - Detect manual clear in onChange event when input becomes empty - Only allow manual clear when clearIcon is present (allowClear enabled) - If allowClear is disabled, input resets to previous valid date - Works for all input methods (keyboard, paste, voice, etc) - No keyboard event detection needed - cleaner and more general Changes: - Input.tsx: Check clearIcon in onChange to allow/prevent manual clear - SingleSelector: Delegate to onClear when date is null and clearIcon exists - useInputProps: Trigger onChange(null) when text becomes empty - Tests: Updated to use fireEvent.change instead of keyDown - Added test for allowClear={false} behavior All 455 tests pass with 100% line coverage on new code
1 parent 0172a1a commit 91955bd

File tree

3 files changed

+41
-88
lines changed

3 files changed

+41
-88
lines changed

src/PickerInput/Selector/Input.tsx

Lines changed: 10 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,18 @@ const Input = React.forwardRef<InputRef, InputProps>((props, ref) => {
142142

143143
// Directly trigger `onChange` if `format` is empty
144144
const onInternalChange: React.ChangeEventHandler<HTMLInputElement> = (event) => {
145+
const text = event.target.value;
146+
147+
// Handle manual clear only when clearIcon is present (allowClear was enabled)
148+
// If clearIcon is not set, reset back to previous valid date instead
149+
if (text === '' && value !== '' && clearIcon) {
150+
onChange('');
151+
setInputValue('');
152+
return;
153+
}
154+
145155
// Hack `onChange` with format to do nothing
146156
if (!format) {
147-
const text = event.target.value;
148-
149157
onModify(text);
150158
setInputValue(text);
151159
onChange(text);
@@ -212,36 +220,7 @@ const Input = React.forwardRef<InputRef, InputProps>((props, ref) => {
212220
});
213221

214222
// ======================= Keyboard =======================
215-
/**
216-
* Handle manual clear when user selects all text and presses Delete/Backspace
217-
* @returns true if input was cleared, false otherwise
218-
*/
219-
const handleManualClear = (event: React.KeyboardEvent<HTMLInputElement>): boolean => {
220-
const inputElement = inputRef.current;
221-
if (
222-
inputElement &&
223-
inputElement.selectionStart === 0 &&
224-
inputElement.selectionEnd === inputValue.length &&
225-
inputValue.length > 0
226-
) {
227-
onChange('');
228-
setInputValue('');
229-
// Prevent default browser behavior (e.g., navigation) after clearing
230-
event.preventDefault();
231-
return true;
232-
}
233-
return false;
234-
};
235-
236223
const onSharedKeyDown: React.KeyboardEventHandler<HTMLInputElement> = (event) => {
237-
const { key } = event;
238-
239-
if ((key === 'Backspace' || key === 'Delete') && !format) {
240-
if (handleManualClear(event)) {
241-
return;
242-
}
243-
}
244-
245224
if (event.key === 'Enter' && validateFormat(inputValue)) {
246225
onSubmit();
247226
}
@@ -293,9 +272,6 @@ const Input = React.forwardRef<InputRef, InputProps>((props, ref) => {
293272
// =============== Remove ===============
294273
case 'Backspace':
295274
case 'Delete':
296-
if (handleManualClear(event)) {
297-
return;
298-
}
299275
nextCellText = '';
300276
nextFillText = cellFormat;
301277
break;

src/PickerInput/Selector/SingleSelector/index.tsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,14 @@ function SingleSelector<DateType extends object = any>(
130130
const rootProps = useRootProps(restProps);
131131

132132
// ======================== Change ========================
133-
const handleSingleChange = (date: DateType | null) => {
134-
if (date === null) {
135-
// When date is null (from manual clear), delegate to onClear handler
136-
// to properly trigger onChange and close the picker
133+
const onSingleChange = (date: DateType | null) => {
134+
if (date === null && clearIcon) {
135+
// Only allow manual clear when clearIcon is present (allowClear was enabled)
137136
onClear?.();
138-
} else {
137+
} else if (date !== null) {
139138
onChange([date]);
140139
}
140+
// If date is null but clearIcon is not set, do nothing - let it reset to previous value
141141
};
142142

143143
const onMultipleRemove = (date: DateType) => {
@@ -156,7 +156,7 @@ function SingleSelector<DateType extends object = any>(
156156
const [getInputProps, getText] = useInputProps<DateType>(
157157
{
158158
...props,
159-
onChange: handleSingleChange,
159+
onChange: onSingleChange,
160160
},
161161
({ valueTexts }) => ({
162162
value: valueTexts[0] || '',

tests/manual-clear.spec.tsx

Lines changed: 25 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -16,69 +16,52 @@ describe('Picker.ManualClear', () => {
1616
});
1717

1818
describe('Single Picker', () => {
19-
it('should trigger onChange when manually clearing input (select all + delete)', async () => {
19+
it('should trigger onChange when manually clearing input', async () => {
2020
const onChange = jest.fn();
2121
const { container } = render(
2222
<Picker
2323
generateConfig={dayGenerateConfig}
2424
value={getDay('2023-08-01')}
2525
onChange={onChange}
2626
locale={enUS}
27+
allowClear
2728
/>,
2829
);
2930

3031
const input = container.querySelector('input') as HTMLInputElement;
3132

3233
openPicker(container);
33-
input.setSelectionRange(0, input.value.length);
34-
fireEvent.keyDown(input, { key: 'Delete', code: 'Delete' });
34+
fireEvent.change(input, { target: { value: '' } });
3535

3636
await waitFakeTimer();
3737

3838
expect(onChange).toHaveBeenCalledWith(null, null);
3939
});
4040

41-
it('should trigger onChange when manually clearing input (select all + backspace)', async () => {
41+
it('should NOT clear when allowClear is disabled - reset to previous value', async () => {
4242
const onChange = jest.fn();
4343
const { container } = render(
4444
<Picker
4545
generateConfig={dayGenerateConfig}
4646
value={getDay('2023-08-01')}
4747
onChange={onChange}
4848
locale={enUS}
49+
allowClear={false}
4950
/>,
5051
);
5152

5253
const input = container.querySelector('input') as HTMLInputElement;
5354

54-
openPicker(container);
55-
input.setSelectionRange(0, input.value.length);
56-
fireEvent.keyDown(input, { key: 'Backspace', code: 'Backspace' });
57-
58-
await waitFakeTimer();
59-
60-
expect(onChange).toHaveBeenCalledWith(null, null);
61-
});
62-
63-
it('should trigger onChange when manually clearing via input change event', async () => {
64-
const onChange = jest.fn();
65-
const { container } = render(
66-
<Picker
67-
generateConfig={dayGenerateConfig}
68-
value={getDay('2023-08-01')}
69-
onChange={onChange}
70-
locale={enUS}
71-
/>,
72-
);
73-
74-
const input = container.querySelector('input') as HTMLInputElement;
55+
expect(input.value).toBe('2023-08-01');
7556

7657
openPicker(container);
7758
fireEvent.change(input, { target: { value: '' } });
59+
fireEvent.blur(input);
7860

7961
await waitFakeTimer();
8062

81-
expect(onChange).toHaveBeenCalledWith(null, null);
63+
expect(onChange).not.toHaveBeenCalled();
64+
expect(input.value).toBe('2023-08-01');
8265
});
8366

8467
it('should reset invalid partial input on blur without triggering onChange', async () => {
@@ -115,14 +98,14 @@ describe('Picker.ManualClear', () => {
11598
onChange={onChange}
11699
locale={enUS}
117100
picker="month"
101+
allowClear
118102
/>,
119103
);
120104

121105
const input = container.querySelector('input') as HTMLInputElement;
122106

123107
openPicker(container);
124-
input.setSelectionRange(0, input.value.length);
125-
fireEvent.keyDown(input, { key: 'Delete', code: 'Delete' });
108+
fireEvent.change(input, { target: { value: '' } });
126109

127110
await waitFakeTimer();
128111

@@ -137,23 +120,19 @@ describe('Picker.ManualClear', () => {
137120
value={getDay('2023-08-01')}
138121
onChange={onChange}
139122
locale={enUS}
123+
allowClear
140124
/>,
141125
);
142126

143127
const input = container.querySelector('input') as HTMLInputElement;
144128

145129
expect(input.value).toBe('2023-08-01');
146130

147-
// Open picker
148131
openPicker(container);
149-
150-
// Simulate selecting all text and delete
151-
input.setSelectionRange(0, input.value.length);
152-
fireEvent.keyDown(input, { key: 'Delete', code: 'Delete' });
132+
fireEvent.change(input, { target: { value: '' } });
153133

154134
await waitFakeTimer();
155135

156-
// Input should be empty
157136
expect(input.value).toBe('');
158137
});
159138

@@ -166,14 +145,14 @@ describe('Picker.ManualClear', () => {
166145
onChange={onChange}
167146
locale={enUS}
168147
format={{ type: 'mask', format: 'YYYY-MM-DD' }}
148+
allowClear
169149
/>,
170150
);
171151

172152
const input = container.querySelector('input') as HTMLInputElement;
173153

174154
openPicker(container);
175-
input.setSelectionRange(0, input.value.length);
176-
fireEvent.keyDown(input, { key: 'Delete', code: 'Delete' });
155+
fireEvent.change(input, { target: { value: '' } });
177156

178157
await waitFakeTimer();
179158

@@ -192,14 +171,14 @@ describe('Picker.ManualClear', () => {
192171
onChange={onChange}
193172
locale={enUS}
194173
needConfirm={false}
174+
allowClear
195175
/>,
196176
);
197177

198178
const startInput = container.querySelectorAll('input')[0] as HTMLInputElement;
199179

200180
openPicker(container, 0);
201-
startInput.setSelectionRange(0, startInput.value.length);
202-
fireEvent.keyDown(startInput, { key: 'Delete', code: 'Delete' });
181+
fireEvent.change(startInput, { target: { value: '' } });
203182
fireEvent.blur(startInput);
204183

205184
await waitFakeTimer();
@@ -216,14 +195,14 @@ describe('Picker.ManualClear', () => {
216195
onChange={onChange}
217196
locale={enUS}
218197
needConfirm={false}
198+
allowClear
219199
/>,
220200
);
221201

222202
const endInput = container.querySelectorAll('input')[1] as HTMLInputElement;
223203

224204
openPicker(container, 1);
225-
endInput.setSelectionRange(0, endInput.value.length);
226-
fireEvent.keyDown(endInput, { key: 'Delete', code: 'Delete' });
205+
fireEvent.change(endInput, { target: { value: '' } });
227206
fireEvent.blur(endInput);
228207

229208
await waitFakeTimer();
@@ -240,21 +219,20 @@ describe('Picker.ManualClear', () => {
240219
onChange={onChange}
241220
locale={enUS}
242221
needConfirm={false}
222+
allowClear
243223
/>,
244224
);
245225

246226
const startInput = container.querySelectorAll('input')[0] as HTMLInputElement;
247227
const endInput = container.querySelectorAll('input')[1] as HTMLInputElement;
248228

249229
openPicker(container, 0);
250-
startInput.setSelectionRange(0, startInput.value.length);
251-
fireEvent.keyDown(startInput, { key: 'Delete', code: 'Delete' });
230+
fireEvent.change(startInput, { target: { value: '' } });
252231
fireEvent.blur(startInput);
253232
await waitFakeTimer();
254233

255234
openPicker(container, 1);
256-
endInput.setSelectionRange(0, endInput.value.length);
257-
fireEvent.keyDown(endInput, { key: 'Delete', code: 'Delete' });
235+
fireEvent.change(endInput, { target: { value: '' } });
258236
fireEvent.blur(endInput);
259237
await waitFakeTimer();
260238

@@ -270,6 +248,7 @@ describe('Picker.ManualClear', () => {
270248
value={[getDay('2023-08-01'), getDay('2023-08-15')]}
271249
onChange={onChange}
272250
locale={enUS}
251+
allowClear
273252
/>,
274253
);
275254

@@ -278,8 +257,7 @@ describe('Picker.ManualClear', () => {
278257
expect(startInput.value).toBe('2023-08-01');
279258

280259
openPicker(container, 0);
281-
startInput.setSelectionRange(0, startInput.value.length);
282-
fireEvent.keyDown(startInput, { key: 'Delete', code: 'Delete' });
260+
fireEvent.change(startInput, { target: { value: '' } });
283261

284262
await waitFakeTimer();
285263

@@ -304,8 +282,7 @@ describe('Picker.ManualClear', () => {
304282

305283
const input1 = container1.querySelector('input') as HTMLInputElement;
306284
openPicker(container1);
307-
input1.setSelectionRange(0, input1.value.length);
308-
fireEvent.keyDown(input1, { key: 'Delete', code: 'Delete' });
285+
fireEvent.change(input1, { target: { value: '' } });
309286
await waitFakeTimer();
310287

311288
const { container: container2 } = render(

0 commit comments

Comments
 (0)