[FX-3114] Do not emit incorrect time values (#4031)

.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

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<Status, 'error' | 'default'>
+  /** 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',

packages/picasso/src/TimePicker/story/Default.example.tsx

@@ -4,15 +4,7 @@ import { TimePicker } from '@toptal/picasso'
 const DefaultExample = () => {
   const [timepickerValue, setTimepickerValue] = useState<string>('18:00')
 
-  const handleChange = (
-    e: React.ChangeEvent<
-      HTMLInputElement | HTMLTextAreaElement | HTMLSelectElement
-    >
-  ) => {
-    setTimepickerValue(e.target.value)
-  }
-
-  return <TimePicker onChange={handleChange} value={timepickerValue} />
+  return <TimePicker onChange={setTimepickerValue} value={timepickerValue} />
 }
 
 export default DefaultExample

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(
+        <TimePicker value={time} onChange={handleChange} />
+      )
+
+      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)
+    })
+  })
 })