From f19b07c0bc096ac6190cc71ec1f8baa1f87da061 Mon Sep 17 00:00:00 2001
From: Ashley Harrison <ashley.harrison@grafana.com>
Date: Mon, 30 Jan 2023 13:32:16 +0000
Subject: [PATCH] Accessibility: Remove unnecessary onClick from title element
 (#59696)

* remove unnecessary onClick from title element

* associate icon label with title + fix unit tests

* remove cursor: pointer;

* remove link color from title as it's no longer clickable

* remove useless aria-label

* stop incorrectly marking title as a ReactNode - it's just a string
---
 .../QueryOperationRow.test.tsx                  |  2 +-
 .../QueryOperationRow/QueryOperationRow.tsx     |  5 ++---
 .../QueryOperationRowHeader.tsx                 | 17 ++++++++++-------
 .../features/inspector/InspectDataTab.test.tsx  |  4 ++--
 4 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx b/public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx
index 2efdd6a2729..3a3db9e0527 100644
--- a/public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx
+++ b/public/app/core/components/QueryOperationRow/QueryOperationRow.test.tsx
@@ -32,7 +32,7 @@ describe('QueryOperationRow', () => {
       const onCloseSpy = jest.fn();
       setup({ isOpen: false, onOpen: onOpenSpy, onClose: onCloseSpy });
 
-      const queryRow = screen.getByText(/^test-title$/);
+      const queryRow = screen.getByRole('button', { name: /^test-title$/ });
       expect(queryRow).toBeInTheDocument();
 
       // open row on click
diff --git a/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx b/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx
index 8b901100558..4c7d826940b 100644
--- a/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx
+++ b/public/app/core/components/QueryOperationRow/QueryOperationRow.tsx
@@ -90,7 +90,6 @@ export function QueryOperationRow({
     },
   };
 
-  const titleElement = title && ReactUtils.renderOrCallToRender(title, renderPropArgs);
   const actionsElement = actions && ReactUtils.renderOrCallToRender(actions, renderPropArgs);
   const headerElementRendered = headerElement && ReactUtils.renderOrCallToRender(headerElement, renderPropArgs);
 
@@ -112,7 +111,7 @@ export function QueryOperationRow({
                     isContentVisible={isContentVisible}
                     onRowToggle={onRowToggle}
                     reportDragMousePosition={reportDragMousePosition}
-                    titleElement={titleElement}
+                    title={title}
                   />
                 </div>
                 {isContentVisible && <div className={styles.content}>{children}</div>}
@@ -135,7 +134,7 @@ export function QueryOperationRow({
         isContentVisible={isContentVisible}
         onRowToggle={onRowToggle}
         reportDragMousePosition={reportDragMousePosition}
-        titleElement={titleElement}
+        title={title}
       />
       {isContentVisible && <div className={styles.content}>{children}</div>}
     </div>
diff --git a/public/app/core/components/QueryOperationRow/QueryOperationRowHeader.tsx b/public/app/core/components/QueryOperationRow/QueryOperationRowHeader.tsx
index 3e1873571d7..76df2471d05 100644
--- a/public/app/core/components/QueryOperationRow/QueryOperationRowHeader.tsx
+++ b/public/app/core/components/QueryOperationRow/QueryOperationRowHeader.tsx
@@ -1,4 +1,5 @@
 import { css, cx } from '@emotion/css';
+import { useId } from '@react-aria/utils';
 import React, { MouseEventHandler } from 'react';
 import { DraggableProvidedDragHandleProps } from 'react-beautiful-dnd';
 
@@ -14,7 +15,7 @@ interface QueryOperationRowHeaderProps {
   isContentVisible: boolean;
   onRowToggle: () => void;
   reportDragMousePosition: MouseEventHandler<HTMLDivElement>;
-  titleElement?: React.ReactNode;
+  title?: string;
   id: string;
 }
 
@@ -27,10 +28,11 @@ export const QueryOperationRowHeader: React.FC<QueryOperationRowHeaderProps> = (
   isContentVisible,
   onRowToggle,
   reportDragMousePosition,
-  titleElement,
+  title,
   id,
 }: QueryOperationRowHeaderProps) => {
   const styles = useStyles2(getStyles);
+  const titleId = useId();
 
   return (
     <div className={styles.header}>
@@ -44,10 +46,13 @@ export const QueryOperationRowHeader: React.FC<QueryOperationRowHeaderProps> = (
           type="button"
           aria-expanded={isContentVisible}
           aria-controls={id}
+          aria-labelledby={titleId}
         />
-        {titleElement && (
-          <div className={styles.titleWrapper} onClick={onRowToggle} aria-label="Query operation row title">
-            <div className={cx(styles.title, disabled && styles.disabled)}>{titleElement}</div>
+        {title && (
+          <div className={styles.titleWrapper}>
+            <div id={titleId} className={cx(styles.title, disabled && styles.disabled)}>
+              {title}
+            </div>
           </div>
         )}
         {headerElement}
@@ -109,13 +114,11 @@ const getStyles = (theme: GrafanaTheme2) => ({
     display: flex;
     align-items: center;
     flex-grow: 1;
-    cursor: pointer;
     overflow: hidden;
     margin-right: ${theme.spacing(0.5)};
   `,
   title: css`
     font-weight: ${theme.typography.fontWeightBold};
-    color: ${theme.colors.text.link};
     margin-left: ${theme.spacing(0.5)};
     overflow: hidden;
     text-overflow: ellipsis;
diff --git a/public/app/features/inspector/InspectDataTab.test.tsx b/public/app/features/inspector/InspectDataTab.test.tsx
index 0dc821cb758..547fb927abf 100644
--- a/public/app/features/inspector/InspectDataTab.test.tsx
+++ b/public/app/features/inspector/InspectDataTab.test.tsx
@@ -50,14 +50,14 @@ describe('InspectDataTab', () => {
     });
     it('should show available options', async () => {
       render(<InspectDataTab {...createProps()} />);
-      const dataOptions = screen.getByText(/Data options/i);
+      const dataOptions = screen.getByRole('button', { name: /Data options/i });
       await userEvent.click(dataOptions);
       expect(screen.getByText(/Show data frame/i)).toBeInTheDocument();
       expect(screen.getByText(/Download for Excel/i)).toBeInTheDocument();
     });
     it('should show available dataFrame options', async () => {
       render(<InspectDataTab {...createProps()} />);
-      const dataOptions = screen.getByText(/Data options/i);
+      const dataOptions = screen.getByRole('button', { name: /Data options/i });
       await userEvent.click(dataOptions);
       const dataFrameInput = screen.getByRole('combobox', { name: /Select dataframe/i });
       await userEvent.click(dataFrameInput);