From 7fab894e9b0a84c01c1a11db3aa4f44464c417c0 Mon Sep 17 00:00:00 2001 From: Thomas Wikman Date: Mon, 29 Apr 2024 11:46:44 +0200 Subject: [PATCH] DateTimePicker: Alternate timezones now behave correctly (#86750) * Add failing tests for timezone handling * Fix `DateTimePicker.tsx` timezone handling - Resolves `onBlur` issue - Resolve Calendar and TimeOfDay issues - Update test to cover different timezone * Handle `console.warn` in test * Handle `console.warn` in test #2 * Better handling of invalid date When parsing date string with `dateTime`, adding a second `formatInput` aids in both parsing the actual string and avoid `console.warn` when `moment` reverts to be using `Date`. * add more test cases * Ash/proposed changes (#86854) * simplify * only need this change * formatting * const > let * add test to ensure calendar is always showing the matching day * separate state * undo story changes * update util function comments * fix for selecting date in the calendar --------- Co-authored-by: Ashley Harrison --- .../src/datetime/moment_wrapper.ts | 2 +- .../DateTimePicker/DateTimePicker.test.tsx | 207 +++++++++++++++--- .../DateTimePicker/DateTimePicker.tsx | 57 +++-- .../TimeRangePicker/CalendarBody.tsx | 39 +--- .../utils/adjustDateForReactCalendar.ts | 28 +++ 5 files changed, 254 insertions(+), 79 deletions(-) create mode 100644 packages/grafana-ui/src/components/DateTimePickers/utils/adjustDateForReactCalendar.ts diff --git a/packages/grafana-data/src/datetime/moment_wrapper.ts b/packages/grafana-data/src/datetime/moment_wrapper.ts index 0b97de1a3bf..6b60bb524bd 100644 --- a/packages/grafana-data/src/datetime/moment_wrapper.ts +++ b/packages/grafana-data/src/datetime/moment_wrapper.ts @@ -53,7 +53,7 @@ export interface DateTimeDuration { export interface DateTime extends Object { add: (amount?: DateTimeInput, unit?: DurationUnit) => DateTime; - set: (unit: DurationUnit, amount: DateTimeInput) => void; + set: (unit: DurationUnit | 'date', amount: DateTimeInput) => void; diff: (amount: DateTimeInput, unit?: DurationUnit, truncate?: boolean) => number; endOf: (unitOfTime: DurationUnit) => DateTime; format: (formatInput?: FormatInput) => string; diff --git a/packages/grafana-ui/src/components/DateTimePickers/DateTimePicker/DateTimePicker.test.tsx b/packages/grafana-ui/src/components/DateTimePickers/DateTimePicker/DateTimePicker.test.tsx index 1f4bed87550..5258ee22ad0 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/DateTimePicker/DateTimePicker.test.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/DateTimePicker/DateTimePicker.test.tsx @@ -2,15 +2,23 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; -import { dateTime } from '@grafana/data'; +import { dateTime, dateTimeAsMoment, dateTimeForTimeZone, getTimeZone, setTimeZoneResolver } from '@grafana/data'; import { Components } from '@grafana/e2e-selectors'; import { DateTimePicker, Props } from './DateTimePicker'; +// An assortment of timezones that we will test the behavior of the DateTimePicker with different timezones +const TEST_TIMEZONES = ['browser', 'Europe/Stockholm', 'America/Indiana/Marengo']; + +const defaultTimeZone = getTimeZone(); +afterAll(() => { + return setTimeZoneResolver(() => defaultTimeZone); +}); + const renderDatetimePicker = (props?: Props) => { const combinedProps = Object.assign( { - date: dateTime('2021-05-05 12:00:00'), + date: dateTimeForTimeZone(getTimeZone(), '2021-05-05 12:00:00'), onChange: () => {}, }, props @@ -26,12 +34,22 @@ describe('Date time picker', () => { expect(screen.queryByTestId('date-time-picker')).toBeInTheDocument(); }); - it('input should have a value', () => { + it.each(TEST_TIMEZONES)('input should have a value (timezone: %s)', (timeZone) => { + setTimeZoneResolver(() => timeZone); renderDatetimePicker(); - expect(screen.queryByDisplayValue('2021-05-05 12:00:00')).toBeInTheDocument(); + const dateTimeInput = screen.getByTestId(Components.DateTimePicker.input); + expect(dateTimeInput).toHaveDisplayValue('2021-05-05 12:00:00'); }); - it('should update date onblur', async () => { + it.each(TEST_TIMEZONES)('should render (timezone %s)', (timeZone) => { + setTimeZoneResolver(() => timeZone); + renderDatetimePicker(); + const dateTimeInput = screen.getByTestId(Components.DateTimePicker.input); + expect(dateTimeInput).toHaveDisplayValue('2021-05-05 12:00:00'); + }); + + it.each(TEST_TIMEZONES)('should update date onblur (timezone: %)', async (timeZone) => { + setTimeZoneResolver(() => timeZone); const onChangeInput = jest.fn(); render(); const dateTimeInput = screen.getByTestId(Components.DateTimePicker.input); @@ -42,7 +60,8 @@ describe('Date time picker', () => { expect(onChangeInput).toHaveBeenCalled(); }); - it('should not update onblur if invalid date', async () => { + it.each(TEST_TIMEZONES)('should not update onblur if invalid date (timezone: %s)', async (timeZone) => { + setTimeZoneResolver(() => timeZone); const onChangeInput = jest.fn(); render(); const dateTimeInput = screen.getByTestId(Components.DateTimePicker.input); @@ -53,31 +72,167 @@ describe('Date time picker', () => { expect(onChangeInput).not.toHaveBeenCalled(); }); - it('should be able to select values in TimeOfDayPicker without blurring the element', async () => { - renderDatetimePicker(); + it.each(TEST_TIMEZONES)( + 'should not change the day at times near the day boundary (timezone: %s)', + async (timeZone) => { + setTimeZoneResolver(() => timeZone); + const onChangeInput = jest.fn(); + render(); - // open the calendar + time picker - await userEvent.click(screen.getByLabelText('Time picker')); + // Click the calendar button + await userEvent.click(screen.getByRole('button', { name: 'Time picker' })); - // open the time of day overlay - await userEvent.click(screen.getAllByRole('textbox')[1]); + // Check the active day is the 5th + expect(screen.getByRole('button', { name: 'May 5, 2021' })).toHaveClass('react-calendar__tile--active'); - // check the hour element is visible - const hourElement = screen.getAllByRole('button', { - name: '00', - })[0]; - expect(hourElement).toBeVisible(); + // open the time of day overlay + await userEvent.click(screen.getAllByRole('textbox')[1]); - // select the hour value and check it's still visible - await userEvent.click(hourElement); - expect(hourElement).toBeVisible(); + // change the hour + await userEvent.click( + screen.getAllByRole('button', { + name: '00', + })[0] + ); - // click outside the overlay and check the hour element is no longer visible + // Check the active day is the 5th + expect(screen.getByRole('button', { name: 'May 5, 2021' })).toHaveClass('react-calendar__tile--active'); + + // change the hour + await userEvent.click( + screen.getAllByRole('button', { + name: '23', + })[0] + ); + + // Check the active day is the 5th + expect(screen.getByRole('button', { name: 'May 5, 2021' })).toHaveClass('react-calendar__tile--active'); + } + ); + + it.each(TEST_TIMEZONES)( + 'should not reset the time when selecting a different day (timezone: %s)', + async (timeZone) => { + setTimeZoneResolver(() => timeZone); + const onChangeInput = jest.fn(); + render(); + + // Click the calendar button + await userEvent.click(screen.getByRole('button', { name: 'Time picker' })); + + // Select a different day in the calendar + await userEvent.click(screen.getByRole('button', { name: 'May 15, 2021' })); + + const timeInput = screen.getAllByRole('textbox')[1]; + expect(timeInput).toHaveClass('rc-time-picker-input'); + expect(timeInput).not.toHaveDisplayValue('00:00:00'); + } + ); + + it.each(TEST_TIMEZONES)( + 'should always show the correct matching day in the calendar (timezone: %s)', + async (timeZone) => { + setTimeZoneResolver(() => timeZone); + const onChangeInput = jest.fn(); + render(); + + const dateTimeInputValue = screen.getByTestId(Components.DateTimePicker.input).getAttribute('value')!; + + // takes the string from the input + // depending on the timezone, this will look something like 2024-04-05 19:59:41 + // parses out the day value and strips the leading 0 + const day = parseInt(dateTimeInputValue.split(' ')[0].split('-')[2], 10); + + // Click the calendar button + await userEvent.click(screen.getByRole('button', { name: 'Time picker' })); + + // Check the active day matches the input + expect(screen.getByRole('button', { name: `May ${day}, 2021` })).toHaveClass('react-calendar__tile--active'); + } + ); + + it.each(TEST_TIMEZONES)( + 'should always show the correct matching day when selecting a date in the calendar (timezone: %s)', + async (timeZone) => { + setTimeZoneResolver(() => timeZone); + const onChangeInput = jest.fn(); + render(); + + // Click the calendar button + await userEvent.click(screen.getByRole('button', { name: 'Time picker' })); + + // Select a new day + const day = 8; + await userEvent.click(screen.getByRole('button', { name: `May ${day}, 2021` })); + await userEvent.click(screen.getByRole('button', { name: 'Apply' })); + + const onChangeInputArg = onChangeInput.mock.calls[0][0]; + + expect(dateTimeAsMoment(dateTimeForTimeZone(timeZone, onChangeInputArg)).date()).toBe(day); + } + ); + + it.each(TEST_TIMEZONES)('should not alter a UTC time when blurring (timezone: %s)', async (timeZone) => { + setTimeZoneResolver(() => timeZone); + const onChangeInput = jest.fn(); + + // render with a UTC value + const { rerender } = render( + + ); + + const inputValue = screen.getByTestId(Components.DateTimePicker.input).getAttribute('value')!; + + // blur the input to trigger an onChange + await userEvent.click(screen.getByTestId(Components.DateTimePicker.input)); await userEvent.click(document.body); - expect( - screen.queryByRole('button', { - name: '00', - }) - ).not.toBeInTheDocument(); + + const onChangeValue = onChangeInput.mock.calls[0][0]; + expect(onChangeInput).toHaveBeenCalledWith(onChangeValue); + + // now rerender with the "changed" value + rerender(); + + // expect the input to show the same value + expect(screen.getByTestId(Components.DateTimePicker.input)).toHaveDisplayValue(inputValue); + + // blur the input to trigger an onChange + await userEvent.click(screen.getByTestId(Components.DateTimePicker.input)); + await userEvent.click(document.body); + + // expect the onChange to be called with the same value + expect(onChangeInput).toHaveBeenCalledWith(onChangeValue); }); + + it.each(TEST_TIMEZONES)( + 'should be able to select values in TimeOfDayPicker without blurring the element (timezone: %s)', + async (timeZone) => { + setTimeZoneResolver(() => timeZone); + renderDatetimePicker(); + + // open the calendar + time picker + await userEvent.click(screen.getByLabelText('Time picker')); + + // open the time of day overlay + await userEvent.click(screen.getAllByRole('textbox')[1]); + + // check the hour element is visible + const hourElement = screen.getAllByRole('button', { + name: '00', + })[0]; + expect(hourElement).toBeVisible(); + + // select the hour value and check it's still visible + await userEvent.click(hourElement); + expect(hourElement).toBeVisible(); + + // click outside the overlay and check the hour element is no longer visible + await userEvent.click(document.body); + expect( + screen.queryByRole('button', { + name: '00', + }) + ).not.toBeInTheDocument(); + } + ); }); diff --git a/packages/grafana-ui/src/components/DateTimePickers/DateTimePicker/DateTimePicker.tsx b/packages/grafana-ui/src/components/DateTimePickers/DateTimePicker/DateTimePicker.tsx index 3df2c3b7565..60d8ba4bdb2 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/DateTimePicker/DateTimePicker.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/DateTimePicker/DateTimePicker.tsx @@ -7,7 +7,15 @@ import React, { FormEvent, ReactNode, useCallback, useEffect, useRef, useState } import Calendar from 'react-calendar'; import { useMedia } from 'react-use'; -import { dateTimeFormat, DateTime, dateTime, GrafanaTheme2, isDateTime } from '@grafana/data'; +import { + dateTimeFormat, + DateTime, + dateTime, + GrafanaTheme2, + isDateTime, + dateTimeForTimeZone, + getTimeZone, +} from '@grafana/data'; import { Components } from '@grafana/e2e-selectors'; import { useStyles2, useTheme2 } from '../../../themes'; @@ -21,6 +29,7 @@ import { Portal } from '../../Portal/Portal'; import { TimeOfDayPicker, POPUP_CLASS_NAME } from '../TimeOfDayPicker'; import { getBodyStyles } from '../TimeRangePicker/CalendarBody'; import { isValid } from '../utils'; +import { adjustDateForReactCalendar } from '../utils/adjustDateForReactCalendar'; export interface Props { /** Input date for the component */ @@ -227,7 +236,7 @@ const DateTimeInput = React.forwardRef( const onBlur = useCallback(() => { if (!internalDate.invalid) { - const date = dateTime(internalDate.value); + const date = dateTimeForTimeZone(getTimeZone(), internalDate.value); onChange(date); } }, [internalDate, onChange]); @@ -276,9 +285,18 @@ const DateTimeCalendar = React.forwardRef ) => { const calendarStyles = useStyles2(getBodyStyles); const styles = useStyles2(getStyles); - const [internalDate, setInternalDate] = useState(() => { + + // need to keep these 2 separate in state since react-calendar doesn't support different timezones + const [timeOfDayDateTime, setTimeOfDayDateTime] = useState(() => { if (date && date.isValid()) { - return date.toDate(); + return dateTimeForTimeZone(getTimeZone(), date); + } + + return dateTimeForTimeZone(getTimeZone(), new Date()); + }); + const [reactCalendarDate, setReactCalendarDate] = useState(() => { + if (date && date.isValid()) { + return adjustDateForReactCalendar(date.toDate(), getTimeZone()); } return new Date(); @@ -286,28 +304,33 @@ const DateTimeCalendar = React.forwardRef const onChangeDate = useCallback['onChange']>>((date) => { if (date && !Array.isArray(date)) { - setInternalDate((prevState) => { - // If we don't use time from prevState - // the time will be reset to 00:00:00 - date.setHours(prevState.getHours()); - date.setMinutes(prevState.getMinutes()); - date.setSeconds(prevState.getSeconds()); - - return date; - }); + setReactCalendarDate(date); } }, []); const onChangeTime = useCallback((date: DateTime) => { - setInternalDate(date.toDate()); + setTimeOfDayDateTime(date); }, []); + // here we need to stitch the 2 date objects back together + const handleApply = () => { + // we take the date that's set by TimeOfDayPicker + const newDate = dateTime(timeOfDayDateTime); + + // and apply the date/month/year set by react-calendar + newDate.set('date', reactCalendarDate.getDate()); + newDate.set('month', reactCalendarDate.getMonth()); + newDate.set('year', reactCalendarDate.getFullYear()); + + onChange(newDate); + }; + return (
} nextAriaLabel="Next month" prevLabel={} @@ -323,14 +346,14 @@ const DateTimeCalendar = React.forwardRef
-