Template variables: Keyboard navigation improvements (#38001)

* Fix variable labels

* Add proper labeling for input

* Add ids to PickerRenderer

* Fix tests

* Update PR feedback

* OptionsPicker: Change to id

* Inherit aria attributes

* Add checkbox role

* Fix typo

* Add proper label reference

* Update role and label

* Prevent spreadng non-DOM attributes

* Move form layout to other component

* Remove haspopup

* Add testid to selector

* Add HTMLProps extension

* Use list

* Move styles outside of class

* Add cx
This commit is contained in:
Tobias Skarhed 2021-08-19 16:28:25 +02:00 committed by GitHub
parent 28cf93e42c
commit 1f091c448f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 110 additions and 39 deletions

View File

@ -47,13 +47,13 @@ export const Pages = {
},
SubMenu: {
submenu: 'Dashboard submenu',
submenuItem: 'Dashboard template variables submenu item',
submenuItemLabels: (item: string) => `Dashboard template variables submenu Label ${item}`,
submenuItem: 'data-testid template variable',
submenuItemLabels: (item: string) => `data-testid Dashboard template variables submenu Label ${item}`,
submenuItemValueDropDownValueLinkTexts: (item: string) =>
`Dashboard template variables Variable Value DropDown value link text ${item}`,
submenuItemValueDropDownDropDown: 'Dashboard template variables Variable Value DropDown DropDown',
`data-testid Dashboard template variables Variable Value DropDown value link text ${item}`,
submenuItemValueDropDownDropDown: 'Variable options',
submenuItemValueDropDownOptionTexts: (item: string) =>
`Dashboard template variables Variable Value DropDown option text ${item}`,
`data-testid Dashboard template variables Variable Value DropDown option text ${item}`,
},
Settings: {
General: {

View File

@ -44,15 +44,15 @@ export const DashboardLinks: FC<Props> = ({ dashboard, links }) => {
href={sanitizeUrl(linkInfo.href)}
target={link.targetBlank ? '_blank' : undefined}
rel="noreferrer"
aria-label={selectors.components.DashboardLinks.link}
data-testid={selectors.components.DashboardLinks.link}
>
<Icon name={linkIconMap[link.icon] as IconName} style={{ marginRight: '4px' }} />
<Icon aria-hidden name={linkIconMap[link.icon] as IconName} style={{ marginRight: '4px' }} />
<span>{linkInfo.title}</span>
</a>
);
return (
<div key={key} className="gf-form" aria-label={selectors.components.DashboardLinks.container}>
<div key={key} className="gf-form" data-testid={selectors.components.DashboardLinks.container}>
{link.tooltip ? <Tooltip content={linkInfo.tooltip}>{linkElement}</Tooltip> : linkElement}
</div>
);

View File

@ -9,6 +9,7 @@ import { Annotations } from './Annotations';
import { SubMenuItems } from './SubMenuItems';
import { DashboardLink } from '../../state/DashboardModel';
import { AnnotationQuery } from '@grafana/data';
import { css } from '@emotion/css';
interface OwnProps {
dashboard: DashboardModel;
@ -47,7 +48,9 @@ class SubMenuUnConnected extends PureComponent<Props> {
return (
<div className="submenu-controls">
<SubMenuItems variables={variables} />
<form aria-label="Template variables" className={styles}>
<SubMenuItems variables={variables} />
</form>
<Annotations
annotations={annotations}
onAnnotationChanged={this.onAnnotationStateChanged}
@ -67,6 +70,12 @@ const mapStateToProps: MapStateToProps<ConnectedProps, OwnProps, StoreState> = (
};
};
const styles = css`
display: flex;
flex-wrap: wrap;
display: contents;
`;
export const SubMenu = connect(mapStateToProps)(SubMenuUnConnected);
SubMenu.displayName = 'SubMenu';

View File

@ -9,6 +9,7 @@ interface Props {
export const SubMenuItems: FunctionComponent<Props> = ({ variables }) => {
const [visibleVariables, setVisibleVariables] = useState<VariableModel[]>([]);
useEffect(() => {
setVisibleVariables(variables.filter((state) => state.hide !== VariableHide.hideVariable));
}, [variables]);
@ -24,7 +25,7 @@ export const SubMenuItems: FunctionComponent<Props> = ({ variables }) => {
<div
key={variable.id}
className="submenu-item gf-form-inline"
aria-label={selectors.pages.Dashboard.SubMenu.submenuItem}
data-testid={selectors.pages.Dashboard.SubMenu.submenuItem}
>
<PickerRenderer variable={variable} />
</div>

View File

@ -56,11 +56,11 @@ function setupTestContext({ pickerState = {}, variable = {} }: Args = {}) {
}
function getSubMenu(text: string) {
return screen.getByLabelText(selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownValueLinkTexts(text));
return screen.getByTestId(selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownValueLinkTexts(text));
}
function getOption(text: string) {
return screen.getByLabelText(selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownOptionTexts('A'));
return screen.getByTestId(selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownOptionTexts('A'));
}
describe('OptionPicker', () => {

View File

@ -75,7 +75,15 @@ export const optionPickerFactory = <Model extends VariableWithOptions | Variable
const linkText = formatVariableLabel(variable);
const loading = variable.state === LoadingState.Loading;
return <VariableLink text={linkText} onClick={this.onShowOptions} loading={loading} onCancel={this.onCancel} />;
return (
<VariableLink
id={variable.id}
text={linkText}
onClick={this.onShowOptions}
loading={loading}
onCancel={this.onCancel}
/>
);
}
onCancel = () => {
@ -83,12 +91,16 @@ export const optionPickerFactory = <Model extends VariableWithOptions | Variable
};
renderOptions(picker: OptionsPickerState) {
const { id } = this.props.variable;
return (
<ClickOutsideWrapper onClick={this.onHideOptions}>
<VariableInput
id={id}
value={picker.queryValue}
onChange={this.props.filterOrSearchOptions}
onNavigate={this.props.navigateOptions}
aria-expanded={true}
aria-controls={`options-${id}`}
/>
<VariableOptions
values={picker.options}
@ -97,6 +109,7 @@ export const optionPickerFactory = <Model extends VariableWithOptions | Variable
highlightIndex={picker.highlightIndex}
multi={picker.multi}
selectedValues={picker.selectedValues}
id={`options-${id}`}
/>
</ClickOutsideWrapper>
);

View File

@ -37,7 +37,8 @@ function PickerLabel({ variable }: PropsWithChildren<Props>): ReactElement | nul
<Tooltip content={variable.description} placement={'bottom'}>
<label
className="gf-form-label gf-form-label--variable"
aria-label={selectors.pages.Dashboard.SubMenu.submenuItemLabels(labelOrName)}
data-testid={selectors.pages.Dashboard.SubMenu.submenuItemLabels(labelOrName)}
htmlFor={variable.id}
>
{labelOrName}
</label>
@ -48,7 +49,8 @@ function PickerLabel({ variable }: PropsWithChildren<Props>): ReactElement | nul
return (
<label
className="gf-form-label gf-form-label--variable"
aria-label={selectors.pages.Dashboard.SubMenu.submenuItemLabels(labelOrName)}
data-testid={selectors.pages.Dashboard.SubMenu.submenuItemLabels(labelOrName)}
htmlFor={variable.id}
>
{labelOrName}
</label>

View File

@ -1,7 +1,7 @@
import React, { PureComponent } from 'react';
import { NavigationKey } from '../types';
export interface Props {
export interface Props extends Omit<React.HTMLProps<HTMLInputElement>, 'onChange' | 'value'> {
onChange: (value: string) => void;
onNavigate: (key: NavigationKey, clearOthers: boolean) => void;
value: string | null;
@ -21,8 +21,10 @@ export class VariableInput extends PureComponent<Props> {
};
render() {
const { value, id, onNavigate, ...restProps } = this.props;
return (
<input
{...restProps}
ref={(instance) => {
if (instance) {
instance.focus();
@ -31,9 +33,10 @@ export class VariableInput extends PureComponent<Props> {
}}
type="text"
className="gf-form-input"
value={this.props.value ?? ''}
value={value ?? ''}
onChange={this.onChange}
onKeyDown={this.onKeyDown}
placeholder="Enter variable value"
/>
);
}

View File

@ -9,12 +9,16 @@ interface Props {
text: string;
loading: boolean;
onCancel: () => void;
/**
* htmlFor, needed for the label
*/
id: string;
}
export const VariableLink: FC<Props> = ({ loading, onClick: propsOnClick, text, onCancel }) => {
export const VariableLink: FC<Props> = ({ loading, onClick: propsOnClick, text, onCancel, id }) => {
const styles = useStyles(getStyles);
const onClick = useCallback(
(event: MouseEvent<HTMLAnchorElement>) => {
(event: MouseEvent<HTMLButtonElement>) => {
event.stopPropagation();
event.preventDefault();
propsOnClick();
@ -26,8 +30,9 @@ export const VariableLink: FC<Props> = ({ loading, onClick: propsOnClick, text,
return (
<div
className={styles.container}
aria-label={selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownValueLinkTexts(`${text}`)}
data-testid={selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownValueLinkTexts(`${text}`)}
title={text}
id={id}
>
<VariableLinkText text={text} />
<LoadingIndicator onCancel={onCancel} />
@ -36,15 +41,18 @@ export const VariableLink: FC<Props> = ({ loading, onClick: propsOnClick, text,
}
return (
<a
<button
onClick={onClick}
className={styles.container}
aria-label={selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownValueLinkTexts(`${text}`)}
data-testid={selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownValueLinkTexts(`${text}`)}
aria-expanded={false}
aria-controls={`options-${id}`}
id={id}
title={text}
>
<VariableLinkText text={text} />
<Icon name="angle-down" size="sm" />
</a>
<Icon aria-hidden name="angle-down" size="sm" />
</button>
);
};

View File

@ -3,14 +3,19 @@ import { Tooltip } from '@grafana/ui';
import { selectors } from '@grafana/e2e-selectors';
import { VariableOption } from '../../types';
import { css, cx } from '@emotion/css';
export interface Props {
export interface Props extends React.HTMLProps<HTMLUListElement> {
multi: boolean;
values: VariableOption[];
selectedValues: VariableOption[];
highlightIndex: number;
onToggle: (option: VariableOption, clearOthers: boolean) => void;
onToggleAll: () => void;
/**
* Used for aria-controls
*/
id: string;
}
export class VariableOptions extends PureComponent<Props> {
@ -31,18 +36,20 @@ export class VariableOptions extends PureComponent<Props> {
}
render() {
const { multi, values } = this.props;
// Don't want to pass faulty rest props to the div
const { multi, values, highlightIndex, selectedValues, onToggle, onToggleAll, ...restProps } = this.props;
return (
<div
className={`${multi ? 'variable-value-dropdown multi' : 'variable-value-dropdown single'}`}
aria-label={selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownDropDown}
>
<div className={`${multi ? 'variable-value-dropdown multi' : 'variable-value-dropdown single'}`}>
<div className="variable-options-wrapper">
<div className="variable-options-column">
<ul
className={listStyles}
aria-label={selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownDropDown}
{...restProps}
>
{this.renderMultiToggle()}
{values.map((option, index) => this.renderOption(option, index))}
</div>
</ul>
</div>
</div>
);
@ -54,12 +61,20 @@ export class VariableOptions extends PureComponent<Props> {
const highlightClass = index === highlightIndex ? `${selectClass} highlighted` : selectClass;
return (
<a key={`${option.value}`} className={highlightClass} onClick={this.onToggle(option)}>
<span className="variable-option-icon"></span>
<span aria-label={selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownOptionTexts(`${option.text}`)}>
{option.text}
</span>
</a>
<li>
<a
key={`${option.value}`}
role="checkbox"
aria-checked={option.selected}
className={highlightClass}
onClick={this.onToggle(option)}
>
<span className="variable-option-icon"></span>
<span data-testid={selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownOptionTexts(`${option.text}`)}>
{option.text}
</span>
</a>
</li>
);
}
@ -78,7 +93,10 @@ export class VariableOptions extends PureComponent<Props> {
? 'variable-options-column-header many-selected'
: 'variable-options-column-header'
}`}
role="checkbox"
aria-checked={selectedValues.length > 1 ? 'mixed' : 'false'}
onClick={this.onToggleAll}
aria-label="Toggle all values"
data-placement="top"
>
<span className="variable-option-icon"></span>
@ -88,3 +106,10 @@ export class VariableOptions extends PureComponent<Props> {
);
}
}
const listStyles = cx(
'variable-options-column',
css`
list-style-type: none;
`
);

View File

@ -50,5 +50,15 @@ export function TextBoxVariablePicker({ variable, onVariableChange }: Props): Re
}
};
return <Input type="text" value={updatedValue} onChange={onChange} onBlur={onBlur} onKeyDown={onKeyDown} />;
return (
<Input
type="text"
value={updatedValue}
onChange={onChange}
onBlur={onBlur}
onKeyDown={onKeyDown}
placeholder="Enter variable value"
id={variable.id}
/>
);
}