Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A11y: enable rule jsx-a11y/anchor-is-valid #56690

Merged
merged 32 commits into from
Oct 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
0bfd16b
Enable jsx-a11y/anchor-is-valid rule
eledobleefe Sep 27, 2022
a057ecf
Fix errors in Segment stories
eledobleefe Sep 27, 2022
7300cc3
Merge main in this branch
eledobleefe Sep 28, 2022
dae81d9
Change a element into button and fix styles in TracePageHeader component
eledobleefe Oct 11, 2022
10bf990
Change a element into button and modify its styles in SpanBarRow comp…
eledobleefe Oct 11, 2022
f8ba930
Change a elements into buttons and modify its styles in DashboardRow
eledobleefe Oct 11, 2022
8c0360d
Change a element into span
eledobleefe Oct 11, 2022
8ba8981
Change a element into button and modify types according to that in Tu…
eledobleefe Oct 11, 2022
54bff1d
Change a element into button and modify styles in LegendSeriesItem
eledobleefe Oct 11, 2022
27396a2
Change a element into button and modify styles in PromQueryFields com…
eledobleefe Oct 13, 2022
f4c4d13
Change a element into button and modify styles in VariableOptions
eledobleefe Oct 13, 2022
157bce0
Change a element into button and modify styles in LogsQueryField
eledobleefe Oct 13, 2022
db79a0f
Modify TutorialCard component according to reviewer's comments
eledobleefe Oct 14, 2022
0b35cba
Add type='button'
eledobleefe Oct 14, 2022
fd7e431
Add type='button' and use classNames
eledobleefe Oct 14, 2022
3291359
Merge main in this branch
eledobleefe Oct 14, 2022
dc4b8f5
Change a into button and modify styles in TagSection component
eledobleefe Oct 14, 2022
f2228bf
Change a element into button and modify styles in FilterSection compo…
eledobleefe Oct 14, 2022
a87b3dd
Create a function to clear the button styles and export it
eledobleefe Oct 14, 2022
c73d48e
Use clearButtonStyles function
eledobleefe Oct 14, 2022
e7f46eb
Fix button styles
eledobleefe Oct 17, 2022
e5db26e
Add theme to VariableOptions component and fix import in OptionsPicker
eledobleefe Oct 17, 2022
2d7665f
Add theme to PromQueryField component
eledobleefe Oct 17, 2022
27aeca5
Add theme to LogsQueryField component and fix import in its test and …
eledobleefe Oct 17, 2022
5f3d379
Merge main in this branch
eledobleefe Oct 17, 2022
1a09833
Merge branch 'main' into eledobleefe/jsx-a11y-55834
eledobleefe Oct 18, 2022
809a337
Delete the corresponding line of the rule
eledobleefe Oct 18, 2022
b667f2d
Use cx to override clearButtonStyles with text-link and muted classes
eledobleefe Oct 18, 2022
9e32007
Make changes suggested by reviewer
eledobleefe Oct 18, 2022
bbb07a8
Merge branch 'main' into eledobleefe/jsx-a11y-55834
eledobleefe Oct 19, 2022
22f6d2c
Use cx to manage which style should be overridden
eledobleefe Oct 20, 2022
cc45efc
Merge branch 'main' into eledobleefe/jsx-a11y-55834
eledobleefe Oct 20, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@
// rules marked "off" are those left in the recommended preset we need to fix
// we should remove the corresponding line and fix them one by one
// any marked "error" contain specific overrides we'll need to keep
"jsx-a11y/anchor-is-valid": "off",
"jsx-a11y/click-events-have-key-events": "off",
"jsx-a11y/interactive-supports-focus": "off",
"jsx-a11y/label-has-associated-control": "off",
Expand Down
9 changes: 9 additions & 0 deletions packages/grafana-ui/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -302,3 +302,12 @@ export function getPropertiesForVariant(theme: GrafanaTheme2, variant: ButtonVar
return getButtonVariantStyles(theme, theme.colors.primary, fill);
}
}

