diff --git a/packages/components/src/TimeInput.test.tsx b/packages/components/src/TimeInput.test.tsx index 092ba2f0e8..6926e0bce2 100644 --- a/packages/components/src/TimeInput.test.tsx +++ b/packages/components/src/TimeInput.test.tsx @@ -114,7 +114,7 @@ describe('selection', () => { input.focus(); input.setSelectionRange(selectionStart, selectionEnd, selectionDirection); - user.type(input, '{Shift}', { + await user.type(input, '{Shift}', { skipClick: true, initialSelectionStart: selectionStart, initialSelectionEnd: selectionEnd, @@ -150,14 +150,14 @@ describe('selection', () => { }); describe('select and type', () => { - async function testSelectAndType( + async function selectAndType( user: ReturnType, cursorPosition: number, - str: string, - expectedResult: string + str: string ) { const elementRef = React.createRef(); - const { unmount } = makeTimeInput({ ref: elementRef }); + const onChange = jest.fn(); + const { unmount } = makeTimeInput({ ref: elementRef, onChange }); const input: HTMLInputElement = screen.getByRole('textbox'); input.focus(); @@ -169,10 +169,36 @@ describe('select and type', () => { initialSelectionEnd: cursorPosition, }); await user.keyboard(str); + return { input, onChange, unmount }; + } + // Test internal/displayed value, but not the onChange callback + async function testSelectAndType( + user: ReturnType, + cursorPosition: number, + str: string, + expectedResult: string + ) { + const { input, unmount } = await selectAndType(user, cursorPosition, str); expect(input.value).toEqual(expectedResult); unmount(); } + // Test the value in onChange callback + async function testSelectAndTypeOnChange( + user: ReturnType, + cursorPosition: number, + str: string, + expectedResult: string + ) { + const { onChange, unmount } = await selectAndType( + user, + cursorPosition, + str + ); + expect(onChange).lastCalledWith(TimeUtils.parseTime(expectedResult)); + unmount(); + } + it('handles typing after autoselecting a segment', async () => { const user = userEvent.setup(); await testSelectAndType(user, 0, '0', '02:34:56'); @@ -247,6 +273,58 @@ describe('select and type', () => { unmount(); }); + it('fills in missing chars and triggers onChange', async () => { + const user = userEvent.setup(); + await testSelectAndTypeOnChange(user, 1, '{backspace}', '00:34:56'); + await testSelectAndTypeOnChange(user, 3, '{backspace}', '12:00:56'); + await testSelectAndTypeOnChange(user, 6, '{backspace}', '12:34:00'); + await testSelectAndTypeOnChange( + user, + 8, + // First backspace clears the whole section + '{backspace}{backspace}{backspace}{backspace}', + '10:00:00' + ); + }); + + it('updates the displayed value on blur', async () => { + const user = userEvent.setup(); + const onChange = jest.fn(); + const { unmount } = makeTimeInput({ onChange }); + const input: HTMLInputElement = screen.getByRole('textbox'); + input.focus(); + await user.type(input, '{shift}{backspace}{backspace}', { + initialSelectionStart: 6, + initialSelectionEnd: 6, + }); + expect(onChange).toBeCalledTimes(2); + expect(onChange).lastCalledWith(TimeUtils.parseTime('12:30:00')); + expect(input.value).toEqual('12:3'); + + input.blur(); + + // Blur should update the internal value to match the last onChange + // but not trigger another onChange + expect(onChange).toBeCalledTimes(2); + + expect(input.value).toEqual('12:30:00'); + + // Fill in missing chars in the middle + input.focus(); + await user.type(input, '{shift}{backspace}', { + skipClick: true, + initialSelectionStart: 3, + initialSelectionEnd: 3, + }); + expect(input.value).toEqual( + `12:${FIXED_WIDTH_SPACE}${FIXED_WIDTH_SPACE}:00` + ); + input.blur(); + expect(input.value).toEqual('12:00:00'); + + unmount(); + }); + it('existing edge cases', async () => { const user = userEvent.setup(); // Ideally it should change the first section to 20, i.e. '20:34:56' @@ -440,3 +518,37 @@ it('updates properly when the value prop is updated', () => { expect(textbox.value).toEqual('00:00:00'); }); + +it('ignores value prop changes matching displayed value', async () => { + const user = userEvent.setup(); + const onChange = jest.fn(); + const { rerender } = makeTimeInput({ value: 1, onChange }); + + const textbox: HTMLInputElement = screen.getByRole('textbox'); + expect(textbox.value).toEqual('00:00:01'); + + textbox.focus(); + await user.type(textbox, '{backspace}', { + skipClick: true, + initialSelectionStart: 8, + initialSelectionEnd: 8, + }); + + expect(textbox.value).toEqual('00:00:0'); + expect(onChange).toBeCalledWith(0); + + // Ignore prop update matching internal state + rerender(); + expect(textbox.value).toEqual('00:00:0'); + expect(onChange).toBeCalledTimes(1); + + // Update internal value + rerender(); + expect(textbox.value).toEqual('00:00:01'); + expect(onChange).toBeCalledTimes(1); + + // Update internal value + rerender(); + expect(textbox.value).toEqual('00:00:00'); + expect(onChange).toBeCalledTimes(1); +}); diff --git a/packages/components/src/TimeInput.tsx b/packages/components/src/TimeInput.tsx index f75a647006..3176c72519 100644 --- a/packages/components/src/TimeInput.tsx +++ b/packages/components/src/TimeInput.tsx @@ -33,6 +33,16 @@ export type TimeInputElement = { setSelection: (newSelection: SelectionSegment) => void; }; +function fixIncompleteValue(value: string): string { + // If value is not a complete HH:mm:ss time, fill missing parts with 0 + if (value != null) { + return `${value + .substring(0, 8) + .replace(/\u2007/g, '0')}${`00:00:00`.substring(value.length)}`; + } + return value; +} + // Forward ref causes a false positive for display-name in eslint: // https://github.com/yannickcr/eslint-plugin-react/issues/2269 // eslint-disable-next-line react/display-name @@ -49,6 +59,7 @@ const TimeInput = React.forwardRef( 'data-testid': dataTestId, } = props; const [value, setValue] = useState(TimeUtils.formatTime(propsValue)); + const parsedValueRef = useRef(propsValue); const [selection, setSelection] = useState(); const inputRef = useRef(null); @@ -68,9 +79,14 @@ const TimeInput = React.forwardRef( useEffect( function setFormattedTime() { - setValue(TimeUtils.formatTime(propsValue)); + // Ignore value prop update if it matches the displayed value + // to preserve the displayed value while typing + if (parsedValueRef.current !== propsValue) { + setValue(TimeUtils.formatTime(propsValue)); + parsedValueRef.current = propsValue; + } }, - [propsValue] + [parsedValueRef, propsValue] ); function getNextSegmentValue( @@ -115,15 +131,27 @@ const TimeInput = React.forwardRef( ); } - function handleChange(newValue: string): void { - log.debug('handleChange', newValue); - setValue(newValue); + const handleChange = useCallback( + (newValue: string): void => { + log.debug('handleChange', newValue); + setValue(newValue); + parsedValueRef.current = TimeUtils.parseTime( + fixIncompleteValue(newValue) + ); + onChange(parsedValueRef.current); + }, + [onChange] + ); - // Only send a change if the value is actually valid - if (TimeUtils.isTimeString(newValue)) { - onChange(TimeUtils.parseTime(newValue)); + const handleBlur = useCallback((): void => { + const fixedValue = fixIncompleteValue(value); + // Update the value displayed in the input + // onChange with the fixed value already triggered in handleChange + if (fixedValue !== value) { + setValue(fixedValue); } - } + onBlur(); + }, [value, onBlur]); const handleSelect = useCallback( (newSelection: SelectionSegment) => { @@ -146,7 +174,7 @@ const TimeInput = React.forwardRef( selection={selection} value={value} onFocus={onFocus} - onBlur={onBlur} + onBlur={handleBlur} data-testid={dataTestId} /> );