From cf1778e696ceb9b79eacfb467e4ce03e95d5e817 Mon Sep 17 00:00:00 2001 From: crazywoola <100913391+crazywoola@users.noreply.github.com> Date: Fri, 10 Oct 2025 13:17:33 +0800 Subject: [PATCH] fix: issue w/ timepicker (#26696) Co-authored-by: lyzno1 Co-authored-by: lyzno1 <92089059+lyzno1@users.noreply.github.com> --- .../time-picker/index.spec.tsx | 95 +++++++++++++ .../time-picker/index.tsx | 131 +++++++++++++---- .../base/date-and-time-picker/types.ts | 2 +- .../date-and-time-picker/utils/dayjs.spec.ts | 67 +++++++++ .../base/date-and-time-picker/utils/dayjs.ts | 134 ++++++++++++++++-- 5 files changed, 388 insertions(+), 41 deletions(-) create mode 100644 web/app/components/base/date-and-time-picker/time-picker/index.spec.tsx create mode 100644 web/app/components/base/date-and-time-picker/utils/dayjs.spec.ts diff --git a/web/app/components/base/date-and-time-picker/time-picker/index.spec.tsx b/web/app/components/base/date-and-time-picker/time-picker/index.spec.tsx new file mode 100644 index 0000000000..40bc2928c8 --- /dev/null +++ b/web/app/components/base/date-and-time-picker/time-picker/index.spec.tsx @@ -0,0 +1,95 @@ +import React from 'react' +import { fireEvent, render, screen } from '@testing-library/react' +import TimePicker from './index' +import dayjs from '../utils/dayjs' +import { isDayjsObject } from '../utils/dayjs' + +jest.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (key: string) => { + if (key === 'time.defaultPlaceholder') return 'Pick a time...' + if (key === 'time.operation.now') return 'Now' + if (key === 'time.operation.ok') return 'OK' + if (key === 'common.operation.clear') return 'Clear' + return key + }, + }), +})) + +jest.mock('@/app/components/base/portal-to-follow-elem', () => ({ + PortalToFollowElem: ({ children }: { children: React.ReactNode }) =>
{children}
, + PortalToFollowElemTrigger: ({ children, onClick }: { children: React.ReactNode, onClick: (e: React.MouseEvent) => void }) => ( +
{children}
+ ), + PortalToFollowElemContent: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), +})) + +jest.mock('./options', () => () =>
) +jest.mock('./header', () => () =>
) + +describe('TimePicker', () => { + const baseProps = { + onChange: jest.fn(), + onClear: jest.fn(), + } + + beforeEach(() => { + jest.clearAllMocks() + }) + + test('renders formatted value for string input (Issue #26692 regression)', () => { + render( + , + ) + + expect(screen.getByDisplayValue('06:45 PM')).toBeInTheDocument() + }) + + test('confirms cleared value when confirming without selection', () => { + render( + , + ) + + const input = screen.getByRole('textbox') + fireEvent.click(input) + + const clearButton = screen.getByRole('button', { name: /clear/i }) + fireEvent.click(clearButton) + + const confirmButton = screen.getByRole('button', { name: 'OK' }) + fireEvent.click(confirmButton) + + expect(baseProps.onChange).toHaveBeenCalledTimes(1) + expect(baseProps.onChange).toHaveBeenCalledWith(undefined) + expect(baseProps.onClear).not.toHaveBeenCalled() + }) + + test('selecting current time emits timezone-aware value', () => { + const onChange = jest.fn() + render( + , + ) + + const nowButton = screen.getByRole('button', { name: 'Now' }) + fireEvent.click(nowButton) + + expect(onChange).toHaveBeenCalledTimes(1) + const emitted = onChange.mock.calls[0][0] + expect(isDayjsObject(emitted)).toBe(true) + expect(emitted?.utcOffset()).toBe(dayjs().tz('America/New_York').utcOffset()) + }) +}) diff --git a/web/app/components/base/date-and-time-picker/time-picker/index.tsx b/web/app/components/base/date-and-time-picker/time-picker/index.tsx index 1fb2cfed11..f23fcf8f4e 100644 --- a/web/app/components/base/date-and-time-picker/time-picker/index.tsx +++ b/web/app/components/base/date-and-time-picker/time-picker/index.tsx @@ -1,6 +1,13 @@ import React, { useCallback, useEffect, useRef, useState } from 'react' -import type { Period, TimePickerProps } from '../types' -import dayjs, { cloneTime, getDateWithTimezone, getHourIn12Hour } from '../utils/dayjs' +import type { Dayjs } from 'dayjs' +import { Period } from '../types' +import type { TimePickerProps } from '../types' +import dayjs, { + getDateWithTimezone, + getHourIn12Hour, + isDayjsObject, + toDayjs, +} from '../utils/dayjs' import { PortalToFollowElem, PortalToFollowElemContent, @@ -13,6 +20,11 @@ import { useTranslation } from 'react-i18next' import { RiCloseCircleFill, RiTimeLine } from '@remixicon/react' import cn from '@/utils/classnames' +const to24Hour = (hour12: string, period: Period) => { + const normalized = Number.parseInt(hour12, 10) % 12 + return period === Period.PM ? normalized + 12 : normalized +} + const TimePicker = ({ value, timezone, @@ -28,7 +40,11 @@ const TimePicker = ({ const [isOpen, setIsOpen] = useState(false) const containerRef = useRef(null) const isInitial = useRef(true) - const [selectedTime, setSelectedTime] = useState(() => value ? getDateWithTimezone({ timezone, date: value }) : undefined) + + // Initialize selectedTime + const [selectedTime, setSelectedTime] = useState(() => { + return toDayjs(value, { timezone }) + }) useEffect(() => { const handleClickOutside = (event: MouseEvent) => { @@ -39,20 +55,47 @@ const TimePicker = ({ return () => document.removeEventListener('mousedown', handleClickOutside) }, []) + // Track previous values to avoid unnecessary updates + const prevValueRef = useRef(value) + const prevTimezoneRef = useRef(timezone) + useEffect(() => { if (isInitial.current) { isInitial.current = false + // Save initial values on first render + prevValueRef.current = value + prevTimezoneRef.current = timezone return } - if (value) { - const newValue = getDateWithTimezone({ date: value, timezone }) - setSelectedTime(newValue) - onChange(newValue) + + // Only update when timezone changes but value doesn't + const valueChanged = prevValueRef.current !== value + const timezoneChanged = prevTimezoneRef.current !== timezone + + // Update reference values + prevValueRef.current = value + prevTimezoneRef.current = timezone + + // Skip if neither timezone changed nor value changed + if (!timezoneChanged && !valueChanged) return + + if (value !== undefined && value !== null) { + const dayjsValue = toDayjs(value, { timezone }) + if (!dayjsValue) return + + setSelectedTime(dayjsValue) + + if (timezoneChanged && !valueChanged) + onChange(dayjsValue) + return } - else { - setSelectedTime(prev => prev ? getDateWithTimezone({ date: prev, timezone }) : undefined) - } - }, [timezone]) + + setSelectedTime((prev) => { + if (!isDayjsObject(prev)) + return undefined + return timezone ? getDateWithTimezone({ date: prev, timezone }) : prev + }) + }, [timezone, value, onChange]) const handleClickTrigger = (e: React.MouseEvent) => { e.stopPropagation() @@ -61,8 +104,16 @@ const TimePicker = ({ return } setIsOpen(true) - if (value) - setSelectedTime(value) + + if (value) { + const dayjsValue = toDayjs(value, { timezone }) + const needsUpdate = dayjsValue && ( + !selectedTime + || !isDayjsObject(selectedTime) + || !dayjsValue.isSame(selectedTime, 'minute') + ) + if (needsUpdate) setSelectedTime(dayjsValue) + } } const handleClear = (e: React.MouseEvent) => { @@ -73,42 +124,68 @@ const TimePicker = ({ } const handleTimeSelect = (hour: string, minute: string, period: Period) => { - const newTime = cloneTime(dayjs(), dayjs(`1/1/2000 ${hour}:${minute} ${period}`)) + const periodAdjustedHour = to24Hour(hour, period) + const nextMinute = Number.parseInt(minute, 10) setSelectedTime((prev) => { - return prev ? cloneTime(prev, newTime) : newTime + const reference = isDayjsObject(prev) + ? prev + : (timezone ? getDateWithTimezone({ timezone }) : dayjs()).startOf('minute') + return reference + .set('hour', periodAdjustedHour) + .set('minute', nextMinute) + .set('second', 0) + .set('millisecond', 0) }) } + const getSafeTimeObject = useCallback(() => { + if (isDayjsObject(selectedTime)) + return selectedTime + return (timezone ? getDateWithTimezone({ timezone }) : dayjs()).startOf('day') + }, [selectedTime, timezone]) + const handleSelectHour = useCallback((hour: string) => { - const time = selectedTime || dayjs().startOf('day') + const time = getSafeTimeObject() handleTimeSelect(hour, time.minute().toString().padStart(2, '0'), time.format('A') as Period) - }, [selectedTime]) + }, [getSafeTimeObject]) const handleSelectMinute = useCallback((minute: string) => { - const time = selectedTime || dayjs().startOf('day') + const time = getSafeTimeObject() handleTimeSelect(getHourIn12Hour(time).toString().padStart(2, '0'), minute, time.format('A') as Period) - }, [selectedTime]) + }, [getSafeTimeObject]) const handleSelectPeriod = useCallback((period: Period) => { - const time = selectedTime || dayjs().startOf('day') + const time = getSafeTimeObject() handleTimeSelect(getHourIn12Hour(time).toString().padStart(2, '0'), time.minute().toString().padStart(2, '0'), period) - }, [selectedTime]) + }, [getSafeTimeObject]) const handleSelectCurrentTime = useCallback(() => { const newDate = getDateWithTimezone({ timezone }) setSelectedTime(newDate) onChange(newDate) setIsOpen(false) - }, [onChange, timezone]) + }, [timezone, onChange]) const handleConfirm = useCallback(() => { - onChange(selectedTime) + const valueToEmit = isDayjsObject(selectedTime) ? selectedTime : undefined + onChange(valueToEmit) setIsOpen(false) - }, [onChange, selectedTime]) + }, [selectedTime, onChange]) const timeFormat = 'hh:mm A' - const displayValue = value?.format(timeFormat) || '' - const placeholderDate = isOpen && selectedTime ? selectedTime.format(timeFormat) : (placeholder || t('time.defaultPlaceholder')) + + const formatTimeValue = useCallback((timeValue: string | Dayjs | undefined): string => { + if (!timeValue) return '' + + const dayjsValue = toDayjs(timeValue, { timezone }) + return dayjsValue?.format(timeFormat) || '' + }, [timezone]) + + const displayValue = formatTimeValue(value) + + const placeholderDate = isOpen && isDayjsObject(selectedTime) + ? selectedTime.format(timeFormat) + : (placeholder || t('time.defaultPlaceholder')) const inputElem = (
diff --git a/web/app/components/base/date-and-time-picker/types.ts b/web/app/components/base/date-and-time-picker/types.ts index 4ac01c142a..b51c2ebb01 100644 --- a/web/app/components/base/date-and-time-picker/types.ts +++ b/web/app/components/base/date-and-time-picker/types.ts @@ -54,7 +54,7 @@ export type TriggerParams = { onClick: (e: React.MouseEvent) => void } export type TimePickerProps = { - value: Dayjs | undefined + value: Dayjs | string | undefined timezone?: string placeholder?: string onChange: (date: Dayjs | undefined) => void diff --git a/web/app/components/base/date-and-time-picker/utils/dayjs.spec.ts b/web/app/components/base/date-and-time-picker/utils/dayjs.spec.ts new file mode 100644 index 0000000000..549ab01029 --- /dev/null +++ b/web/app/components/base/date-and-time-picker/utils/dayjs.spec.ts @@ -0,0 +1,67 @@ +import dayjs from './dayjs' +import { + getDateWithTimezone, + isDayjsObject, + toDayjs, +} from './dayjs' + +describe('dayjs utilities', () => { + const timezone = 'UTC' + + test('toDayjs parses time-only strings with timezone support', () => { + const result = toDayjs('18:45', { timezone }) + expect(result).toBeDefined() + expect(result?.format('HH:mm')).toBe('18:45') + expect(result?.utcOffset()).toBe(getDateWithTimezone({ timezone }).utcOffset()) + }) + + test('toDayjs parses 12-hour time strings', () => { + const tz = 'America/New_York' + const result = toDayjs('07:15 PM', { timezone: tz }) + expect(result).toBeDefined() + expect(result?.format('HH:mm')).toBe('19:15') + expect(result?.utcOffset()).toBe(getDateWithTimezone({ timezone: tz }).utcOffset()) + }) + + test('isDayjsObject detects dayjs instances', () => { + const date = dayjs() + expect(isDayjsObject(date)).toBe(true) + expect(isDayjsObject(getDateWithTimezone({ timezone }))).toBe(true) + expect(isDayjsObject('2024-01-01')).toBe(false) + expect(isDayjsObject({})).toBe(false) + }) + + test('toDayjs parses datetime strings in target timezone', () => { + const value = '2024-05-01 12:00:00' + const tz = 'America/New_York' + + const result = toDayjs(value, { timezone: tz }) + + expect(result).toBeDefined() + expect(result?.hour()).toBe(12) + expect(result?.format('YYYY-MM-DD HH:mm')).toBe('2024-05-01 12:00') + }) + + test('toDayjs parses ISO datetime strings in target timezone', () => { + const value = '2024-05-01T14:30:00' + const tz = 'Europe/London' + + const result = toDayjs(value, { timezone: tz }) + + expect(result).toBeDefined() + expect(result?.hour()).toBe(14) + expect(result?.minute()).toBe(30) + }) + + test('toDayjs handles dates without time component', () => { + const value = '2024-05-01' + const tz = 'America/Los_Angeles' + + const result = toDayjs(value, { timezone: tz }) + + expect(result).toBeDefined() + expect(result?.format('YYYY-MM-DD')).toBe('2024-05-01') + expect(result?.hour()).toBe(0) + expect(result?.minute()).toBe(0) + }) +}) diff --git a/web/app/components/base/date-and-time-picker/utils/dayjs.ts b/web/app/components/base/date-and-time-picker/utils/dayjs.ts index fef35bf6ca..808b50247a 100644 --- a/web/app/components/base/date-and-time-picker/utils/dayjs.ts +++ b/web/app/components/base/date-and-time-picker/utils/dayjs.ts @@ -10,6 +10,25 @@ dayjs.extend(timezone) export default dayjs const monthMaps: Record = {} +const DEFAULT_OFFSET_STR = 'UTC+0' +const TIME_ONLY_REGEX = /^(\d{1,2}):(\d{2})(?::(\d{2})(?:\.(\d{1,3}))?)?$/ +const TIME_ONLY_12H_REGEX = /^(\d{1,2}):(\d{2})(?::(\d{2}))?\s?(AM|PM)$/i + +const COMMON_PARSE_FORMATS = [ + 'YYYY-MM-DD', + 'YYYY/MM/DD', + 'DD-MM-YYYY', + 'DD/MM/YYYY', + 'MM-DD-YYYY', + 'MM/DD/YYYY', + 'YYYY-MM-DDTHH:mm:ss.SSSZ', + 'YYYY-MM-DDTHH:mm:ssZ', + 'YYYY-MM-DD HH:mm:ss', + 'YYYY-MM-DDTHH:mm', + 'YYYY-MM-DDTHH:mmZ', + 'YYYY-MM-DDTHH:mm:ss', + 'YYYY-MM-DDTHH:mm:ss.SSS', +] export const cloneTime = (targetDate: Dayjs, sourceDate: Dayjs) => { return targetDate.clone() @@ -76,21 +95,116 @@ export const getHourIn12Hour = (date: Dayjs) => { return hour === 0 ? 12 : hour >= 12 ? hour - 12 : hour } -export const getDateWithTimezone = (props: { date?: Dayjs, timezone?: string }) => { - return props.date ? dayjs.tz(props.date, props.timezone) : dayjs().tz(props.timezone) +export const getDateWithTimezone = ({ date, timezone }: { date?: Dayjs, timezone?: string }) => { + if (!timezone) + return (date ?? dayjs()).clone() + return date ? dayjs.tz(date, timezone) : dayjs().tz(timezone) } -// Asia/Shanghai -> UTC+8 -const DEFAULT_OFFSET_STR = 'UTC+0' export const convertTimezoneToOffsetStr = (timezone?: string) => { if (!timezone) return DEFAULT_OFFSET_STR const tzItem = tz.find(item => item.value === timezone) - if(!tzItem) + if (!tzItem) return DEFAULT_OFFSET_STR return `UTC${tzItem.name.charAt(0)}${tzItem.name.charAt(2)}` } +export const isDayjsObject = (value: unknown): value is Dayjs => dayjs.isDayjs(value) + +export type ToDayjsOptions = { + timezone?: string + format?: string + formats?: string[] +} + +const warnParseFailure = (value: string) => { + if (process.env.NODE_ENV !== 'production') + console.warn('[TimePicker] Failed to parse time value', value) +} + +const normalizeMillisecond = (value: string | undefined) => { + if (!value) return 0 + if (value.length === 3) return Number(value) + if (value.length > 3) return Number(value.slice(0, 3)) + return Number(value.padEnd(3, '0')) +} + +const applyTimezone = (date: Dayjs, timezone?: string) => { + return timezone ? getDateWithTimezone({ date, timezone }) : date +} + +export const toDayjs = (value: string | Dayjs | undefined, options: ToDayjsOptions = {}): Dayjs | undefined => { + if (!value) + return undefined + + const { timezone: tzName, format, formats } = options + + if (isDayjsObject(value)) + return applyTimezone(value, tzName) + + if (typeof value !== 'string') + return undefined + + const trimmed = value.trim() + + if (format) { + const parsedWithFormat = tzName + ? dayjs.tz(trimmed, format, tzName, true) + : dayjs(trimmed, format, true) + if (parsedWithFormat.isValid()) + return parsedWithFormat + } + + const timeMatch = TIME_ONLY_REGEX.exec(trimmed) + if (timeMatch) { + const base = applyTimezone(dayjs(), tzName).startOf('day') + const rawHour = Number(timeMatch[1]) + const minute = Number(timeMatch[2]) + const second = timeMatch[3] ? Number(timeMatch[3]) : 0 + const millisecond = normalizeMillisecond(timeMatch[4]) + + return base + .set('hour', rawHour) + .set('minute', minute) + .set('second', second) + .set('millisecond', millisecond) + } + + const timeMatch12h = TIME_ONLY_12H_REGEX.exec(trimmed) + if (timeMatch12h) { + const base = applyTimezone(dayjs(), tzName).startOf('day') + let hour = Number(timeMatch12h[1]) % 12 + const isPM = timeMatch12h[4]?.toUpperCase() === 'PM' + if (isPM) + hour += 12 + const minute = Number(timeMatch12h[2]) + const second = timeMatch12h[3] ? Number(timeMatch12h[3]) : 0 + + return base + .set('hour', hour) + .set('minute', minute) + .set('second', second) + .set('millisecond', 0) + } + + const candidateFormats = formats ?? COMMON_PARSE_FORMATS + for (const fmt of candidateFormats) { + const parsed = tzName + ? dayjs.tz(trimmed, fmt, tzName, true) + : dayjs(trimmed, fmt, true) + if (parsed.isValid()) + return parsed + } + + const fallbackParsed = tzName ? dayjs.tz(trimmed, tzName) : dayjs(trimmed) + if (fallbackParsed.isValid()) + return fallbackParsed + + warnParseFailure(value) + return undefined +} + // Parse date with multiple format support export const parseDateWithFormat = (dateString: string, format?: string): Dayjs | null => { if (!dateString) return null @@ -103,15 +217,7 @@ export const parseDateWithFormat = (dateString: string, format?: string): Dayjs // Try common date formats const formats = [ - 'YYYY-MM-DD', // Standard format - 'YYYY/MM/DD', // Slash format - 'DD-MM-YYYY', // European format - 'DD/MM/YYYY', // European slash format - 'MM-DD-YYYY', // US format - 'MM/DD/YYYY', // US slash format - 'YYYY-MM-DDTHH:mm:ss.SSSZ', // ISO format - 'YYYY-MM-DDTHH:mm:ssZ', // ISO format (no milliseconds) - 'YYYY-MM-DD HH:mm:ss', // Standard datetime format + ...COMMON_PARSE_FORMATS, ] for (const fmt of formats) {