From 38a07dca8b692cd5bf87e85adc68dfe149c7d60a Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Tue, 3 Dec 2024 17:33:45 -0800 Subject: [PATCH 1/4] remove buggy useEffect usage --- .../form/call-taker/date-time-picker.tsx | 65 +++++++++++-------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/lib/components/form/call-taker/date-time-picker.tsx b/lib/components/form/call-taker/date-time-picker.tsx index 96292df86..7ff8c1653 100644 --- a/lib/components/form/call-taker/date-time-picker.tsx +++ b/lib/components/form/call-taker/date-time-picker.tsx @@ -5,7 +5,13 @@ import { IntlShape, useIntl } from 'react-intl' import { isMatch, parse } from 'date-fns' import { OverlayTrigger, Tooltip } from 'react-bootstrap' import coreUtils from '@opentripplanner/core-utils' -import React, { useEffect, useRef, useState } from 'react' +import React, { + ChangeEvent, + useCallback, + useEffect, + useRef, + useState +} from 'react' import { AppReduxState, FilterType, SortType } from '../../../util/state-types' import { DepartArriveTypeMap, DepartArriveValue } from '../date-time-modal' @@ -197,32 +203,39 @@ const DateTimeOptions = ({ }) }) } - - if ( - syncSortWithDepartArrive && - DepartArriveTypeMap[departArrive] !== sort.type - ) { - importedUpdateItineraryFilter({ - sort: { - ...sort, - type: DepartArriveTypeMap[departArrive] - } - }) - } }, [dateTime, departArrive, homeTimezone, setQueryParam]) - // Handler for updating the time and date fields when NOW is selected - useEffect(() => { - if (departArrive === 'NOW') { - setTime(getCurrentTime(homeTimezone)) - setDate(getCurrentDate(homeTimezone)) - setTypedTime( - safeFormat(dateTime, timeFormat, { - timeZone: homeTimezone + const handleDepartArriveChange = useCallback( + (e: ChangeEvent) => { + const newValue = e.target.value as DepartArriveValue + setDepartArrive(newValue) + + // Handler for updating the time and date fields when NOW is selected + if (newValue === 'NOW') { + setTime(getCurrentTime(homeTimezone)) + setDate(getCurrentDate(homeTimezone)) + setTypedTime( + safeFormat(dateTime, timeFormat, { + timeZone: homeTimezone + }) + ) + } + + // Update sort type if needed + if ( + syncSortWithDepartArrive && + DepartArriveTypeMap[newValue] !== sort.type + ) { + importedUpdateItineraryFilter({ + sort: { + ...sort, + type: DepartArriveTypeMap[newValue] + } }) - ) - } - }, [departArrive, setTime, setDate, homeTimezone]) + } + }, + [syncSortWithDepartArrive, sort, importedUpdateItineraryFilter] + ) const unsetNow = () => { if (departArrive === 'NOW') setDepartArrive('DEPART') @@ -231,8 +244,8 @@ const DateTimeOptions = ({ return ( <> { - setTime(e.target.value) + handleTimeChange(e.target.value) setTypedTime(e.target.value) unsetNow() }} From 6ef15879f46da0019fd3002272d5bbae4b5ebd8a Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Wed, 4 Dec 2024 13:48:41 -0800 Subject: [PATCH 3/4] adjust initial state loading --- .../form/call-taker/date-time-picker.tsx | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/lib/components/form/call-taker/date-time-picker.tsx b/lib/components/form/call-taker/date-time-picker.tsx index 88ae8f203..5f93a0026 100644 --- a/lib/components/form/call-taker/date-time-picker.tsx +++ b/lib/components/form/call-taker/date-time-picker.tsx @@ -75,6 +75,30 @@ const safeFormat = (date: Date | '', time: string, options?: OptionsWithTZ) => { } return '' } +/** + * Parse a time input expressed in the agency time zone. + * @returns A date if the parsing succeeded, or null. + */ +const parseInputAsTime = ( + homeTimezone: string, + timeInput: string = getCurrentTime(homeTimezone), + date: string = getCurrentDate(homeTimezone) +) => { + if (!timeInput) timeInput = getCurrentTime(homeTimezone) + + // Match one of the supported time formats + const matchedTimeFormat = SUPPORTED_TIME_FORMATS.find((timeFormat) => + isMatch(timeInput, timeFormat) + ) + if (matchedTimeFormat) { + const resolvedDateTime = format( + parse(timeInput, matchedTimeFormat, new Date()), + 'HH:mm:ss' + ) + return toDate(`${date}T${resolvedDateTime}`) + } + return '' +} type Props = { date?: string @@ -127,37 +151,17 @@ const DateTimeOptions = ({ ) const [date, setDate] = useState(initialDate) const [time, setTime] = useState(initialTime) - const [typedTime, setTypedTime] = useState(initialTime) + const [typedTime, setTypedTime] = useState( + safeFormat(parseInputAsTime(homeTimezone, time, date), timeFormat, { + timeZone: homeTimezone + }) + ) const timeRef = useRef(null) const intl = useIntl() - /** - * Parse a time input expressed in the agency time zone. - * @returns A date if the parsing succeeded, or null. - */ - const parseInputAsTime = ( - timeInput: string = getCurrentTime(homeTimezone), - date: string = getCurrentDate(homeTimezone) - ) => { - if (!timeInput) timeInput = getCurrentTime(homeTimezone) - - // Match one of the supported time formats - const matchedTimeFormat = SUPPORTED_TIME_FORMATS.find((timeFormat) => - isMatch(timeInput, timeFormat) - ) - if (matchedTimeFormat) { - const resolvedDateTime = format( - parse(timeInput, matchedTimeFormat, new Date()), - 'HH:mm:ss' - ) - return toDate(`${date}T${resolvedDateTime}`) - } - return '' - } - - const dateTime = parseInputAsTime(time, date) + const dateTime = parseInputAsTime(homeTimezone, time, date) // Update state when external state is updated useEffect(() => { From caf809ca655e5b24f7c9705ae66de21a3b6426bb Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Mon, 16 Dec 2024 16:34:31 -0800 Subject: [PATCH 4/4] useCallback in input props, add comments and eslint disable to effect deps --- .../form/call-taker/date-time-picker.tsx | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/lib/components/form/call-taker/date-time-picker.tsx b/lib/components/form/call-taker/date-time-picker.tsx index 5f93a0026..47ae53ba7 100644 --- a/lib/components/form/call-taker/date-time-picker.tsx +++ b/lib/components/form/call-taker/date-time-picker.tsx @@ -169,12 +169,16 @@ const DateTimeOptions = ({ if (initialTime !== time) { handleTimeChange(initialTime || '') } + // This effect is design to flow from state to component only + // eslint-disable-next-line react-hooks/exhaustive-deps }, [initialTime, initialDate]) useEffect(() => { if (initialDepartArrive && departArrive !== initialDepartArrive) { setDepartArrive(initialDepartArrive) } + // This effect is design to flow from state to component only + // eslint-disable-next-line react-hooks/exhaustive-deps }, [initialDepartArrive]) // Handler for setting the query parameters @@ -272,11 +276,14 @@ const DateTimeOptions = ({ > { - handleTimeChange(e.target.value) - setTypedTime(e.target.value) - unsetNow() - }} + onChange={useCallback( + (e) => { + handleTimeChange(e.target.value) + setTypedTime(e.target.value) + unsetNow() + }, + [handleTimeChange, setTypedTime, unsetNow] + )} onFocus={(e) => e.target.select()} onKeyDown={onKeyDown} ref={timeRef} @@ -293,15 +300,18 @@ const DateTimeOptions = ({ { - if (!e.target.value) { - e.preventDefault() - // TODO: prevent selection from advancing to next field - return - } - setDate(e.target.value) - unsetNow() - }} + onChange={useCallback( + (e) => { + if (!e.target.value) { + e.preventDefault() + // TODO: prevent selection from advancing to next field + return + } + setDate(e.target.value) + unsetNow() + }, + [unsetNow, setDate] + )} onKeyDown={onKeyDown} style={{ fontSize: '14px',