From 53a8998c851e077d972f83c397594bcd3e43e2d4 Mon Sep 17 00:00:00 2001
From: Konrad Lalik <konrad.lalik@grafana.com>
Date: Fri, 10 Feb 2023 10:23:40 +0100
Subject: [PATCH] Alerting: a11y improvements (#63072)

---
 .../features/alerting/TestRuleResult.test.tsx |  21 ++-
 .../app/features/alerting/TestRuleResult.tsx  |  22 ++-
 .../expressions/AlertConditionIndicator.tsx   |  29 ++--
 .../components/expressions/Expression.tsx     |  15 +-
 .../receivers/form/CollapsibleSection.tsx     |  27 ++--
 .../rule-editor/DashboardPicker.tsx           | 145 ++++++++++--------
 .../panel/alertlist/AlertInstances.tsx        |  12 +-
 7 files changed, 162 insertions(+), 109 deletions(-)

diff --git a/public/app/features/alerting/TestRuleResult.test.tsx b/public/app/features/alerting/TestRuleResult.test.tsx
index d80398c0564..c62f8ace394 100644
--- a/public/app/features/alerting/TestRuleResult.test.tsx
+++ b/public/app/features/alerting/TestRuleResult.test.tsx
@@ -4,20 +4,22 @@ import React from 'react';
 import { PanelModel } from '../dashboard/state';
 import { createDashboardModelFixture, createPanelJSONFixture } from '../dashboard/state/__fixtures__/dashboardFixtures';
 
-import { TestRuleResult, Props } from './TestRuleResult';
+import { TestRuleResult } from './TestRuleResult';
+
+const backendSrv = {
+  post: jest.fn(),
+};
 
 jest.mock('@grafana/runtime', () => {
   const original = jest.requireActual('@grafana/runtime');
 
   return {
     ...original,
-    getBackendSrv: () => ({
-      post: jest.fn(),
-    }),
+    getBackendSrv: () => backendSrv,
   };
 });
 
-const props: Props = {
+const props: React.ComponentProps<typeof TestRuleResult> = {
   panel: new PanelModel({ id: 1 }),
   dashboard: createDashboardModelFixture({
     panels: [createPanelJSONFixture({ id: 1 })],
@@ -30,9 +32,14 @@ describe('TestRuleResult', () => {
   });
 
   it('should call testRule when mounting', () => {
-    jest.spyOn(TestRuleResult.prototype, 'testRule');
+    jest.spyOn(backendSrv, 'post');
     render(<TestRuleResult {...props} />);
 
-    expect(TestRuleResult.prototype.testRule).toHaveBeenCalled();
+    expect(backendSrv.post).toHaveBeenCalledWith(
+      '/api/alerts/test',
+      expect.objectContaining({
+        panelId: 1,
+      })
+    );
   });
 });
diff --git a/public/app/features/alerting/TestRuleResult.tsx b/public/app/features/alerting/TestRuleResult.tsx
index 3de19747775..dc1504f916f 100644
--- a/public/app/features/alerting/TestRuleResult.tsx
+++ b/public/app/features/alerting/TestRuleResult.tsx
@@ -1,11 +1,20 @@
 import React, { PureComponent } from 'react';
 
 import { getBackendSrv } from '@grafana/runtime';
-import { LoadingPlaceholder, JSONFormatter, Icon, HorizontalGroup, ClipboardButton } from '@grafana/ui';
+import {
+  LoadingPlaceholder,
+  JSONFormatter,
+  Icon,
+  HorizontalGroup,
+  ClipboardButton,
+  clearButtonStyles,
+  withTheme2,
+  Themeable2,
+} from '@grafana/ui';
 
 import { DashboardModel, PanelModel } from '../dashboard/state';
 
-export interface Props {
+export interface Props extends Themeable2 {
   dashboard: DashboardModel;
   panel: PanelModel;
 }
@@ -16,7 +25,7 @@ interface State {
   testRuleResponse: {};
 }
 
-export class TestRuleResult extends PureComponent<Props, State> {
+class UnThemedTestRuleResult extends PureComponent<Props, State> {
   readonly state: State = {
     isLoading: false,
     allNodesExpanded: null,
@@ -90,6 +99,7 @@ export class TestRuleResult extends PureComponent<Props, State> {
 
   render() {
     const { testRuleResponse, isLoading } = this.state;
+    const clearButton = clearButtonStyles(this.props.theme);
 
     if (isLoading === true) {
       return <LoadingPlaceholder text="Evaluating rule" />;
@@ -101,7 +111,9 @@ export class TestRuleResult extends PureComponent<Props, State> {
       <>
         <div className="pull-right">
           <HorizontalGroup spacing="md">
-            <div onClick={this.onToggleExpand}>{this.renderExpandCollapse()}</div>
+            <button type="button" className={clearButton} onClick={this.onToggleExpand}>
+              {this.renderExpandCollapse()}
+            </button>
             <ClipboardButton getText={this.getTextForClipboard} icon="copy">
               Copy to Clipboard
             </ClipboardButton>
@@ -113,3 +125,5 @@ export class TestRuleResult extends PureComponent<Props, State> {
     );
   }
 }
+
+export const TestRuleResult = withTheme2(UnThemedTestRuleResult);
diff --git a/public/app/features/alerting/unified/components/expressions/AlertConditionIndicator.tsx b/public/app/features/alerting/unified/components/expressions/AlertConditionIndicator.tsx
index f48efde64b7..3572af7d877 100644
--- a/public/app/features/alerting/unified/components/expressions/AlertConditionIndicator.tsx
+++ b/public/app/features/alerting/unified/components/expressions/AlertConditionIndicator.tsx
@@ -2,7 +2,7 @@ import { css } from '@emotion/css';
 import React, { FC } from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data';
-import { Badge, useStyles2 } from '@grafana/ui';
+import { Badge, clearButtonStyles, useStyles2 } from '@grafana/ui';
 
 interface AlertConditionProps {
   enabled?: boolean;
@@ -33,22 +33,27 @@ export const AlertConditionIndicator: FC<AlertConditionProps> = ({
 
   if (!enabled) {
     return (
-      <div className={styles.actionLink} onClick={() => onSetCondition && onSetCondition()}>
+      <button type="button" className={styles.actionLink} onClick={() => onSetCondition && onSetCondition()}>
         Make this the alert condition
-      </div>
+      </button>
     );
   }
 
   return null;
 };
 
-const getStyles = (theme: GrafanaTheme2) => ({
-  actionLink: css`
-    color: ${theme.colors.text.link};
-    cursor: pointer;
+const getStyles = (theme: GrafanaTheme2) => {
+  const clearButton = clearButtonStyles(theme);
 
-    &:hover {
-      text-decoration: underline;
-    }
-  `,
-});
+  return {
+    actionLink: css`
+      ${clearButton};
+      color: ${theme.colors.text.link};
+      cursor: pointer;
+
+      &:hover {
+        text-decoration: underline;
+      }
+    `,
+  };
+};
diff --git a/public/app/features/alerting/unified/components/expressions/Expression.tsx b/public/app/features/alerting/unified/components/expressions/Expression.tsx
index 01b898564eb..b2767a1ea3d 100644
--- a/public/app/features/alerting/unified/components/expressions/Expression.tsx
+++ b/public/app/features/alerting/unified/components/expressions/Expression.tsx
@@ -5,7 +5,7 @@ import React, { FC, useCallback, useState } from 'react';
 import { DataFrame, dateTimeFormat, GrafanaTheme2, LoadingState, PanelData } from '@grafana/data';
 import { isTimeSeries } from '@grafana/data/src/dataframe/utils';
 import { Stack } from '@grafana/experimental';
-import { AutoSizeInput, Icon, IconButton, Select, useStyles2 } from '@grafana/ui';
+import { AutoSizeInput, clearButtonStyles, Icon, IconButton, Select, useStyles2 } from '@grafana/ui';
 import { ClassicConditions } from 'app/features/expressions/components/ClassicConditions';
 import { Math } from 'app/features/expressions/components/Math';
 import { Reduce } from 'app/features/expressions/components/Reduce';
@@ -175,6 +175,7 @@ interface HeaderProps {
 
 const Header: FC<HeaderProps> = ({ refId, queryType, onUpdateRefId, onUpdateExpressionType, onRemoveExpression }) => {
   const styles = useStyles2(getStyles);
+  const clearButton = useStyles2(clearButtonStyles);
   /**
    * There are 3 edit modes:
    *
@@ -195,9 +196,9 @@ const Header: FC<HeaderProps> = ({ refId, queryType, onUpdateRefId, onUpdateExpr
       <Stack direction="row" gap={0.5} alignItems="center">
         <Stack direction="row" gap={1} alignItems="center" wrap={false}>
           {!editingRefId && (
-            <div className={styles.editable} onClick={() => setEditMode('refId')}>
+            <button type="button" className={cx(clearButton, styles.editable)} onClick={() => setEditMode('refId')}>
               <div className={styles.expression.refId}>{refId}</div>
-            </div>
+            </button>
           )}
           {editingRefId && (
             <AutoSizeInput
@@ -216,10 +217,14 @@ const Header: FC<HeaderProps> = ({ refId, queryType, onUpdateRefId, onUpdateExpr
             />
           )}
           {!editingType && (
-            <div className={styles.editable} onClick={() => setEditMode('expressionType')}>
+            <button
+              type="button"
+              className={cx(clearButton, styles.editable)}
+              onClick={() => setEditMode('expressionType')}
+            >
               <div className={styles.mutedText}>{capitalize(queryType)}</div>
               <Icon size="xs" name="pen" className={styles.mutedIcon} onClick={() => setEditMode('expressionType')} />
-            </div>
+            </button>
           )}
           {editingType && (
             <Select
diff --git a/public/app/features/alerting/unified/components/receivers/form/CollapsibleSection.tsx b/public/app/features/alerting/unified/components/receivers/form/CollapsibleSection.tsx
index bd424939cf4..0097045d924 100644
--- a/public/app/features/alerting/unified/components/receivers/form/CollapsibleSection.tsx
+++ b/public/app/features/alerting/unified/components/receivers/form/CollapsibleSection.tsx
@@ -27,12 +27,15 @@ export const CollapsibleSection = ({
 
   return (
     <div className={cx(styles.wrapper, className)}>
-      <div className={styles.heading} onClick={toggleCollapse}>
-        <CollapseToggle className={styles.caret} size={size} onToggle={toggleCollapse} isCollapsed={isCollapsed} />
-        <h6>{label}</h6>
-      </div>
+      <CollapseToggle
+        className={styles.toggle}
+        size={size}
+        onToggle={toggleCollapse}
+        isCollapsed={isCollapsed}
+        text={label}
+      />
       {description && <p className={styles.description}>{description}</p>}
-      <div className={isCollapsed ? styles.hidden : undefined}>{children}</div>
+      <div className={isCollapsed ? styles.hidden : styles.content}>{children}</div>
     </div>
   );
 };
@@ -42,14 +45,9 @@ const getStyles = (theme: GrafanaTheme2) => ({
     margin-top: ${theme.spacing(1)};
     padding-bottom: ${theme.spacing(1)};
   `,
-  caret: css`
-    margin-left: -${theme.spacing(0.5)}; // make it align with fields despite icon size
-  `,
-  heading: css`
-    cursor: pointer;
-    h6 {
-      display: inline-block;
-    }
+  toggle: css`
+    margin: ${theme.spacing(1, 0)};
+    padding: 0;
   `,
   hidden: css`
     display: none;
@@ -60,4 +58,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
     font-weight: ${theme.typography.fontWeightRegular};
     margin: 0;
   `,
+  content: css`
+    padding-left: ${theme.spacing(3)};
+  `,
 });
diff --git a/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.tsx b/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.tsx
index ab0fd01aa52..72d8afc57b9 100644
--- a/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.tsx
@@ -5,7 +5,16 @@ import AutoSizer from 'react-virtualized-auto-sizer';
 import { FixedSizeList } from 'react-window';
 
 import { GrafanaTheme2 } from '@grafana/data/src';
-import { FilterInput, LoadingPlaceholder, useStyles2, Icon, Modal, Button, Alert } from '@grafana/ui';
+import {
+  FilterInput,
+  LoadingPlaceholder,
+  useStyles2,
+  Icon,
+  Modal,
+  Button,
+  Alert,
+  clearButtonStyles,
+} from '@grafana/ui';
 
 import { dashboardApi } from '../../api/dashboardApi';
 
@@ -99,17 +108,18 @@ export const DashboardPicker = ({ dashboardUid, panelId, isOpen, onChange, onDis
     const isSelected = selectedDashboardUid === dashboard.uid;
 
     return (
-      <div
+      <button
+        type="button"
         title={dashboard.title}
         style={style}
-        className={cx(styles.row, { [styles.rowOdd]: index % 2 === 1, [styles.rowSelected]: isSelected })}
+        className={cx(styles.rowButton, { [styles.rowOdd]: index % 2 === 1, [styles.rowSelected]: isSelected })}
         onClick={() => handleDashboardChange(dashboard.uid)}
       >
         <div className={styles.dashboardTitle}>{dashboard.title}</div>
         <div className={styles.dashboardFolder}>
           <Icon name="folder" /> {dashboard.folderTitle ?? 'General'}
         </div>
-      </div>
+      </button>
     );
   };
 
@@ -118,13 +128,14 @@ export const DashboardPicker = ({ dashboardUid, panelId, isOpen, onChange, onDis
     const isSelected = selectedPanelId === panel.id.toString();
 
     return (
-      <div
+      <button
+        type="button"
         style={style}
-        className={cx(styles.row, { [styles.rowOdd]: index % 2 === 1, [styles.rowSelected]: isSelected })}
+        className={cx(styles.rowButton, { [styles.rowOdd]: index % 2 === 1, [styles.rowSelected]: isSelected })}
         onClick={() => setSelectedPanelId(panel.id.toString())}
       >
         {panel.title || '<No title>'}
-      </div>
+      </button>
     );
   };
 
@@ -221,60 +232,66 @@ export const DashboardPicker = ({ dashboardUid, panelId, isOpen, onChange, onDis
   );
 };
 
-const getPickerStyles = (theme: GrafanaTheme2) => ({
-  container: css`
-    display: grid;
-    grid-template-columns: 1fr 1fr;
-    grid-template-rows: min-content auto;
-    gap: ${theme.spacing(2)};
-    flex: 1;
-  `,
-  column: css`
-    flex: 1 1 auto;
-  `,
-  dashboardTitle: css`
-    height: 22px;
-    font-weight: ${theme.typography.fontWeightBold};
-  `,
-  dashboardFolder: css`
-    height: 20px;
-    font-size: ${theme.typography.bodySmall.fontSize};
-    color: ${theme.colors.text.secondary};
-    display: flex;
-    flex-direction: row;
-    justify-content: flex-start;
-    column-gap: ${theme.spacing(1)};
-    align-items: center;
-  `,
-  row: css`
-    padding: ${theme.spacing(0.5)};
-    overflow: hidden;
-    text-overflow: ellipsis;
-    white-space: nowrap;
-    cursor: pointer;
-    border: 2px solid transparent;
-  `,
-  rowSelected: css`
-    border-color: ${theme.colors.primary.border};
-  `,
-  rowOdd: css`
-    background-color: ${theme.colors.background.secondary};
-  `,
-  loadingPlaceholder: css`
-    height: 100%;
-    display: flex;
-    justify-content: center;
-    align-items: center;
-  `,
-  modal: css`
-    height: 100%;
-  `,
-  modalContent: css`
-    flex: 1;
-    display: flex;
-    flex-direction: column;
-  `,
-  modalAlert: css`
-    flex-grow: 0;
-  `,
-});
+const getPickerStyles = (theme: GrafanaTheme2) => {
+  const clearButton = clearButtonStyles(theme);
+
+  return {
+    container: css`
+      display: grid;
+      grid-template-columns: 1fr 1fr;
+      grid-template-rows: min-content auto;
+      gap: ${theme.spacing(2)};
+      flex: 1;
+    `,
+    column: css`
+      flex: 1 1 auto;
+    `,
+    dashboardTitle: css`
+      height: 22px;
+      font-weight: ${theme.typography.fontWeightBold};
+    `,
+    dashboardFolder: css`
+      height: 20px;
+      font-size: ${theme.typography.bodySmall.fontSize};
+      color: ${theme.colors.text.secondary};
+      display: flex;
+      flex-direction: row;
+      justify-content: flex-start;
+      column-gap: ${theme.spacing(1)};
+      align-items: center;
+    `,
+    rowButton: css`
+      ${clearButton};
+      padding: ${theme.spacing(0.5)};
+      overflow: hidden;
+      text-overflow: ellipsis;
+      text-align: left;
+      white-space: nowrap;
+      cursor: pointer;
+      border: 2px solid transparent;
+    `,
+    rowSelected: css`
+      border-color: ${theme.colors.primary.border};
+    `,
+    rowOdd: css`
+      background-color: ${theme.colors.background.secondary};
+    `,
+    loadingPlaceholder: css`
+      height: 100%;
+      display: flex;
+      justify-content: center;
+      align-items: center;
+    `,
+    modal: css`
+      height: 100%;
+    `,
+    modalContent: css`
+      flex: 1;
+      display: flex;
+      flex-direction: column;
+    `,
+    modalAlert: css`
+      flex-grow: 0;
+    `,
+  };
+};
diff --git a/public/app/plugins/panel/alertlist/AlertInstances.tsx b/public/app/plugins/panel/alertlist/AlertInstances.tsx
index d598fec4094..6c1edec5eb3 100644
--- a/public/app/plugins/panel/alertlist/AlertInstances.tsx
+++ b/public/app/plugins/panel/alertlist/AlertInstances.tsx
@@ -1,10 +1,10 @@
-import { css } from '@emotion/css';
+import { css, cx } from '@emotion/css';
 import { noop } from 'lodash';
 import pluralize from 'pluralize';
 import React, { useCallback, useEffect, useMemo, useState } from 'react';
 
 import { GrafanaTheme2, PanelProps } from '@grafana/data';
-import { Icon, useStyles2 } from '@grafana/ui';
+import { clearButtonStyles, Icon, useStyles2 } from '@grafana/ui';
 import { AlertInstancesTable } from 'app/features/alerting/unified/components/rules/AlertInstancesTable';
 import { sortAlerts } from 'app/features/alerting/unified/utils/misc';
 import { Alert } from 'app/types/unified-alerting';
@@ -24,6 +24,7 @@ export const AlertInstances = ({ alerts, options }: Props) => {
   const defaultShowInstances = options.groupMode === GroupMode.Custom ? true : options.showInstances;
   const [displayInstances, setDisplayInstances] = useState<boolean>(defaultShowInstances);
   const styles = useStyles2(getStyles);
+  const clearButton = useStyles2(clearButtonStyles);
 
   const toggleDisplayInstances = useCallback(() => {
     setDisplayInstances((display) => !display);
@@ -50,11 +51,14 @@ export const AlertInstances = ({ alerts, options }: Props) => {
   return (
     <div>
       {options.groupMode === GroupMode.Default && (
-        <div className={uncollapsible ? styles.clickable : ''} onClick={() => toggleShowInstances()}>
+        <button
+          className={cx(clearButton, uncollapsible ? styles.clickable : '')}
+          onClick={() => toggleShowInstances()}
+        >
           {uncollapsible && <Icon name={displayInstances ? 'angle-down' : 'angle-right'} size={'md'} />}
           <span>{`${filteredAlerts.length} ${pluralize('instance', filteredAlerts.length)}`}</span>
           {hiddenInstances > 0 && <span>, {`${hiddenInstances} hidden by filters`}</span>}
-        </div>
+        </button>
       )}
       {displayInstances && (
         <AlertInstancesTable