From aba7013ce5e1e8abb2ba7b7c0d1e8604e1b0f1fb Mon Sep 17 00:00:00 2001 From: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com> Date: Wed, 4 Aug 2021 09:53:52 +0200 Subject: [PATCH] TimeRangePicker: Accessiblity improvements (#36912) * Add low hanging aria markup * Add a list of radio buttons * Add ref to put focus * Add landmark markup * Add labels to icon buttons * Add radio button groupings * Add keyboard focus styles * Improve markup * Update test selector * Remove testid in favor of text * Fiddle around with test labels * Get correct testid selector * Add aria-expanded to button * Change order of quick select for keyboard nav * Create headings for easier heading navigation * Add another h3 * Add testid * Use selectors in e2e test * Add dataid selector for narrow absolute button * Update e2e test with proper selectors * Add more verbose aria-label * Change editor to picker --- .../src/selectors/components.ts | 5 +- .../grafana-e2e/src/flows/setTimeRange.ts | 11 ++-- .../src/components/Button/ToolbarButton.tsx | 8 ++- .../DateTimePickers/TimeRangePicker.test.tsx | 2 +- .../DateTimePickers/TimeRangePicker.tsx | 27 ++++++-- .../TimeRangePicker/TimePickerCalendar.tsx | 4 +- .../TimeRangePicker/TimePickerContent.tsx | 63 ++++++++++++------- .../TimeRangePicker/TimePickerFooter.tsx | 8 +-- .../TimeRangePicker/TimePickerTitle.tsx | 4 +- .../TimeRangePicker/TimeRangeForm.test.tsx | 4 +- .../TimeRangePicker/TimeRangeForm.tsx | 6 +- .../TimeRangePicker/TimeRangeList.tsx | 25 +++++--- .../TimeRangePicker/TimeRangeOption.tsx | 45 ++++++++++--- 13 files changed, 146 insertions(+), 66 deletions(-) diff --git a/packages/grafana-e2e-selectors/src/selectors/components.ts b/packages/grafana-e2e-selectors/src/selectors/components.ts index 7f422a68219..7997356efb2 100644 --- a/packages/grafana-e2e-selectors/src/selectors/components.ts +++ b/packages/grafana-e2e-selectors/src/selectors/components.ts @@ -6,11 +6,12 @@ // prefix your selector string with 'data-test-id' so that when create the selectors we know to search for it on the right attribute export const Components = { TimePicker: { - openButton: 'TimePicker Open Button', + openButton: 'data-testid TimePicker Open Button', fromField: 'TimePicker from field', toField: 'TimePicker to field', - applyTimeRange: 'TimePicker submit button', + applyTimeRange: 'data-testid TimePicker submit button', calendar: 'TimePicker calendar', + absoluteTimeRangeTitle: 'data-testid-absolute-time-range-narrow', }, DataSource: { TestData: { diff --git a/packages/grafana-e2e/src/flows/setTimeRange.ts b/packages/grafana-e2e/src/flows/setTimeRange.ts index ec781236dc6..33c20c73844 100644 --- a/packages/grafana-e2e/src/flows/setTimeRange.ts +++ b/packages/grafana-e2e/src/flows/setTimeRange.ts @@ -8,7 +8,7 @@ export interface TimeRangeConfig { } export const setTimeRange = ({ from, to, zone }: TimeRangeConfig) => { - e2e().get('[aria-label="TimePicker Open Button"]').click(); + e2e.components.TimePicker.openButton().click(); if (zone) { e2e().contains('button', 'Change time zone').click(); @@ -21,9 +21,10 @@ export const setTimeRange = ({ from, to, zone }: TimeRangeConfig) => { } // For smaller screens - e2e().get('[aria-label="TimePicker absolute time range"]').click(); + e2e.components.TimePicker.absoluteTimeRangeTitle().click(); - e2e().get('[aria-label="TimePicker from field"]').clear().type(from); - e2e().get('[aria-label="TimePicker to field"]').clear().type(to); - e2e().get('[aria-label="TimePicker submit button"]').click(); + e2e.components.TimePicker.fromField().clear().type(from); + e2e.components.TimePicker.toField().clear().type(to); + + e2e.components.TimePicker.applyTimeRange().click(); }; diff --git a/packages/grafana-ui/src/components/Button/ToolbarButton.tsx b/packages/grafana-ui/src/components/Button/ToolbarButton.tsx index ca77b32cb9c..9ecec261c08 100644 --- a/packages/grafana-ui/src/components/Button/ToolbarButton.tsx +++ b/packages/grafana-ui/src/components/Button/ToolbarButton.tsx @@ -69,7 +69,13 @@ export const ToolbarButton = forwardRef( }); const body = ( - + {!collapsed && ( -
+
@@ -246,7 +263,7 @@ const NarrowScreenForm: React.FC = (props) => { )}
)} - + ); }; @@ -261,9 +278,9 @@ const FullScreenForm: React.FC = (props) => { return ( <>
-
+

Absolute time range -

+ = (props) => { if (isEditing) { return (
-
+
{ @@ -59,13 +59,13 @@ export const TimePickerFooter: FC = (props) => { autoFocus={true} onBlur={onToggleChangeTz} /> -
+
); } return ( -
+
@@ -78,7 +78,7 @@ export const TimePickerFooter: FC = (props) => { -
+
); }; diff --git a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerTitle.tsx b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerTitle.tsx index bb5ee3395d9..1fa12216e7b 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerTitle.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerTitle.tsx @@ -9,6 +9,8 @@ const getStyle = stylesFactory((theme: GrafanaTheme) => { font-size: ${theme.typography.size.md}; font-weight: ${theme.typography.weight.semibold}; color: ${theme.colors.formLabel}; + margin: 0; + display: flex; `, }; }); @@ -17,7 +19,7 @@ export const TimePickerTitle = memo>(({ children }) => { const theme = useTheme(); const styles = getStyle(theme); - return {children}; + return

{children}

; }); TimePickerTitle.displayName = 'TimePickerTitle'; diff --git a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeForm.test.tsx b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeForm.test.tsx index ce39c06511a..37c0c33d503 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeForm.test.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeForm.test.tsx @@ -31,10 +31,10 @@ function setup(initial: TimeRange = defaultTimeRange, timeZone = 'utc'): TimeRan describe('TimeRangeForm', () => { it('should render form correcty', () => { - const { getByLabelText } = setup(); + const { getByLabelText, getByText } = setup(); const { TimePicker } = selectors.components; - expect(getByLabelText(TimePicker.applyTimeRange)).toBeInTheDocument(); + expect(getByText('Apply time range')).toBeInTheDocument(); expect(getByLabelText(TimePicker.fromField)).toBeInTheDocument(); expect(getByLabelText(TimePicker.toField)).toBeInTheDocument(); }); diff --git a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeForm.tsx b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeForm.tsx index dbad82eceeb..ba7c338836c 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeForm.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeForm.tsx @@ -96,7 +96,7 @@ export const TimeRangeForm: React.FC = (props) => { const icon = isFullscreen ? null : @@ -132,7 +132,7 @@ export const TimeRangeForm: React.FC = (props) => { timeZone={timeZone} isReversed={isReversed} /> - + ); }; diff --git a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeList.tsx b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeList.tsx index a918e3bc5b8..bfffb65350b 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeList.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeList.tsx @@ -26,7 +26,7 @@ const getOptionsStyles = stylesFactory(() => { }); interface Props { - title?: string; + title: string; options: TimeOption[]; value?: TimeOption; onChange: (option: TimeOption) => void; @@ -46,30 +46,35 @@ export const TimeRangeList: React.FC = (props) => { } return ( - <> -
- {title} -
- - +
+
+
+
+ {title} +
+ +
+
+
); }; -const Options: React.FC = ({ options, value, onChange }) => { +const Options: React.FC = ({ options, value, onChange, title }) => { const styles = getOptionsStyles(); return ( <> -
+
    {options.map((option, index) => ( ))} -
+
); diff --git a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeOption.tsx b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeOption.tsx index 36d6bc2f337..a622e8d6f0c 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeOption.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeOption.tsx @@ -2,12 +2,15 @@ import React, { memo } from 'react'; import { css, cx } from '@emotion/css'; import { GrafanaTheme2, TimeOption } from '@grafana/data'; import { useStyles2 } from '../../../themes/ThemeContext'; +import { getFocusStyles } from '../../../themes/mixins'; +import { v4 as uuidv4 } from 'uuid'; const getStyles = (theme: GrafanaTheme2) => { return { container: css` display: flex; align-items: center; + flex-direction: row-reverse; justify-content: space-between; padding: 7px 9px 7px 9px; @@ -16,10 +19,20 @@ const getStyles = (theme: GrafanaTheme2) => { cursor: pointer; } `, - selected: css` - background: ${theme.colors.action.selected}; - } - `, + selected: css` + background: ${theme.colors.action.selected}; + font-weight: ${theme.typography.fontWeightMedium}; + `, + radio: css` + opacity: 0; + + &:focus-visible + label { + ${getFocusStyles(theme)}; + } + `, + label: css` + cursor: pointer; + `, }; }; @@ -27,15 +40,31 @@ interface Props { value: TimeOption; selected?: boolean; onSelect: (option: TimeOption) => void; + /** + * Input identifier. This should be the same for all options in a group. + */ + name: string; } -export const TimeRangeOption = memo(({ value, onSelect, selected = false }) => { +export const TimeRangeOption = memo(({ value, onSelect, selected = false, name }) => { const styles = useStyles2(getStyles); + // In case there are more of the same timerange in the list + const id = uuidv4(); return ( -
onSelect(value)} tabIndex={-1}> - {value.display} -
+
  • + onSelect(value)} + /> + +
    • ); });