From 4f7a93c47a311f9b22ff975e9216c7adc1cb84f1 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Wed, 9 Mar 2022 17:25:43 -0800 Subject: [PATCH] refactor(TimezoneSelector): simplify override logics and tests --- .../TimezoneSelector.test.tsx | 140 +++++++++++------- .../src/components/TimezoneSelector/index.tsx | 51 +++---- 2 files changed, 114 insertions(+), 77 deletions(-) diff --git a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx index bddb9bfc66a1d..035cff842c9e2 100644 --- a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx +++ b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx @@ -18,60 +18,100 @@ */ import React from 'react'; import moment from 'moment-timezone'; -import { render, screen } from 'spec/helpers/testing-library'; +import { render, screen, waitFor } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import TimezoneSelector from './index'; -describe('TimezoneSelector', () => { - let timezone: string | undefined; - const onTimezoneChange = jest.fn(zone => { - timezone = zone; - }); - beforeEach(() => { - timezone = undefined; - }); - it('renders a TimezoneSelector with a default if undefined', () => { - jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York'); - render( - , - ); - expect(onTimezoneChange).toHaveBeenCalledWith('America/Nassau'); - }); - it('should properly select values from the offsetsToName map', async () => { - jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York'); - render( - , - ); +jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York'); - const select = screen.getByRole('combobox', { - name: 'Timezone selector', - }); - expect(select).toBeInTheDocument(); - userEvent.click(select); +const getSelectOptions = () => + waitFor(() => document.querySelectorAll('.ant-select-item-option-content')); - const isDaylight = moment('now').isDST(); - let findTitle = 'GMT -07:00 (Mountain Standard Time)'; - if (isDaylight) { - findTitle = 'GMT -06:00 (Mountain Daylight Time)'; - } - const selection = await screen.findByTitle(findTitle); - expect(selection).toBeInTheDocument(); - userEvent.click(selection); - expect(selection).toBeVisible(); - }); - it('renders a TimezoneSelector with the closest value if passed in', async () => { - render( - , - ); - expect(onTimezoneChange).toHaveBeenLastCalledWith('America/Vancouver'); +it('use the timezone from `moment` if no timezone provided', () => { + const onTimezoneChange = jest.fn(); + render(); + expect(onTimezoneChange).toHaveBeenCalledTimes(1); + expect(onTimezoneChange).toHaveBeenCalledWith('America/Nassau'); +}); + +it('update to closest deduped timezone when timezone is provided', async () => { + const onTimezoneChange = jest.fn(); + render( + , + ); + expect(onTimezoneChange).toHaveBeenCalledTimes(1); + expect(onTimezoneChange).toHaveBeenLastCalledWith('America/Vancouver'); +}); + +it('use the default timezone when an invalid timezone is provided', async () => { + const onTimezoneChange = jest.fn(); + render( + , + ); + expect(onTimezoneChange).toHaveBeenCalledTimes(1); + expect(onTimezoneChange).toHaveBeenLastCalledWith('Africa/Abidjan'); +}); + +it('can select a timezone values and returns canonical value', async () => { + const onTimezoneChange = jest.fn(); + render( + , + ); + + const searchInput = screen.getByRole('combobox', { + name: 'Timezone selector', }); + expect(searchInput).toBeInTheDocument(); + userEvent.click(searchInput); + const isDaylight = moment(moment.now()).isDST(); + + const selectedTimezone = isDaylight + ? 'GMT -04:00 (Eastern Daylight Time)' + : 'GMT -05:00 (Eastern Standard Time)'; + + // selected option ranks first + const options = await getSelectOptions(); + expect(options[0]).toHaveTextContent(selectedTimezone); + + // others are ranked by offset + expect(options[1]).toHaveTextContent('GMT -11:00 (Pacific/Pago_Pago)'); + expect(options[2]).toHaveTextContent('GMT -10:00 (Hawaii Standard Time)'); + expect(options[3]).toHaveTextContent('GMT -10:00 (America/Adak)'); + + // search for mountain time + await userEvent.type(searchInput, 'mou', { delay: 10 }); + + const findTitle = isDaylight + ? 'GMT -06:00 (Mountain Daylight Time)' + : 'GMT -07:00 (Mountain Standard Time)'; + const selectOption = await screen.findByTitle(findTitle); + expect(selectOption).toBeInTheDocument(); + userEvent.click(selectOption); + expect(onTimezoneChange).toHaveBeenCalledTimes(1); + expect(onTimezoneChange).toHaveBeenLastCalledWith('America/Cambridge_Bay'); +}); + +it('can update props and rerender with different values', async () => { + const onTimezoneChange = jest.fn(); + const { rerender } = render( + , + ); + expect(screen.getByTitle('GMT +04:00 (Asia/Dubai)')).toBeInTheDocument(); + rerender( + , + ); + expect(screen.getByTitle('GMT +08:00 (Australia/Perth)')).toBeInTheDocument(); + expect(onTimezoneChange).toHaveBeenCalledTimes(0); }); diff --git a/superset-frontend/src/components/TimezoneSelector/index.tsx b/superset-frontend/src/components/TimezoneSelector/index.tsx index a566073f034fa..d4a1db9dfb6a7 100644 --- a/superset-frontend/src/components/TimezoneSelector/index.tsx +++ b/superset-frontend/src/components/TimezoneSelector/index.tsx @@ -17,7 +17,7 @@ * under the License. */ -import React, { useEffect, useRef, useCallback } from 'react'; +import React, { useEffect, useMemo } from 'react'; import moment from 'moment-timezone'; import { t } from '@superset-ui/core'; import { Select } from 'src/components'; @@ -92,43 +92,40 @@ const TIMEZONE_OPTIONS = TIMEZONES.map(zone => ({ timezoneName: zone.name, })); +const TIMEZONE_OPTIONS_SORT_COMPARATOR = ( + a: typeof TIMEZONE_OPTIONS[number], + b: typeof TIMEZONE_OPTIONS[number], +) => + moment.tz(currentDate, a.timezoneName).utcOffset() - + moment.tz(currentDate, b.timezoneName).utcOffset(); + +TIMEZONE_OPTIONS.sort(TIMEZONE_OPTIONS_SORT_COMPARATOR); + +const matchTimezoneToOptions = (timezone: string) => + TIMEZONE_OPTIONS.find(option => option.offsets === getOffsetKey(timezone)) + ?.value || DEFAULT_TIMEZONE.value; + const TimezoneSelector = ({ onTimezoneChange, timezone }: TimezoneProps) => { - const prevTimezone = useRef(timezone); - const matchTimezoneToOptions = (timezone: string) => - TIMEZONE_OPTIONS.find(option => option.offsets === getOffsetKey(timezone)) - ?.value || DEFAULT_TIMEZONE.value; - - const updateTimezone = useCallback( - (tz: string) => { - // update the ref to track changes - prevTimezone.current = tz; - // the parent component contains the state for the value - onTimezoneChange(tz); - }, - [onTimezoneChange], + const validTimezone = useMemo( + () => matchTimezoneToOptions(timezone || moment.tz.guess()), + [timezone], ); + // force trigger a timezone update if provided `timezone` is not invalid useEffect(() => { - const updatedTz = matchTimezoneToOptions(timezone || moment.tz.guess()); - if (prevTimezone.current !== updatedTz) { - updateTimezone(updatedTz); + if (timezone !== validTimezone) { + onTimezoneChange(validTimezone); } - }, [timezone, updateTimezone]); + }, [validTimezone, onTimezoneChange, timezone]); return (