diff --git a/.changeset/loud-geese-destroy.md b/.changeset/loud-geese-destroy.md new file mode 100644 index 0000000000..7aa8f4880c --- /dev/null +++ b/.changeset/loud-geese-destroy.md @@ -0,0 +1,8 @@ +--- +'@toptal/picasso': major +--- + +### TimePicker + +- change the signature of `onChange` handler to accept `string` instead of event object. In order to migrate, please replace usage of `event.target.value` with `value` in your `onChange` handler +- do not emit incorrect time values, e.g. `12:--` or `60:00`. When input has incorrect value, the `onChange` handler will be called with empty string value diff --git a/packages/picasso/src/TimePicker/TimePicker.tsx b/packages/picasso/src/TimePicker/TimePicker.tsx index d696d0b902..adf592c8ad 100644 --- a/packages/picasso/src/TimePicker/TimePicker.tsx +++ b/packages/picasso/src/TimePicker/TimePicker.tsx @@ -1,4 +1,4 @@ -import React from 'react' +import React, { useEffect, useState } from 'react' import type { BaseProps } from '@toptal/picasso-shared' import type { Theme } from '@material-ui/core/styles' import { makeStyles } from '@material-ui/core/styles' @@ -24,6 +24,7 @@ export interface Props | 'id' | 'value' | 'onSelect' + | 'onChange' | 'type' | 'multiline' | 'rows' @@ -44,12 +45,16 @@ export interface Props value?: string /** Indicate whether `TimePicker` is in `error` or `default` state */ status?: Extract + /** Called on input change */ + onChange?: (value: string) => void } +const VALID_TIME_REGEX = new RegExp(/^([0-1][0-9]|2[0-3]):[0-5][0-9]$/) + export const TimePicker = (props: Props) => { const { - onChange, - value, + onChange: externalOnChange, + value: externalValue, width, className, error, @@ -58,6 +63,31 @@ export const TimePicker = (props: Props) => { ...rest } = props + const [value, setValue] = useState(externalValue) + + useEffect(() => { + // Set internal value based on the provided one if the later is correct + if (externalValue && VALID_TIME_REGEX.test(externalValue)) { + setValue(externalValue) + } + }, [externalValue]) + + const onChange = ( + event: React.ChangeEvent< + HTMLInputElement | HTMLTextAreaElement | HTMLSelectElement + > + ) => { + const newValue = event.target.value + + setValue(newValue) + + if (newValue && VALID_TIME_REGEX.test(newValue)) { + externalOnChange?.(newValue) + } else { + externalOnChange?.('') + } + } + usePropDeprecationWarning({ props, name: 'error', diff --git a/packages/picasso/src/TimePicker/story/Default.example.tsx b/packages/picasso/src/TimePicker/story/Default.example.tsx index 1ee2dc5374..66b1cb89a3 100644 --- a/packages/picasso/src/TimePicker/story/Default.example.tsx +++ b/packages/picasso/src/TimePicker/story/Default.example.tsx @@ -4,15 +4,7 @@ import { TimePicker } from '@toptal/picasso' const DefaultExample = () => { const [timepickerValue, setTimepickerValue] = useState('18:00') - const handleChange = ( - e: React.ChangeEvent< - HTMLInputElement | HTMLTextAreaElement | HTMLSelectElement - > - ) => { - setTimepickerValue(e.target.value) - } - - return + return } export default DefaultExample diff --git a/packages/picasso/src/TimePicker/test.tsx b/packages/picasso/src/TimePicker/test.tsx index c80ecff190..b605b0fa7b 100644 --- a/packages/picasso/src/TimePicker/test.tsx +++ b/packages/picasso/src/TimePicker/test.tsx @@ -29,4 +29,26 @@ describe('TimePicker', () => { expect(handleChange).toHaveBeenCalledTimes(1) }) + + describe('when invalid time is entered', () => { + it('calls onChange with empty value', () => { + const time = '09:00' + const handleChange = jest.fn() + const { getByDisplayValue } = render( + + ) + + const input = getByDisplayValue(time) + + fireEvent.change(input, { target: { value: '12:--' } }) + expect(handleChange).toHaveBeenCalledTimes(1) + expect(handleChange).toHaveBeenCalledWith('') + + const newTime = '12:12' + + fireEvent.change(input, { target: { value: newTime } }) + expect(handleChange).toHaveBeenCalledTimes(2) + expect(handleChange).toHaveBeenCalledWith(newTime) + }) + }) })