Chore: Refactor TimeRangePicker for aria-label selectors (#78399)

* Change from aria-label to data-testid for e2e selectors

* translate

* update tests

* swap buttons:

.betterer.results

@@ -752,20 +752,6 @@ exports[`better eslint`] = {
     "packages/grafana-ui/src/components/DateTimePickers/TimeRangeInput.tsx:5381": [
       [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"]
     ],
-    "packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/CalendarHeader.tsx:5381": [
-      [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"]
-    ],
-    "packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerCalendar.tsx:5381": [
-      [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"]
-    ],
-    "packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerFooter.tsx:5381": [
-      [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"]
-    ],
-    "packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeContent.tsx:5381": [
-      [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"],
-      [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "1"],
-      [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "2"]
-    ],
     "packages/grafana-ui/src/components/Drawer/Drawer.tsx:5381": [
       [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"]
     ],

packages/grafana-e2e-selectors/src/selectors/components.ts

@@ -16,13 +16,13 @@ export const Components = {
   TimePicker: {
     openButton: 'data-testid TimePicker Open Button',
     overlayContent: 'data-testid TimePicker Overlay Content',
-    fromField: 'Time Range from field',
+    fromField: 'data-testid Time Range from field',
-    toField: 'Time Range to field',
+    toField: 'data-testid Time Range to field',
     applyTimeRange: 'data-testid TimePicker submit button',
     calendar: {
-      label: 'Time Range calendar',
+      label: 'data-testid Time Range calendar',
-      openButton: 'Open time range calendar',
+      openButton: 'data-testid Open time range calendar',
-      closeButton: 'Close time range Calendar',
+      closeButton: 'data-testid Close time range Calendar',
     },
     absoluteTimeRangeTitle: 'data-testid-absolute-time-range-narrow',
   },

packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/CalendarFooter.tsx

@@ -1,44 +1,23 @@
-import { css } from '@emotion/css';
 import React from 'react';
 
-import { GrafanaTheme2 } from '@grafana/data';
-
-import { useStyles2 } from '../../../themes';
 import { Trans } from '../../../utils/i18n';
 import { Button } from '../../Button';
+import { Stack } from '../../Layout/Stack/Stack';
 
 import { TimePickerCalendarProps } from './TimePickerCalendar';
 
 export function Footer({ onClose, onApply }: TimePickerCalendarProps) {
-  const styles = useStyles2(getFooterStyles);
-
   return (
-    <div className={styles.container}>
+    <Stack gap={2} justifyContent="space-between">
-      <Button className={styles.apply} onClick={onApply}>
-        <Trans i18nKey="time-picker.calendar.apply-button">Apply time range</Trans>
-      </Button>
       <Button variant="secondary" onClick={onClose}>
         <Trans i18nKey="time-picker.calendar.cancel-button">Cancel</Trans>
       </Button>
-    </div>
+
+      <Button onClick={onApply}>
+        <Trans i18nKey="time-picker.calendar.apply-button">Apply time range</Trans>
+      </Button>
+    </Stack>
   );
 }
 
 Footer.displayName = 'Footer';
-
-const getFooterStyles = (theme: GrafanaTheme2) => {
-  return {
-    container: css({
-      backgroundColor: theme.colors.background.primary,
-      display: 'flex',
-      justifyContent: 'center',
-      padding: '10px',
-      alignItems: 'stretch',
-    }),
-    apply: css({
-      marginRight: '4px',
-      width: '100%',
-      justifyContent: 'center',
-    }),
-  };
-};

packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/CalendarHeader.tsx

@@ -1,44 +1,30 @@
-import { css } from '@emotion/css';
 import React from 'react';
 
-import { GrafanaTheme2 } from '@grafana/data';
 import { selectors } from '@grafana/e2e-selectors';
 
-import { useStyles2 } from '../../../themes';
+import { Trans, t } from '../../../utils/i18n';
-import { Trans } from '../../../utils/i18n';
+import { IconButton } from '../../IconButton/IconButton';
-import { Button } from '../../Button';
+import { Stack } from '../../Layout/Stack/Stack';
 
 import { TimePickerCalendarProps } from './TimePickerCalendar';
 import { TimePickerTitle } from './TimePickerTitle';
 
 export function Header({ onClose }: TimePickerCalendarProps) {
-  const styles = useStyles2(getHeaderStyles);
-
   return (
-    <div className={styles.container}>
+    <Stack justifyContent="space-between">
       <TimePickerTitle>
         <Trans i18nKey="time-picker.calendar.select-time">Select a time range</Trans>
       </TimePickerTitle>
-      <Button
+
-        aria-label={selectors.components.TimePicker.calendar.closeButton}
+      <IconButton
-        icon="times"
+        data-testid={selectors.components.TimePicker.calendar.closeButton}
+        tooltip={t(`time-picker.calendar.close`, 'Close calendar')}
+        name="times"
         variant="secondary"
         onClick={onClose}
       />
-    </div>
+    </Stack>
   );
 }
 
 Header.displayName = 'Header';
-
-const getHeaderStyles = (theme: GrafanaTheme2) => {
-  return {
-    container: css({
-      backgroundColor: theme.colors.background.primary,
-      display: 'flex',
-      alignItems: 'center',
-      justifyContent: 'space-between',
-      padding: '7px',
-    }),
-  };
-};

packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerCalendar.tsx

@@ -19,27 +19,28 @@ export const getStyles = (theme: GrafanaTheme2, isReversed = false) => {
     container: css({
       top: 0,
       position: 'absolute',
-      [`${isReversed ? 'left' : 'right'}`]: '544px',
+      [`${isReversed ? 'left' : 'right'}`]: '546px', // lmao
+    }),
+
+    modalContainer: css({
+      label: 'modalContainer',
+      margin: '0 auto',
+    }),
+
+    calendar: css({
+      display: 'flex',
+      flexDirection: 'column',
+      gap: theme.spacing(1),
+      padding: theme.spacing(1),
+      label: 'calendar',
       boxShadow: theme.shadows.z3,
       backgroundColor: theme.colors.background.primary,
-      zIndex: -1,
       border: `1px solid ${theme.colors.border.weak}`,
-      borderTopLeftRadius: theme.shape.radius.default,
+      borderRadius: theme.shape.radius.default,
-      borderBottomLeftRadius: theme.shape.radius.default,
-
-      '&:after': {
-        display: 'block',
-        backgroundColor: theme.colors.background.primary,
-        width: '19px',
-        height: '100%',
-        content: `${!isReversed ? '" "' : '""'}`,
-        position: 'absolute',
-        top: 0,
-        right: '-19px',
-        borderLeft: `1px solid ${theme.colors.border.weak}`,
-      },
     }),
+
     modal: css({
+      label: 'modal',
       boxShadow: theme.shadows.z3,
       left: '50%',
       position: 'fixed',
@@ -47,10 +48,6 @@ export const getStyles = (theme: GrafanaTheme2, isReversed = false) => {
       transform: 'translate(-50%, -50%)',
       zIndex: theme.zIndex.modal,
     }),
-    content: css({
-      margin: '0 auto',
-      width: '268px',
-    }),
   };
 };
 
@@ -61,6 +58,11 @@ export interface TimePickerCalendarProps {
   onClose: () => void;
   onApply: (e: FormEvent<HTMLButtonElement>) => void;
   onChange: (from: DateTime, to: DateTime) => void;
+
+  /**
+   * When true, the calendar is rendered as a floating "tooltip" next to the input.
+   * When false, the calendar is rendered "fullscreen" in a modal. Yes. Don't ask.
+   */
   isFullscreen: boolean;
   timeZone?: TimeZone;
   isReversed?: boolean;
@@ -70,7 +72,7 @@ function TimePickerCalendar(props: TimePickerCalendarProps) {
   const theme = useTheme2();
   const { modalBackdrop } = useStyles2(getModalStyles);
   const styles = getStyles(theme, props.isReversed);
-  const { isOpen, isFullscreen, onClose } = props;
+  const { isOpen, isFullscreen: isFullscreenProp, onClose } = props;
   const ref = React.createRef<HTMLElement>();
   const { dialogProps } = useDialog(
     {
@@ -87,17 +89,31 @@ function TimePickerCalendar(props: TimePickerCalendarProps) {
     ref
   );
 
+  // This prop is confusingly worded, so rename it to something more intuitive.
+  const showInModal = !isFullscreenProp;
+
   if (!isOpen) {
     return null;
   }
 
-  if (isFullscreen) {
+  const calendar = (
+    <section
+      className={styles.calendar}
+      ref={ref}
+      {...overlayProps}
+      {...dialogProps}
+      data-testid={selectors.components.TimePicker.calendar.label}
+    >
+      <Header {...props} />
+      <Body {...props} />
+      {showInModal && <Footer {...props} />}
+    </section>
+  );
+
+  if (!showInModal) {
     return (
       <FocusScope contain restoreFocus autoFocus>
-        <section className={styles.container} ref={ref} {...overlayProps} {...dialogProps}>
+        <div className={styles.container}>{calendar}</div>
-          <Header {...props} />
-          <Body {...props} />
-        </section>
       </FocusScope>
     );
   }
@@ -105,14 +121,11 @@ function TimePickerCalendar(props: TimePickerCalendarProps) {
   return (
     <OverlayContainer>
       <div className={modalBackdrop} />
+
       <FocusScope contain autoFocus restoreFocus>
-        <section className={styles.modal} ref={ref} {...overlayProps} {...dialogProps}>
+        <div className={styles.modal}>
-          <div className={styles.content} aria-label={selectors.components.TimePicker.calendar.label}>
+          <div className={styles.modalContainer}>{calendar}</div>
-            <Header {...props} />
+        </div>
-            <Body {...props} />
-            <Footer {...props} />
-          </div>
-        </section>
       </FocusScope>
     </OverlayContainer>
   );

packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerContent.test.tsx

@@ -96,22 +96,22 @@ describe('TimePickerContent', () => {
 
     it('renders with absolute picker when absolute value and quick ranges are visible', () => {
       renderComponent({ value: absoluteValue, isFullscreen: false });
-      expect(screen.queryByLabelText(/time range from field/i)).toBeInTheDocument();
+      expect(screen.queryByLabelText('From')).toBeInTheDocument();
     });
 
     it('renders with absolute picker when absolute value and quick ranges are hidden', () => {
       renderComponent({ value: absoluteValue, isFullscreen: false, hideQuickRanges: true });
-      expect(screen.queryByLabelText(/time range from field/i)).toBeInTheDocument();
+      expect(screen.queryByLabelText('From')).toBeInTheDocument();
     });
 
     it('renders without absolute picker when narrow screen and quick ranges are visible', () => {
       renderComponent({ value: relativeValue, isFullscreen: false });
-      expect(screen.queryByLabelText(/time range from field/i)).not.toBeInTheDocument();
+      expect(screen.queryByLabelText('From')).not.toBeInTheDocument();
     });
 
     it('renders with absolute picker when narrow screen and quick ranges are hidden', () => {
       renderComponent({ value: relativeValue, isFullscreen: false, hideQuickRanges: true });
-      expect(screen.queryByLabelText(/time range from field/i)).toBeInTheDocument();
+      expect(screen.queryByLabelText('From')).toBeInTheDocument();
     });
 
     it('renders without timezone picker', () => {

packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerFooter.tsx

@@ -109,7 +109,7 @@ export const TimePickerFooter = (props: Props) => {
             </section>
           ) : (
             <section
-              aria-label={selectors.components.TimeZonePicker.containerV2}
+              data-testid={selectors.components.TimeZonePicker.containerV2}
               className={cx(style.timeZoneContainer, style.timeSettingContainer)}
             >
               <Field

packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeContent.test.tsx

@@ -36,18 +36,17 @@ function setup(initial: TimeRange = defaultTimeRange, timeZone = 'utc'): TimeRan
 describe('TimeRangeForm', () => {
   it('should render form correcty', () => {
     const { getByLabelText, getByText, getAllByRole } = setup();
-    const { TimePicker } = selectors.components;
 
     expect(getByText('Apply time range')).toBeInTheDocument();
-    expect(getAllByRole('button', { name: TimePicker.calendar.openButton })).toHaveLength(2);
+    expect(getAllByRole('button', { name: 'Open calendar' })).toHaveLength(2);
-    expect(getByLabelText(TimePicker.fromField)).toBeInTheDocument();
+    expect(getByLabelText('From')).toBeInTheDocument();
-    expect(getByLabelText(TimePicker.toField)).toBeInTheDocument();
+    expect(getByLabelText('To')).toBeInTheDocument();
   });
 
   it('should display calendar when clicking the calendar icon', () => {
     const { getByLabelText, getAllByRole } = setup();
     const { TimePicker } = selectors.components;
-    const openCalendarButton = getAllByRole('button', { name: TimePicker.calendar.openButton });
+    const openCalendarButton = getAllByRole('button', { name: 'Open calendar' });
 
     fireEvent.click(openCalendarButton[0]);
     expect(getByLabelText(TimePicker.calendar.label)).toBeInTheDocument();
@@ -55,24 +54,23 @@ describe('TimeRangeForm', () => {
 
   it('should have passed time range entered in form', () => {
     const { getByLabelText } = setup();
-    const { TimePicker } = selectors.components;
 
     const fromValue = defaultTimeRange.raw.from as string;
     const toValue = defaultTimeRange.raw.to as string;
 
-    expect(getByLabelText(TimePicker.fromField)).toHaveValue(fromValue);
+    expect(getByLabelText('From')).toHaveValue(fromValue);
-    expect(getByLabelText(TimePicker.toField)).toHaveValue(toValue);
+    expect(getByLabelText('To')).toHaveValue(toValue);
   });
 
   it('should close calendar when clicking the close icon', () => {
     const { queryByLabelText, getAllByRole, getByRole } = setup();
     const { TimePicker } = selectors.components;
-    const openCalendarButton = getAllByRole('button', { name: TimePicker.calendar.openButton });
+    const openCalendarButton = getAllByRole('button', { name: 'Open calendar' });
 
     fireEvent.click(openCalendarButton[0]);
-    expect(getByRole('button', { name: TimePicker.calendar.closeButton })).toBeInTheDocument();
+    expect(getByRole('button', { name: 'Close calendar' })).toBeInTheDocument();
 
-    fireEvent.click(getByRole('button', { name: TimePicker.calendar.closeButton }));
+    fireEvent.click(getByRole('button', { name: 'Close calendar' }));
     expect(queryByLabelText(TimePicker.calendar.label)).toBeNull();
   });
 
@@ -85,8 +83,7 @@ describe('TimeRangeForm', () => {
 
   it('should have passed time range selected in calendar', () => {
     const { getAllByRole, getCalendarDayByLabelText } = setup();
-    const { TimePicker } = selectors.components;
+    const openCalendarButton = getAllByRole('button', { name: 'Open calendar' });
-    const openCalendarButton = getAllByRole('button', { name: TimePicker.calendar.openButton });
 
     fireEvent.click(openCalendarButton[0]);
     const from = getCalendarDayByLabelText('June 17, 2021');
@@ -98,8 +95,7 @@ describe('TimeRangeForm', () => {
 
   it('should select correct time range in calendar when having a custom time zone', () => {
     const { getAllByRole, getCalendarDayByLabelText } = setup(defaultTimeRange, 'Asia/Tokyo');
-    const { TimePicker } = selectors.components;
+    const openCalendarButton = getAllByRole('button', { name: 'Open calendar' });
-    const openCalendarButton = getAllByRole('button', { name: TimePicker.calendar.openButton });
 
     fireEvent.click(openCalendarButton[1]);
     const from = getCalendarDayByLabelText('June 17, 2021');

packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeContent.tsx

@@ -1,5 +1,5 @@
 import { css } from '@emotion/css';
-import React, { FormEvent, useCallback, useEffect, useState } from 'react';
+import React, { FormEvent, useCallback, useEffect, useId, useState } from 'react';
 
 import {
   DateTime,
@@ -55,6 +55,9 @@ export const TimeRangeContent = (props: Props) => {
   const [to, setTo] = useState<InputState>(toValue);
   const [isOpen, setOpen] = useState(false);
 
+  const fromFieldId = useId();
+  const toFieldId = useId();
+
   // Synchronize internal state with external value
   useEffect(() => {
     const [fromValue, toValue] = valueToState(value.raw.from, value.raw.to, timeZone);
@@ -113,7 +116,8 @@ export const TimeRangeContent = (props: Props) => {
 
   const icon = (
     <Button
-      aria-label={selectors.components.TimePicker.calendar.openButton}
+      aria-label={t('time-picker.range-content.open-input-calendar', 'Open calendar')}
+      data-testid={selectors.components.TimePicker.calendar.openButton}
       icon="calendar-alt"
       variant="secondary"
       type="button"
@@ -130,11 +134,12 @@ export const TimeRangeContent = (props: Props) => {
           error={from.errorMessage}
         >
           <Input
+            id={fromFieldId}
             onClick={(event) => event.stopPropagation()}
             onChange={(event) => onChange(event.currentTarget.value, to.value)}
             addonAfter={icon}
             onKeyDown={submitOnEnter}
-            aria-label={selectors.components.TimePicker.fromField}
+            data-testid={selectors.components.TimePicker.fromField}
             value={from.value}
           />
         </Field>
@@ -143,11 +148,12 @@ export const TimeRangeContent = (props: Props) => {
       <div className={style.fieldContainer}>
         <Field label={t('time-picker.range-content.to-input', 'To')} invalid={to.invalid} error={to.errorMessage}>
           <Input
+            id={toFieldId}
             onClick={(event) => event.stopPropagation()}
             onChange={(event) => onChange(from.value, event.currentTarget.value)}
             addonAfter={icon}
             onKeyDown={submitOnEnter}
-            aria-label={selectors.components.TimePicker.toField}
+            data-testid={selectors.components.TimePicker.toField}
             value={to.value}
           />
         </Field>

public/app/core/components/TimePicker/TimePickerWithHistory.test.tsx

@@ -9,8 +9,8 @@ import { TimePickerWithHistory } from './TimePickerWithHistory';
 describe('TimePickerWithHistory', () => {
   // In some of the tests we close and re-open the picker. When we do that we must re-find these inputs
   // as new elements will have been mounted
-  const getFromField = () => screen.getByLabelText('Time Range from field');
+  const getFromField = () => screen.getByLabelText('From');
-  const getToField = () => screen.getByLabelText('Time Range to field');
+  const getToField = () => screen.getByLabelText('To');
   const getApplyButton = () => screen.getByRole('button', { name: 'Apply time range' });
 
   const LOCAL_STORAGE_KEY = 'grafana.dashboard.timepicker.history';

public/locales/de-DE/grafana.json

@@ -1233,6 +1233,7 @@
     "calendar": {
       "apply-button": "Zeitbereich anwenden",
       "cancel-button": "Abbrechen",
+      "close": "",
       "select-time": "Einen Zeitbereich auswählen"
     },
     "content": {
@@ -1252,6 +1253,7 @@
       "default-error": "Ein vergangenes Datum oder \"heutiges\" eingeben",
       "fiscal-year": "Geschäftsjahr",
       "from-input": "Von",
+      "open-input-calendar": "",
       "range-error": "„Von“ darf nicht nach „Bis“ sein",
       "to-input": "Bis"
     },

public/locales/en-US/grafana.json

@@ -1233,6 +1233,7 @@
     "calendar": {
       "apply-button": "Apply time range",
       "cancel-button": "Cancel",
+      "close": "Close calendar",
       "select-time": "Select a time range"
     },
     "content": {
@@ -1252,6 +1253,7 @@
       "default-error": "Please enter a past date or \"now\"",
       "fiscal-year": "Fiscal year",
       "from-input": "From",
+      "open-input-calendar": "Open calendar",
       "range-error": "\"From\" can't be after \"To\"",
       "to-input": "To"
     },

public/locales/es-ES/grafana.json

@@ -1239,6 +1239,7 @@
     "calendar": {
       "apply-button": "Aplicar intervalo de tiempo",
       "cancel-button": "Cancelar",
+      "close": "",
       "select-time": "Seleccionar un intervalo de tiempo"
     },
     "content": {
@@ -1258,6 +1259,7 @@
       "default-error": "Introducir una fecha anterior o «ahora»",
       "fiscal-year": "Ejercicio fiscal",
       "from-input": "Desde",
+      "open-input-calendar": "",
       "range-error": "«Desde» no puede ser posterior a «hasta»",
       "to-input": "Hasta"
     },

public/locales/fr-FR/grafana.json

@@ -1239,6 +1239,7 @@
     "calendar": {
       "apply-button": "Appliquer la plage de temps",
       "cancel-button": "Annuler",
+      "close": "",
       "select-time": "Sélectionner une plage de temps"
     },
     "content": {
@@ -1258,6 +1259,7 @@
       "default-error": "Entrez une date dans le passé ou « maintenant »",
       "fiscal-year": "Exercice fiscal",
       "from-input": "De",
+      "open-input-calendar": "",
       "range-error": "« De » ne peut pas être ultérieur à « À »",
       "to-input": "À"
     },

public/locales/pseudo-LOCALE/grafana.json

@@ -1233,6 +1233,7 @@
     "calendar": {
       "apply-button": "Åppľy ŧįmę řäʼnģę",
       "cancel-button": "Cäʼnčęľ",
+      "close": "Cľőşę čäľęʼnđäř",
       "select-time": "Ŝęľęčŧ ä ŧįmę řäʼnģę"
     },
     "content": {
@@ -1252,6 +1253,7 @@
       "default-error": "Pľęäşę ęʼnŧęř ä päşŧ đäŧę őř \"ʼnőŵ\"",
       "fiscal-year": "Fįşčäľ yęäř",
       "from-input": "Fřőm",
+      "open-input-calendar": "Øpęʼn čäľęʼnđäř",
       "range-error": "\"Fřőm\" čäʼn'ŧ þę äƒŧęř \"Ŧő\"",
       "to-input": "Ŧő"
     },

public/locales/zh-Hans/grafana.json

@@ -1227,6 +1227,7 @@
     "calendar": {
       "apply-button": "应用时间范围",
       "cancel-button": "取消",
+      "close": "",
       "select-time": "选择时间范围"
     },
     "content": {
@@ -1246,6 +1247,7 @@
       "default-error": "请输入一个过去的日期或\"现在\"",
       "fiscal-year": "财政年度",
       "from-input": "发件人",
+      "open-input-calendar": "",
       "range-error": "“发件人”不能在“收件人”之后",
       "to-input": "结束"
     },