export const clearButtonStyles = (theme: GrafanaTheme2) => {
return css`
background: transparent;
color: ${theme.colors.text.primary};
border: none;
padding: 0;
`;
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import { Segment, Icon, SegmentSection } from '@grafana/ui';
import { SegmentSyncProps } from './Segment';

const AddButton = (
<a className="gf-form-label query-part">
<span className="gf-form-label query-part">
<Icon name="plus-circle" />
</a>
</span>
);

const toOption = (value: any) => ({ label: value, value: value });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import { SegmentAsync, Icon, SegmentSection } from '@grafana/ui';
import { SegmentAsyncProps } from './SegmentAsync';

const AddButton = (
<a className="gf-form-label query-part">
<span className="gf-form-label query-part">
<Icon name="plus" />
</a>
</span>
);

const toOption = (value: any) => ({ label: value, value: value });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,15 @@ export const InputWithAutoFocus = () => {
{inputComponents.map((InputComponent: any, i: number) => (
<InputComponent initialValue="test" key={i} />
))}
<a
<button
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd be careful when changing anything to button as it has submit type by default. Normally its not a big deal, but when used in forms it will trigger a default submit behavior. Therefore I think we'd add type='button' to all the buttons by default (our Button component already has that).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comment @Clarity-89 😊 I will add type='button'.

type="button"
className="gf-form-label query-part"
onClick={() => {
setInputComponents([...inputComponents, InputComponent]);
}}
>
<Icon name="plus" />
</a>
</button>
</SegmentFrame>
);
};
Expand Down
2 changes: 1 addition & 1 deletion packages/grafana-ui/src/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export { RangeSlider } from './Slider/RangeSlider';
export { Form } from './Forms/Form';
export { sharedInputStyle } from './Forms/commonStyles';
export { InputControl } from './InputControl';
export { Button, LinkButton, type ButtonVariant, ButtonGroup, type ButtonProps } from './Button';
export { Button, LinkButton, type ButtonVariant, ButtonGroup, type ButtonProps, clearButtonStyles } from './Button';
export { ToolbarButton, ToolbarButtonRow } from './ToolbarButton';
export { ValuePicker } from './ValuePicker/ValuePicker';
export { fieldMatchersUI } from './MatchersUI/fieldMatchersUI';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ const getStyles = (theme: GrafanaTheme2) => {
&:hover small {
text-decoration: none;
}
/* Adapt styles when changing from a element into button */
background: transparent;
text-align: left;
border: none;
`,
TracePageHeaderDetailToggle: css`
label: TracePageHeaderDetailToggle;
Expand Down Expand Up @@ -236,7 +240,8 @@ export default function TracePageHeader(props: TracePageHeaderEmbedProps) {
<div className={styles.TracePageHeaderTitleRow}>
{links && links.length > 0 && <ExternalLinks links={links} className={styles.TracePageHeaderBack} />}
{canCollapse ? (
<a
<button
type="button"
className={styles.TracePageHeaderTitleLink}
onClick={onSlimViewClicked}
role="switch"
Expand All @@ -249,7 +254,7 @@ export default function TracePageHeader(props: TracePageHeaderEmbedProps) {
)}
/>
{title}
</a>
</button>
) : (
title
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,9 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => {
&:hover > small {
color: ${autoColor(theme, '#000')};
}
text-align: left;
background: transparent;
border: none;
`,
nameDetailExpanded: css`
label: nameDetailExpanded;
Expand Down Expand Up @@ -437,7 +440,8 @@ export class UnthemedSpanBarRow extends React.PureComponent<SpanBarRowProps> {
addHoverIndentGuideId={addHoverIndentGuideId}
removeHoverIndentGuideId={removeHoverIndentGuideId}
/>
<a
<button
type="button"
className={cx(styles.name, { [styles.nameDetailExpanded]: isDetailExpanded })}
aria-checked={isDetailExpanded}
title={labelDetail}
Expand Down Expand Up @@ -478,7 +482,7 @@ export class UnthemedSpanBarRow extends React.PureComponent<SpanBarRowProps> {
</span>
<small className={styles.endpointName}>{rpc ? rpc.operationName : operationName}</small>
<small className={styles.endpointName}> {this.getSpanBarLabel(span, spanBarOptions, label)}</small>
</a>
</button>
{createSpanLink &&
(() => {
const links = createSpanLink(span);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ export class DashboardRow extends React.Component<DashboardRowProps, any> {

return (
<div className={classes} data-testid="dashboard-row-container">
<a
<button
className="dashboard-row__title pointer"
type="button"
data-testid={selectors.components.DashboardRow.title(title)}
onClick={this.onToggle}
>
Expand All @@ -86,17 +87,17 @@ export class DashboardRow extends React.Component<DashboardRowProps, any> {
<span className="dashboard-row__panel_count">
({count} {panels})
</span>
</a>
</button>
{canEdit && (
<div className="dashboard-row__actions">
<RowOptionsButton
title={this.props.panel.title}
repeat={this.props.panel.repeat}
onUpdate={this.onUpdate}
/>
<a className="pointer" onClick={this.onDelete} role="button" aria-label="Delete row">
<button type="button" className="pointer" onClick={this.onDelete} aria-label="Delete row">
<Icon name="trash-alt" />
</a>
</button>
</div>
)}
{this.props.panel.collapsed === true && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@ export const RowOptionsButton: FC<RowOptionsButtonProps> = ({ repeat, title, onU
<ModalsController>
{({ showModal, hideModal }) => {
return (
<a
<button
type="button"
className="pointer"
role="button"
aria-label="Row options"
onClick={() => {
showModal(RowOptionsModal, { title, repeat, onDismiss: hideModal, onUpdate: onUpdateChange(hideModal) });
}}
>
<Icon name="cog" />
</a>
</button>
);
}}
</ModalsController>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ export const REMOVE_FILTER_KEY = '-- remove filter --';
const REMOVE_VALUE = { label: REMOVE_FILTER_KEY, value: REMOVE_FILTER_KEY };

const plusSegment: ReactElement = (
<a className="gf-form-label query-part" aria-label="Add Filter">
<span className="gf-form-label query-part" aria-label="Add Filter">
<Icon name="plus" />
</a>
</span>
);

const fetchFilterKeys = async (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { VariableOption, VariableWithMultiSupport, VariableWithOptions } from '.
import { toKeyedVariableIdentifier } from '../../utils';
import { VariableInput } from '../shared/VariableInput';
import { VariableLink } from '../shared/VariableLink';
import { VariableOptions } from '../shared/VariableOptions';
import VariableOptions from '../shared/VariableOptions';
import { NavigationKey, VariablePickerProps } from '../types';

import { commitChangesToVariable, filterOrSearchOptions, navigateOptions, openOptions } from './actions';
Expand Down
40 changes: 27 additions & 13 deletions public/app/features/variables/pickers/shared/VariableOptions.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { css, cx } from '@emotion/css';
import classNames from 'classnames';
import React, { PureComponent } from 'react';

import { selectors } from '@grafana/e2e-selectors';
import { Tooltip } from '@grafana/ui';
import { Tooltip, Themeable2, withTheme2, clearButtonStyles } from '@grafana/ui';

import { VariableOption } from '../../types';

export interface Props extends React.HTMLProps<HTMLUListElement> {
export interface Props extends React.HTMLProps<HTMLUListElement>, Themeable2 {
multi: boolean;
values: VariableOption[];
selectedValues: VariableOption[];
Expand All @@ -19,19 +20,19 @@ export interface Props extends React.HTMLProps<HTMLUListElement> {
id: string;
}

export class VariableOptions extends PureComponent<Props> {
onToggle = (option: VariableOption) => (event: React.MouseEvent<HTMLAnchorElement>) => {
class VariableOptions extends PureComponent<Props> {
onToggle = (option: VariableOption) => (event: React.MouseEvent<HTMLButtonElement>) => {
const clearOthers = event.shiftKey || event.ctrlKey || event.metaKey;
this.handleEvent(event);
this.props.onToggle(option, clearOthers);
};

onToggleAll = (event: React.MouseEvent<HTMLAnchorElement>) => {
onToggleAll = (event: React.MouseEvent<HTMLButtonElement>) => {
this.handleEvent(event);
this.props.onToggleAll();
};

handleEvent(event: React.MouseEvent<HTMLAnchorElement>) {
handleEvent(event: React.MouseEvent<HTMLButtonElement>) {
event.preventDefault();
event.stopPropagation();
}
Expand All @@ -57,37 +58,43 @@ export class VariableOptions extends PureComponent<Props> {
}

renderOption(option: VariableOption, index: number) {
const { highlightIndex } = this.props;
const { highlightIndex, theme } = this.props;
const selectClass = option.selected ? 'variable-option pointer selected' : 'variable-option pointer';
const highlightClass = index === highlightIndex ? `${selectClass} highlighted` : selectClass;

return (
<li key={`${option.value}`}>
<a role="checkbox" aria-checked={option.selected} className={highlightClass} onClick={this.onToggle(option)}>
<button
role="checkbox"
type="button"
aria-checked={option.selected}
className={classNames(highlightClass, clearButtonStyles(theme), noStyledButton)}
onClick={this.onToggle(option)}
>
<span className="variable-option-icon"></span>
<span data-testid={selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownOptionTexts(`${option.text}`)}>
{option.text}
</span>
</a>
</button>
</li>
);
}

renderMultiToggle() {
const { multi, selectedValues } = this.props;
const { multi, selectedValues, theme } = this.props;

if (!multi) {
return null;
}

return (
<Tooltip content={'Clear selections'} placement={'top'}>
<a
<button
className={`${
selectedValues.length > 1
? 'variable-options-column-header many-selected'
: 'variable-options-column-header'
}`}
} ${noStyledButton} ${clearButtonStyles(theme)}`}
role="checkbox"
aria-checked={selectedValues.length > 1 ? 'mixed' : 'false'}
onClick={this.onToggleAll}
Expand All @@ -96,7 +103,7 @@ export class VariableOptions extends PureComponent<Props> {
>
<span className="variable-option-icon"></span>
Selected ({selectedValues.length})
</a>
</button>
</Tooltip>
);
}
Expand All @@ -108,3 +115,10 @@ const listStyles = cx(
list-style-type: none;
`
);

const noStyledButton = css`
width: 100%;
text-align: left;
`;

export default withTheme2(VariableOptions);
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { CloudWatchDatasource } from '../datasource';
import { CloudWatchJsonData, CloudWatchLogsQuery, CloudWatchQuery } from '../types';

import CloudWatchLink from './CloudWatchLink';
import { CloudWatchLogsQueryField } from './LogsQueryField';
import CloudWatchLogsQueryField from './LogsQueryField';

type Props = QueryEditorProps<CloudWatchDatasource, CloudWatchQuery, CloudWatchJsonData> & {
query: CloudWatchLogsQuery;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { act } from 'react-dom/test-utils';
import { ExploreId } from '../../../../types';
import { setupMockedDataSource } from '../__mocks__/CloudWatchDataSource';

import { CloudWatchLogsQueryField } from './LogsQueryField';
import CloudWatchLogsQueryField from './LogsQueryField';

jest
.spyOn(_, 'debounce')
Expand Down