RefreshPicker: make widget accessible (#40570)

* adds better aria-label for run and interval buttons

* enable refreshPicker to be keyboard navigable

* adds support for closing menu using esc key

* Fix: weird behaviour when navigating menu items

* adds focus trapping to refresh picker

* WIP: sanitize time interval values for screen readers to pronounce correctly

* WIP: improve sanitizeLabel function to work for all use cases

* Chore: move label sanitization to refreshPicker component instead

* Chore: add fallback label when ariaLabel prop is not set

* Chore: fix some type errors

* code cleanup

* update tests

* rename function to be more descriptive

* remove unnecessary type casting

* WIP: use cleaner solution

* WIP: use parseDuration util instead

* use more descriptive aria label

* fix: modify parseDuration util to output correct interval unit format

* fix: move interval unit format logic to refreshPicker

* Chore: add back old refreshPicker e2e selectors for backward compatibility

* Fix: improve refresh picker to voice out selected interval option

* Fix: use appropriate aria roles and states to aid screen reader a11y

* Fix: support dropdown expansion using down arrow key

* Chore: use better type construct

* Fix: add support for tab to close menu

* add more context to the deprecation warning message

* Chore: use formatDuration util instead to format interval labels

* Chore: small syntax fix

* chore: syntax fix

* syntax fix

* Chore: add back lockfile
This commit is contained in:
Uchechukwu Obasi 2021-11-10 11:01:06 +01:00 committed by GitHub
parent 2139a3dfa8
commit e7fd41d779
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 146 additions and 76 deletions

View File

@ -9,7 +9,7 @@ e2e.scenario({
scenario: () => { scenario: () => {
e2e.pages.Explore.visit(); e2e.pages.Explore.visit();
e2e.pages.Explore.General.container().should('have.length', 1); e2e.pages.Explore.General.container().should('have.length', 1);
e2e.components.RefreshPicker.runButton().should('have.length', 1); e2e.components.RefreshPicker.runButtonV2().should('have.length', 1);
e2e.components.DataSource.TestData.QueryTab.scenarioSelectContainer() e2e.components.DataSource.TestData.QueryTab.scenarioSelectContainer()
.should('be.visible') .should('be.visible')

View File

@ -19,7 +19,7 @@ describe('Trace view', () => {
e2e().wait(500); e2e().wait(500);
e2e.components.RefreshPicker.runButton().should('be.visible').click(); e2e.components.RefreshPicker.runButtonV2().should('be.visible').click();
e2e().wait('@longTrace'); e2e().wait('@longTrace');

View File

@ -137,8 +137,16 @@ export const Components = {
active: () => '[class*="-activeTabStyle"]', active: () => '[class*="-activeTabStyle"]',
}, },
RefreshPicker: { RefreshPicker: {
/**
* @deprecated use runButtonV2 from Grafana 8.3 instead
*/
runButton: 'RefreshPicker run button', runButton: 'RefreshPicker run button',
/**
* @deprecated use intervalButtonV2 from Grafana 8.3 instead
*/
intervalButton: 'RefreshPicker interval button', intervalButton: 'RefreshPicker interval button',
runButtonV2: 'data-testid RefreshPicker run button',
intervalButtonV2: 'data-testid RefreshPicker interval button',
}, },
QueryTab: { QueryTab: {
content: 'Query editor tab content', content: 'Query editor tab content',

View File

@ -48,6 +48,7 @@
"clipboard": "2.0.4", "clipboard": "2.0.4",
"core-js": "3.10.0", "core-js": "3.10.0",
"d3": "5.15.0", "d3": "5.15.0",
"date-fns": "2.25.0",
"emotion": "11.0.0", "emotion": "11.0.0",
"hoist-non-react-statics": "3.3.2", "hoist-non-react-statics": "3.3.2",
"immutable": "3.8.2", "immutable": "3.8.2",

View File

@ -7,6 +7,7 @@ import { css } from '@emotion/css';
import { useStyles2 } from '../../themes/ThemeContext'; import { useStyles2 } from '../../themes/ThemeContext';
import { Menu } from '../Menu/Menu'; import { Menu } from '../Menu/Menu';
import { MenuItem } from '../Menu/MenuItem'; import { MenuItem } from '../Menu/MenuItem';
import { FocusScope } from '@react-aria/focus';
export interface Props<T> extends HTMLAttributes<HTMLButtonElement> { export interface Props<T> extends HTMLAttributes<HTMLButtonElement> {
className?: string; className?: string;
@ -37,6 +38,13 @@ const ButtonSelectComponent = <T,>(props: Props<T>) => {
setIsOpen(!isOpen); setIsOpen(!isOpen);
}; };
const onArrowKeyDown = (event: React.KeyboardEvent) => {
event.stopPropagation();
if (event.key === 'ArrowDown' || event.key === 'Enter') {
setIsOpen(!isOpen);
}
};
const onChangeInternal = (item: SelectableValue<T>) => { const onChangeInternal = (item: SelectableValue<T>) => {
onChange(item); onChange(item);
setIsOpen(false); setIsOpen(false);
@ -48,6 +56,7 @@ const ButtonSelectComponent = <T,>(props: Props<T>) => {
className={className} className={className}
isOpen={isOpen} isOpen={isOpen}
onClick={onToggle} onClick={onToggle}
onKeyDown={onArrowKeyDown}
narrow={narrow} narrow={narrow}
variant={variant} variant={variant}
{...restProps} {...restProps}
@ -56,17 +65,22 @@ const ButtonSelectComponent = <T,>(props: Props<T>) => {
</ToolbarButton> </ToolbarButton>
{isOpen && ( {isOpen && (
<div className={styles.menuWrapper}> <div className={styles.menuWrapper}>
<ClickOutsideWrapper onClick={onCloseMenu} parent={document}> <ClickOutsideWrapper onClick={onCloseMenu} parent={document} includeButtonPress={false}>
<Menu> <FocusScope contain autoFocus restoreFocus>
{options.map((item) => ( <Menu onClose={onCloseMenu}>
<MenuItem {options.map((item) => (
key={`${item.value}`} <MenuItem
label={(item.label || item.value) as string} key={`${item.value}`}
onClick={() => onChangeInternal(item)} label={(item.label || item.value) as string}
active={item.value === value?.value} onClick={() => onChangeInternal(item)}
/> active={item.value === value?.value}
))} ariaChecked={item.value === value?.value}
</Menu> ariaLabel={item.ariaLabel || item.label}
role="menuitemradio"
/>
))}
</Menu>
</FocusScope>
</ClickOutsideWrapper> </ClickOutsideWrapper>
</div> </div>
)} )}

View File

@ -11,6 +11,7 @@ export interface MenuProps extends React.HTMLAttributes<HTMLDivElement> {
children: React.ReactNode; children: React.ReactNode;
ariaLabel?: string; ariaLabel?: string;
onOpen?: (focusOnItem: (itemId: number) => void) => void; onOpen?: (focusOnItem: (itemId: number) => void) => void;
onClose?: () => void;
onKeyDown?: React.KeyboardEventHandler; onKeyDown?: React.KeyboardEventHandler;
} }
@ -20,7 +21,7 @@ type MenuItemElement = HTMLAnchorElement & HTMLButtonElement;
/** @internal */ /** @internal */
export const Menu = React.forwardRef<HTMLDivElement, MenuProps>( export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(
({ header, children, ariaLabel, onOpen, onKeyDown, ...otherProps }, forwardedRef) => { ({ header, children, ariaLabel, onOpen, onClose, onKeyDown, ...otherProps }, forwardedRef) => {
const styles = useStyles2(getStyles); const styles = useStyles2(getStyles);
const [focusedItem, setFocusedItem] = useState(UNFOCUSED); const [focusedItem, setFocusedItem] = useState(UNFOCUSED);
@ -39,7 +40,7 @@ export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(
useEffectOnce(() => { useEffectOnce(() => {
const firstMenuItem = localRef?.current?.querySelector(`[data-role="menuitem"]`) as MenuItemElement | null; const firstMenuItem = localRef?.current?.querySelector(`[data-role="menuitem"]`) as MenuItemElement | null;
if (firstMenuItem) { if (firstMenuItem) {
firstMenuItem.tabIndex = 0; setFocusedItem(0);
} }
onOpen?.(setFocusedItem); onOpen?.(setFocusedItem);
}); });
@ -68,6 +69,14 @@ export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(
event.stopPropagation(); event.stopPropagation();
setFocusedItem(menuItemsCount - 1); setFocusedItem(menuItemsCount - 1);
break; break;
case 'Escape':
event.preventDefault();
event.stopPropagation();
onClose?.();
break;
case 'Tab':
onClose?.();
break;
default: default:
break; break;
} }

View File

@ -11,10 +11,14 @@ export interface MenuItemProps<T = any> {
label: string; label: string;
/** Aria label for accessibility support */ /** Aria label for accessibility support */
ariaLabel?: string; ariaLabel?: string;
/** Aria checked for accessibility support */
ariaChecked?: boolean;
/** Target of the menu item (i.e. new window) */ /** Target of the menu item (i.e. new window) */
target?: LinkTarget; target?: LinkTarget;
/** Icon of the menu item */ /** Icon of the menu item */
icon?: IconName; icon?: IconName;
/** Role of the menu item */
role?: string;
/** Url of the menu item */ /** Url of the menu item */
url?: string; url?: string;
/** Handler for the click behaviour */ /** Handler for the click behaviour */
@ -29,45 +33,57 @@ export interface MenuItemProps<T = any> {
/** @internal */ /** @internal */
export const MenuItem = React.memo( export const MenuItem = React.memo(
React.forwardRef<HTMLAnchorElement & HTMLButtonElement, MenuItemProps>( React.forwardRef<HTMLAnchorElement & HTMLButtonElement, MenuItemProps>((props, ref) => {
({ url, icon, label, ariaLabel, target, onClick, className, active, tabIndex = -1 }, ref) => { const {
const styles = useStyles2(getStyles); url,
const itemStyle = cx( icon,
{ label,
[styles.item]: true, ariaLabel,
[styles.activeItem]: active, ariaChecked,
}, target,
className onClick,
); className,
active,
role = 'menuitem',
tabIndex = -1,
} = props;
const styles = useStyles2(getStyles);
const itemStyle = cx(
{
[styles.item]: true,
[styles.activeItem]: active,
},
className
);
const Wrapper = url === undefined ? 'button' : 'a'; const Wrapper = url === undefined ? 'button' : 'a';
return ( return (
<Wrapper <Wrapper
target={target} target={target}
className={itemStyle} className={itemStyle}
rel={target === '_blank' ? 'noopener noreferrer' : undefined} rel={target === '_blank' ? 'noopener noreferrer' : undefined}
href={url} href={url}
onClick={ onClick={
onClick onClick
? (event) => { ? (event) => {
if (!(event.ctrlKey || event.metaKey || event.shiftKey) && onClick) { if (!(event.ctrlKey || event.metaKey || event.shiftKey) && onClick) {
event.preventDefault(); event.preventDefault();
onClick(event); onClick(event);
}
} }
: undefined }
} : undefined
role={url === undefined ? 'menuitem' : undefined} }
data-role="menuitem" // used to identify menuitem in Menu.tsx role={url === undefined ? role : undefined}
ref={ref} data-role="menuitem" // used to identify menuitem in Menu.tsx
aria-label={ariaLabel} ref={ref}
tabIndex={tabIndex} aria-label={ariaLabel}
> aria-checked={ariaChecked}
{icon && <Icon name={icon} className={styles.icon} aria-hidden />} {label} tabIndex={tabIndex}
</Wrapper> >
); {icon && <Icon name={icon} className={styles.icon} aria-hidden />} {label}
} </Wrapper>
) );
})
); );
MenuItem.displayName = 'MenuItem'; MenuItem.displayName = 'MenuItem';

View File

@ -7,17 +7,17 @@ describe('RefreshPicker', () => {
const result = intervalsToOptions(); const result = intervalsToOptions();
expect(result).toEqual([ expect(result).toEqual([
{ value: '', label: 'Off' }, { value: '', label: 'Off', ariaLabel: 'Turn off auto refresh' },
{ value: '5s', label: '5s' }, { value: '5s', label: '5s', ariaLabel: '5 seconds' },
{ value: '10s', label: '10s' }, { value: '10s', label: '10s', ariaLabel: '10 seconds' },
{ value: '30s', label: '30s' }, { value: '30s', label: '30s', ariaLabel: '30 seconds' },
{ value: '1m', label: '1m' }, { value: '1m', label: '1m', ariaLabel: '1 minute' },
{ value: '5m', label: '5m' }, { value: '5m', label: '5m', ariaLabel: '5 minutes' },
{ value: '15m', label: '15m' }, { value: '15m', label: '15m', ariaLabel: '15 minutes' },
{ value: '30m', label: '30m' }, { value: '30m', label: '30m', ariaLabel: '30 minutes' },
{ value: '1h', label: '1h' }, { value: '1h', label: '1h', ariaLabel: '1 hour' },
{ value: '2h', label: '2h' }, { value: '2h', label: '2h', ariaLabel: '2 hours' },
{ value: '1d', label: '1d' }, { value: '1d', label: '1d', ariaLabel: '1 day' },
]); ]);
}); });
}); });
@ -29,9 +29,9 @@ describe('RefreshPicker', () => {
const result = intervalsToOptions({ intervals }); const result = intervalsToOptions({ intervals });
expect(result).toEqual([ expect(result).toEqual([
{ value: '', label: 'Off' }, { value: '', label: 'Off', ariaLabel: 'Turn off auto refresh' },
{ value: '5s', label: '5s' }, { value: '5s', label: '5s', ariaLabel: '5 seconds' },
{ value: '10s', label: '10s' }, { value: '10s', label: '10s', ariaLabel: '10 seconds' },
]); ]);
}); });
}); });

View File

@ -1,11 +1,13 @@
import React, { PureComponent } from 'react'; import React, { PureComponent } from 'react';
import { SelectableValue } from '@grafana/data'; import formatDuration from 'date-fns/formatDuration';
import { SelectableValue, parseDuration } from '@grafana/data';
import { ButtonSelect } from '../Dropdown/ButtonSelect'; import { ButtonSelect } from '../Dropdown/ButtonSelect';
import { ButtonGroup, ToolbarButton, ToolbarButtonVariant } from '../Button'; import { ButtonGroup, ToolbarButton, ToolbarButtonVariant } from '../Button';
import { selectors } from '@grafana/e2e-selectors'; import { selectors } from '@grafana/e2e-selectors';
// Default intervals used in the refresh picker component // Default intervals used in the refresh picker component
export const defaultIntervals = ['5s', '10s', '30s', '1m', '5m', '15m', '30m', '1h', '2h', '1d']; export const defaultIntervals = ['5s', '10s', '30s', '1m', '5m', '15m', '30m', '1h', '2h', '1d'];
const offLabel = 'Auto refresh turned off. Choose refresh time interval';
export interface Props { export interface Props {
intervals?: string[]; intervals?: string[];
@ -22,8 +24,8 @@ export interface Props {
} }
export class RefreshPicker extends PureComponent<Props> { export class RefreshPicker extends PureComponent<Props> {
static offOption = { label: 'Off', value: '' }; static offOption = { label: 'Off', value: '', ariaLabel: 'Turn off auto refresh' };
static liveOption = { label: 'Live', value: 'LIVE' }; static liveOption = { label: 'Live', value: 'LIVE', ariaLabel: 'Turn on live streaming' };
static isLive = (refreshInterval?: string): boolean => refreshInterval === RefreshPicker.liveOption.value; static isLive = (refreshInterval?: string): boolean => refreshInterval === RefreshPicker.liveOption.value;
constructor(props: Props) { constructor(props: Props) {
@ -71,7 +73,7 @@ export class RefreshPicker extends PureComponent<Props> {
onClick={onRefresh} onClick={onRefresh}
variant={variant} variant={variant}
icon={isLoading ? 'fa fa-spinner' : 'sync'} icon={isLoading ? 'fa fa-spinner' : 'sync'}
aria-label={selectors.components.RefreshPicker.runButton} data-testid={selectors.components.RefreshPicker.runButtonV2}
> >
{text} {text}
</ToolbarButton> </ToolbarButton>
@ -81,7 +83,12 @@ export class RefreshPicker extends PureComponent<Props> {
options={options} options={options}
onChange={this.onChangeSelect as any} onChange={this.onChangeSelect as any}
variant={variant} variant={variant}
aria-label={selectors.components.RefreshPicker.intervalButton} data-testid={selectors.components.RefreshPicker.intervalButtonV2}
aria-label={
selectedValue.value === ''
? offLabel
: `Choose refresh time interval with current interval ${selectedValue.ariaLabel} selected`
}
/> />
)} )}
</ButtonGroup> </ButtonGroup>
@ -93,7 +100,21 @@ export function intervalsToOptions({ intervals = defaultIntervals }: { intervals
SelectableValue<string> SelectableValue<string>
> { > {
const intervalsOrDefault = intervals || defaultIntervals; const intervalsOrDefault = intervals || defaultIntervals;
const options = intervalsOrDefault.map((interval) => ({ label: interval, value: interval })); const options = intervalsOrDefault.map((interval) => {
const duration: { [key: string]: string | number } = parseDuration(interval);
const key = Object.keys(duration)[0];
const value = duration[key];
duration[key] = Number(value);
const ariaLabel = formatDuration(duration);
return {
label: interval,
value: interval,
ariaLabel: ariaLabel,
};
});
options.unshift(RefreshPicker.offOption); options.unshift(RefreshPicker.offOption);
return options; return options;

View File

@ -34,14 +34,14 @@ describe('DashNavTimeControls', () => {
const container = render( const container = render(
<DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" /> <DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" />
); );
expect(container.queryByLabelText(/RefreshPicker run button/i)).toBeInTheDocument(); expect(container.queryByLabelText(/Refresh dashboard/i)).toBeInTheDocument();
}); });
it('renders RefreshPicker with interval button in panel view', () => { it('renders RefreshPicker with interval button in panel view', () => {
const container = render( const container = render(
<DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" /> <DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" />
); );
expect(container.queryByLabelText(/RefreshPicker interval button/i)).toBeInTheDocument(); expect(container.queryByLabelText(/Choose refresh time interval/i)).toBeInTheDocument();
}); });
it('should not render RefreshPicker interval button in panel edit', () => { it('should not render RefreshPicker interval button in panel edit', () => {
@ -51,7 +51,7 @@ describe('DashNavTimeControls', () => {
const container = render( const container = render(
<DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" /> <DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" />
); );
expect(container.queryByLabelText(/RefreshPicker interval button/i)).not.toBeInTheDocument(); expect(container.queryByLabelText(/Choose refresh time interval/i)).not.toBeInTheDocument();
}); });
it('should render RefreshPicker run button in panel edit', () => { it('should render RefreshPicker run button in panel edit', () => {
@ -61,6 +61,6 @@ describe('DashNavTimeControls', () => {
const container = render( const container = render(
<DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" /> <DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" />
); );
expect(container.queryByLabelText(/RefreshPicker run button/i)).toBeInTheDocument(); expect(container.queryByLabelText(/Refresh dashboard/i)).toBeInTheDocument();
}); });
}); });

View File

@ -2766,6 +2766,7 @@ __metadata:
css-minimizer-webpack-plugin: ^3.1.1 css-minimizer-webpack-plugin: ^3.1.1
csstype: 3.0.9 csstype: 3.0.9
d3: 5.15.0 d3: 5.15.0
date-fns: 2.25.0
emotion: 11.0.0 emotion: 11.0.0
enzyme: 3.11.0 enzyme: 3.11.0
expose-loader: 3.0.0 expose-loader: 3.0.0