Accessibility: Enable rule jsx-a11y/no-noninteractive-element-interactions (#58077)

* fixes for no-noninteractive-element-interactions * remaining fixes * add type="button" * fix unit tests
2024-11-25 10:20:29 -06:00 · 2022-11-03 10:55:58 +00:00 · 2022-11-03 10:55:58 +00:00 · 514d3111f4
commit 514d3111f4
parent 65bd5c65d8
16 changed files with 79 additions and 65 deletions
--- a/.eslintrc
+++ b/.eslintrc
@ -81,7 +81,6 @@
            "ignoreNonDOM": true
          }
        ],
-        "jsx-a11y/no-noninteractive-element-interactions": "off",
        "jsx-a11y/no-static-element-interactions": "off"
      }
    }
--- a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeOption.tsx
+++ b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeOption.tsx
@ -14,12 +14,6 @@ const getStyles = (theme: GrafanaTheme2) => {
      align-items: center;
      flex-direction: row-reverse;
      justify-content: space-between;
-      padding: 7px 9px 7px 9px;
-
-      &:hover {
-        background: ${theme.colors.action.hover};
-        cursor: pointer;
-      }
    `,
    selected: css`
      background: ${theme.colors.action.selected};
@ -27,6 +21,7 @@ const getStyles = (theme: GrafanaTheme2) => {
    `,
    radio: css`
      opacity: 0;
+      width: 0 !important;

      &:focus-visible + label {
        ${getFocusStyles(theme)};
@ -34,6 +29,13 @@ const getStyles = (theme: GrafanaTheme2) => {
    `,
    label: css`
      cursor: pointer;
+      flex: 1;
+      padding: 7px 9px 7px 9px;
+
+      &:hover {
+        background: ${theme.colors.action.hover};
+        cursor: pointer;
+      }
    `,
  };
 };
@ -54,7 +56,7 @@ export const TimeRangeOption = memo<Props>(({ value, onSelect, selected = false,
  const id = uuidv4();

  return (
-    <li onClick={() => onSelect(value)} className={cx(styles.container, selected && styles.selected)}>
+    <li className={cx(styles.container, selected && styles.selected)}>
      <input
        className={styles.radio}
        checked={selected}
--- a/packages/grafana-ui/src/components/Typeahead/Typeahead.test.tsx
+++ b/packages/grafana-ui/src/components/Typeahead/Typeahead.test.tsx
@ -25,10 +25,12 @@ describe('Typeahead', () => {
      render(<Typeahead origin="test" groupedItems={completionItemGroups} isOpen />);
      expect(screen.getByTestId('typeahead')).toBeInTheDocument();

-      const items = screen.getAllByRole('listitem');
-      expect(items).toHaveLength(2);
-      expect(items[0]).toHaveTextContent('my group');
-      expect(items[1]).toHaveTextContent('first item');
+      const groupTitles = screen.getAllByRole('listitem');
+      expect(groupTitles).toHaveLength(1);
+      expect(groupTitles[0]).toHaveTextContent('my group');
+      const items = screen.getAllByRole('menuitem');
+      expect(items).toHaveLength(1);
+      expect(items[0]).toHaveTextContent('first item');
    });

    it('can be rendered properly even if the size of items is large', () => {
--- a/packages/grafana-ui/src/components/Typeahead/Typeahead.tsx
+++ b/packages/grafana-ui/src/components/Typeahead/Typeahead.tsx
@ -162,7 +162,7 @@ export class Typeahead extends PureComponent<Props, State> {

    return (
      <Portal origin={origin} isOpen={isOpen} style={this.menuPosition}>
-        <ul className="typeahead" data-testid="typeahead">
+        <ul role="menu" className="typeahead" data-testid="typeahead">
          <FixedSizeList
            ref={this.listRef}
            itemCount={allItems.length}
--- a/packages/grafana-ui/src/components/Typeahead/TypeaheadItem.tsx
+++ b/packages/grafana-ui/src/components/Typeahead/TypeaheadItem.tsx
@ -22,6 +22,9 @@ interface Props {

 const getStyles = (theme: GrafanaTheme2) => ({
  typeaheadItem: css`
+    border: none;
+    background: none;
+    text-align: left;
    label: type-ahead-item;
    height: auto;
    font-family: ${theme.typography.fontFamilyMonospace};
@ -77,27 +80,31 @@ export const TypeaheadItem = (props: Props) => {
  }

  return (
-    <li
-      className={className}
-      style={style}
-      onMouseDown={onClickItem}
-      onMouseEnter={onMouseEnter}
-      onMouseLeave={onMouseLeave}
-    >
-      {item.highlightParts !== undefined ? (
-        <PartialHighlighter
-          text={label}
-          highlightClassName={highlightClassName}
-          highlightParts={item.highlightParts}
-        ></PartialHighlighter>
-      ) : (
-        <Highlighter
-          textToHighlight={label}
-          searchWords={[prefix ?? '']}
-          autoEscape={true}
-          highlightClassName={highlightClassName}
-        />
-      )}
+    <li role="none">
+      <button
+        role="menuitem"
+        className={className}
+        style={style}
+        onMouseDown={onClickItem}
+        onMouseEnter={onMouseEnter}
+        onMouseLeave={onMouseLeave}
+        type="button"
+      >
+        {item.highlightParts !== undefined ? (
+          <PartialHighlighter
+            text={label}
+            highlightClassName={highlightClassName}
+            highlightParts={item.highlightParts}
+          ></PartialHighlighter>
+        ) : (
+          <Highlighter
+            textToHighlight={label}
+            searchWords={[prefix ?? '']}
+            autoEscape={true}
+            highlightClassName={highlightClassName}
+          />
+        )}
+      </button>
    </li>
  );
 };
--- a/public/app/features/dashboard/components/AnnotationSettings/AnnotationSettingsList.tsx
+++ b/public/app/features/dashboard/components/AnnotationSettings/AnnotationSettingsList.tsx
@ -57,7 +57,7 @@ export const AnnotationSettingsList = ({ dashboard, onNew, onEdit }: Props) => {
  return (
    <VerticalGroup>
      {annotations.length > 0 && (
-        <table className="filter-table filter-table--hover">
+        <table role="grid" className="filter-table filter-table--hover">
          <thead>
            <tr>
              <th>Query name</th>
@ -69,26 +69,26 @@ export const AnnotationSettingsList = ({ dashboard, onNew, onEdit }: Props) => {
            {dashboard.annotations.list.map((annotation, idx) => (
              <tr key={`${annotation.name}-${idx}`}>
                {annotation.builtIn ? (
-                  <td style={{ width: '90%' }} className="pointer" onClick={() => onEdit(idx)}>
+                  <td role="gridcell" style={{ width: '90%' }} className="pointer" onClick={() => onEdit(idx)}>
                    {getAnnotationName(annotation)}
                  </td>
                ) : (
-                  <td className="pointer" onClick={() => onEdit(idx)}>
+                  <td role="gridcell" className="pointer" onClick={() => onEdit(idx)}>
                    {getAnnotationName(annotation)}
                  </td>
                )}
-                <td className="pointer" onClick={() => onEdit(idx)}>
+                <td role="gridcell" className="pointer" onClick={() => onEdit(idx)}>
                  {dataSourceSrv.getInstanceSettings(annotation.datasource)?.name || annotation.datasource?.uid}
                </td>
-                <td style={{ width: '1%' }}>
+                <td role="gridcell" style={{ width: '1%' }}>
                  {idx !== 0 && <IconButton name="arrow-up" aria-label="arrow-up" onClick={() => onMove(idx, -1)} />}
                </td>
-                <td style={{ width: '1%' }}>
+                <td role="gridcell" style={{ width: '1%' }}>
                  {dashboard.annotations.list.length > 1 && idx !== dashboard.annotations.list.length - 1 ? (
                    <IconButton name="arrow-down" aria-label="arrow-down" onClick={() => onMove(idx, 1)} />
                  ) : null}
                </td>
-                <td style={{ width: '1%' }}>
+                <td role="gridcell" style={{ width: '1%' }}>
                  {!annotation.builtIn && (
                    <DeleteButton
                      size="sm"
--- a/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.test.tsx
+++ b/public/app/features/dashboard/components/DashboardSettings/AnnotationsSettings.test.tsx
@ -101,7 +101,7 @@ describe('AnnotationsSettings', () => {
  test('it renders empty list cta if only builtIn annotation', async () => {
    setup(dashboard);

-    expect(screen.queryByRole('table')).toBeInTheDocument();
+    expect(screen.queryByRole('grid')).toBeInTheDocument();
    expect(screen.getByRole('row', { name: /annotations & alerts \(built\-in\) grafana/i })).toBeInTheDocument();
    expect(
      screen.getByTestId(selectors.components.CallToActionCard.buttonV2('Add annotation query'))
--- a/public/app/features/dashboard/components/LinksSettings/LinkSettingsList.tsx
+++ b/public/app/features/dashboard/components/LinksSettings/LinkSettingsList.tsx
@ -53,7 +53,7 @@ export const LinkSettingsList: React.FC<LinkSettingsListProps> = ({ dashboard, o

  return (
    <>
-      <table className="filter-table filter-table--hover">
+      <table role="grid" className="filter-table filter-table--hover">
        <thead>
          <tr>
            <th>Type</th>
@ -64,28 +64,28 @@ export const LinkSettingsList: React.FC<LinkSettingsListProps> = ({ dashboard, o
        <tbody>
          {links.map((link, idx) => (
            <tr key={`${link.title}-${idx}`}>
-              <td className="pointer" onClick={() => onEdit(idx)}>
+              <td role="gridcell" className="pointer" onClick={() => onEdit(idx)}>
                <Icon name="external-link-alt" /> &nbsp; {link.type}
              </td>
-              <td>
+              <td role="gridcell">
                <HorizontalGroup>
                  {link.title && <span>{link.title}</span>}
                  {link.type === 'link' && <span>{link.url}</span>}
                  {link.type === 'dashboards' && <TagList tags={link.tags ?? []} />}
                </HorizontalGroup>
              </td>
-              <td style={{ width: '1%' }}>
+              <td style={{ width: '1%' }} role="gridcell">
                {idx !== 0 && <IconButton name="arrow-up" aria-label="arrow-up" onClick={() => moveLink(idx, -1)} />}
              </td>
-              <td style={{ width: '1%' }}>
+              <td style={{ width: '1%' }} role="gridcell">
                {links.length > 1 && idx !== links.length - 1 ? (
                  <IconButton name="arrow-down" aria-label="arrow-down" onClick={() => moveLink(idx, 1)} />
                ) : null}
              </td>
-              <td style={{ width: '1%' }}>
+              <td style={{ width: '1%' }} role="gridcell">
                <IconButton aria-label="copy" name="copy" onClick={() => duplicateLink(link, idx)} />
              </td>
-              <td style={{ width: '1%' }}>
+              <td style={{ width: '1%' }} role="gridcell">
                <DeleteButton
                  aria-label={`Delete link with title "${link.title}"`}
                  size="sm"
--- a/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeaderCorner.test.tsx
+++ b/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeaderCorner.test.tsx
@ -20,7 +20,7 @@ describe('Panel header corner test', () => {
    setup();

    expect(
-      screen.getByRole('region', { name: selectors.components.Panels.Panel.headerCornerInfo('info') })
+      screen.getByRole('button', { name: selectors.components.Panels.Panel.headerCornerInfo('info') })
    ).toBeInTheDocument();
  });
 });
--- a/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeaderCorner.tsx
+++ b/public/app/features/dashboard/dashgrid/PanelHeader/PanelHeaderCorner.tsx
@ -86,10 +86,10 @@ export class PanelHeaderCorner extends Component<Props> {

    return (
      <Tooltip content={content} placement="top-start" theme={theme} interactive>
-        <section className={className} onClick={onClick} aria-label={ariaLabel}>
+        <button type="button" className={className} onClick={onClick} aria-label={ariaLabel}>
          <i aria-hidden className="fa" />
          <span className="panel-info-corner-inner" />
-        </section>
+        </button>
      </Tooltip>
    );
  }
--- a/public/app/features/storage/Breadcrumb.tsx
+++ b/public/app/features/storage/Breadcrumb.tsx
@ -18,8 +18,8 @@ export function Breadcrumb({ pathName, onPathChange, rootIcon }: Props) {
  return (
    <ul className={styles.breadCrumb}>
      {rootIcon && (
-        <li onClick={() => onPathChange('')}>
-          <Icon name={rootIcon} />
+        <li>
+          <Icon name={rootIcon} onClick={() => onPathChange('')} />
        </li>
      )}
      {paths.map((path, index) => {
--- a/public/app/features/variables/editor/VariableEditorList.tsx
+++ b/public/app/features/variables/editor/VariableEditorList.tsx
@ -54,6 +54,7 @@ export function VariableEditorList({
            <table
              className="filter-table filter-table--hover"
              aria-label={selectors.pages.Dashboard.Settings.Variables.List.table}
+              role="grid"
            >
              <thead>
                <tr>
--- a/public/app/features/variables/editor/VariableEditorListRow.tsx
+++ b/public/app/features/variables/editor/VariableEditorListRow.tsx
@ -52,7 +52,7 @@ export function VariableEditorListRow({
            ...provided.draggableProps.style,
          }}
        >
-          <td className={styles.column}>
+          <td role="gridcell" className={styles.column}>
            <Button
              size="xs"
              fill="text"
@ -67,6 +67,7 @@ export function VariableEditorListRow({
            </Button>
          </td>
          <td
+            role="gridcell"
            className={styles.definitionColumn}
            onClick={(event) => {
              event.preventDefault();
@ -77,15 +78,15 @@ export function VariableEditorListRow({
            {definition}
          </td>

-          <td className={styles.column}>
+          <td role="gridcell" className={styles.column}>
            <VariableCheckIndicator passed={passed} />
          </td>

-          <td className={styles.column}>
+          <td role="gridcell" className={styles.column}>
            <VariableUsagesButton id={variable.id} isAdhoc={isAdHoc(variable)} usages={usagesNetwork} />
          </td>

-          <td className={styles.column}>
+          <td role="gridcell" className={styles.column}>
            <IconButton
              onClick={(event) => {
                event.preventDefault();
@ -98,7 +99,7 @@ export function VariableEditorListRow({
            />
          </td>

-          <td className={styles.column}>
+          <td role="gridcell" className={styles.column}>
            <IconButton
              onClick={(event) => {
                event.preventDefault();
@ -110,7 +111,7 @@ export function VariableEditorListRow({
              aria-label={selectors.pages.Dashboard.Settings.Variables.List.tableRowRemoveButtons(variable.name)}
            />
          </td>
-          <td className={styles.column}>
+          <td role="gridcell" className={styles.column}>
            <div {...provided.dragHandleProps} className={styles.dragHandle}>
              <Icon name="draggabledots" size="lg" />
            </div>
--- a/public/app/plugins/panel/graph/Legend/Legend.tsx
+++ b/public/app/plugins/panel/graph/Legend/Legend.tsx
@ -265,7 +265,7 @@ class LegendTable extends PureComponent<Partial<LegendComponentProps>> {
    }

    return (
-      <table>
+      <table role="grid">
        <colgroup>
          <col style={{ width: '100%' }} />
        </colgroup>
--- a/public/app/plugins/panel/graph/Legend/LegendSeriesItem.tsx
+++ b/public/app/plugins/panel/graph/Legend/LegendSeriesItem.tsx
@ -107,7 +107,7 @@ export class LegendItem extends PureComponent<LegendItemProps, LegendItemState>
    if (asTable) {
      return (
        <tr className={`graph-legend-series ${seriesOptionClasses}`}>
-          <td>
+          <td role="gridcell">
            <div className="graph-legend-series__table-name">{seriesLabel}</div>
          </td>
          {valueItems}
@ -221,7 +221,7 @@ interface LegendValueProps {
 function LegendValue({ value, valueName, asTable, onValueClick }: LegendValueProps) {
  if (asTable) {
    return (
-      <td className={`graph-legend-value ${valueName}`} onClick={onValueClick}>
+      <td role="gridcell" className={`graph-legend-value ${valueName}`} onClick={onValueClick}>
        {value}
      </td>
    );
--- a/public/sass/components/_panel_header.scss
+++ b/public/sass/components/_panel_header.scss
@ -112,6 +112,8 @@ $panel-header-no-title-zindex: 1;
 }

 .panel-info-corner {
+  background: none;
+  border: none;
  color: $text-muted;
  cursor: pointer;
  position: absolute;
@ -123,8 +125,8 @@ $panel-header-no-title-zindex: 1;
  top: 0;

  .fa {
-    position: relative;
-    top: -2px;
+    position: absolute;
+    top: 6px;
    left: 6px;
    font-size: 75%;
    z-index: $panel-header-no-title-zindex + 2;