From 29119a7d08b3123caccffa3ed42b24379c65912f Mon Sep 17 00:00:00 2001
From: Levente Balogh <balogh.levente.hu@gmail.com>
Date: Wed, 18 Jan 2023 10:16:41 +0100
Subject: [PATCH] UI/Alert: Infer the `role` property based on the `severity`
 (#61242)

* feat(UI/Alert): add two new optional properties: 'ariaLabel' and 'role'

* docs(UI/Alert): add some docs to the props

* feat: infer the role based on the severity

* fix: stop overriding props

* fix: fix a test depending on the wrong alert role
---
 .../src/components/Alert/Alert.test.tsx       | 28 ++++++++++++++++++-
 .../grafana-ui/src/components/Alert/Alert.tsx | 20 +++++++------
 .../Forms/QueryEditorField.test.tsx           |  2 +-
 3 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/packages/grafana-ui/src/components/Alert/Alert.test.tsx b/packages/grafana-ui/src/components/Alert/Alert.test.tsx
index d8f7c670066..01de576a7d6 100644
--- a/packages/grafana-ui/src/components/Alert/Alert.test.tsx
+++ b/packages/grafana-ui/src/components/Alert/Alert.test.tsx
@@ -4,8 +4,34 @@ import React from 'react';
 import { Alert } from './Alert';
 
 describe('Alert', () => {
-  it('sets the accessible label correctly based on the title', () => {
+  it('sets the accessible label correctly based on the title if there is no aria-label set', () => {
     render(<Alert title="Uh oh spagghettios!" />);
     expect(screen.getByRole('alert', { name: 'Uh oh spagghettios!' })).toBeInTheDocument();
   });
+
+  it('prefers the aria-label attribute over the title if it is set', () => {
+    render(<Alert title="Uh oh spagghettios!" aria-label="A fancy label" />);
+    expect(screen.queryByRole('alert', { name: 'Uh oh spagghettios!' })).not.toBeInTheDocument();
+    expect(screen.getByRole('alert', { name: 'A fancy label' })).toBeInTheDocument();
+  });
+
+  it('infers the role based on the severity in case it is not set manually', () => {
+    render(<Alert title="Error message" severity="error" />);
+    expect(screen.getByRole('alert', { name: 'Error message' })).toBeInTheDocument();
+
+    render(<Alert title="Warning message" severity="warning" />);
+    expect(screen.getByRole('alert', { name: 'Warning message' })).toBeInTheDocument();
+
+    render(<Alert title="Success message" severity="success" />);
+    expect(screen.getByRole('status', { name: 'Success message' })).toBeInTheDocument();
+
+    render(<Alert title="Info message" severity="info" />);
+    expect(screen.getByRole('status', { name: 'Info message' })).toBeInTheDocument();
+  });
+
+  it('is possible to set the role manually', () => {
+    render(<Alert title="Error message" severity="error" role="status" />);
+    expect(screen.queryByRole('alert', { name: 'Error message' })).not.toBeInTheDocument();
+    expect(screen.getByRole('status', { name: 'Error message' })).toBeInTheDocument();
+  });
 });
diff --git a/packages/grafana-ui/src/components/Alert/Alert.tsx b/packages/grafana-ui/src/components/Alert/Alert.tsx
index a8ba6290504..b9ae2e94f53 100644
--- a/packages/grafana-ui/src/components/Alert/Alert.tsx
+++ b/packages/grafana-ui/src/components/Alert/Alert.tsx
@@ -1,6 +1,5 @@
 import { css, cx } from '@emotion/css';
-import { useId } from '@react-aria/utils';
-import React, { HTMLAttributes, ReactNode } from 'react';
+import React, { AriaRole, HTMLAttributes, ReactNode } from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { selectors } from '@grafana/e2e-selectors';
@@ -56,15 +55,22 @@ export const Alert = React.forwardRef<HTMLDivElement, Props>(
     const theme = useTheme2();
     const hasTitle = Boolean(title);
     const styles = getStyles(theme, severity, hasTitle, elevated, bottomSpacing, topSpacing);
-    const titleId = useId();
+    const rolesBySeverity: Record<AlertVariant, AriaRole> = {
+      error: 'alert',
+      warning: 'alert',
+      info: 'status',
+      success: 'status',
+    };
+    const role = restProps['role'] || rolesBySeverity[severity];
+    const ariaLabel = restProps['aria-label'] || title;
 
     return (
       <div
         ref={ref}
         className={cx(styles.alert, className)}
         data-testid={selectors.components.Alert.alertV2(severity)}
-        role="alert"
-        aria-labelledby={titleId}
+        role={role}
+        aria-label={ariaLabel}
         {...restProps}
       >
         <div className={styles.icon}>
@@ -72,9 +78,7 @@ export const Alert = React.forwardRef<HTMLDivElement, Props>(
         </div>
 
         <div className={styles.body}>
-          <div id={titleId} className={styles.title}>
-            {title}
-          </div>
+          <div className={styles.title}>{title}</div>
           {children && <div className={styles.content}>{children}</div>}
         </div>
 
diff --git a/public/app/features/correlations/Forms/QueryEditorField.test.tsx b/public/app/features/correlations/Forms/QueryEditorField.test.tsx
index e2f8ec081c5..b5989a0f077 100644
--- a/public/app/features/correlations/Forms/QueryEditorField.test.tsx
+++ b/public/app/features/correlations/Forms/QueryEditorField.test.tsx
@@ -69,7 +69,7 @@ describe('QueryEditorField', () => {
   it('shows an info alert when no datasource is selected', async () => {
     renderWithContext(<QueryEditorField name="query" />);
 
-    expect(await screen.findByRole('alert', { name: 'No data source selected' })).toBeInTheDocument();
+    expect(await screen.findByRole('status', { name: 'No data source selected' })).toBeInTheDocument();
   });
 
   it('shows an info alert when datasaource does not export a query editor', async () => {