QueryRow: Make toggle actions screen-readers accessible (#67998)

e2e/panels-suite/panelEdit_queries.spec.ts

@@ -65,7 +65,7 @@ e2e.scenario({
     });
 
     // Disable row with refId A
-    e2e.components.QueryEditorRow.actionButton('Disable/enable query').eq(1).should('be.visible').click();
+    e2e.components.QueryEditorRow.actionButton('Disable query').eq(1).should('be.visible').click();
 
     expectInspectorResultAndClose((keys) => {
       const length = keys.length;
@@ -73,7 +73,7 @@ e2e.scenario({
     });
 
     // Enable row with refId B
-    e2e.components.QueryEditorRow.actionButton('Disable/enable query').eq(1).should('be.visible').click();
+    e2e.components.QueryEditorRow.actionButton('Disable query').eq(1).should('be.visible').click();
 
     expectInspectorResultAndClose((keys) => {
       const length = keys.length;

packages/grafana-e2e-selectors/src/selectors/components.ts

@@ -193,7 +193,7 @@ export const Components = {
     rows: 'Query editor row',
   },
   QueryEditorRow: {
-    actionButton: (title: string) => `${title} query operation action`,
+    actionButton: (title: string) => `${title}`,
     title: (refId: string) => `Query editor row title ${refId}`,
     container: (refId: string) => `Query editor row ${refId}`,
   },

public/app/core/components/QueryOperationRow/QueryOperationAction.test.tsx

@@ -1,24 +1,24 @@
 import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
-import React from 'react';
+import React, { ComponentPropsWithoutRef } from 'react';
 
 import { selectors } from '@grafana/e2e-selectors';
 
-import { QueryOperationAction, QueryOperationActionProps } from './QueryOperationAction';
+import { QueryOperationAction, QueryOperationToggleAction } from './QueryOperationAction';
 
-const setup = (propOverrides?: Partial<QueryOperationActionProps>) => {
-  const props: QueryOperationActionProps = {
-    icon: 'panel-add',
-    title: 'test',
-    onClick: jest.fn(),
-    disabled: false,
-    ...propOverrides,
-  };
+describe('QueryOperationAction tests', () => {
+  function setup(propOverrides?: Partial<ComponentPropsWithoutRef<typeof QueryOperationAction>>) {
+    const props: ComponentPropsWithoutRef<typeof QueryOperationAction> = {
+      icon: 'panel-add',
+      title: 'test',
+      onClick: jest.fn(),
+      disabled: false,
+      ...propOverrides,
+    };
 
-  render(<QueryOperationAction {...props} />);
-};
+    render(<QueryOperationAction {...props} />);
+  }
 
-describe('QueryOperationAction tests', () => {
   it('should render component', () => {
     setup();
 
@@ -51,3 +51,36 @@ describe('QueryOperationAction tests', () => {
     expect(clickSpy).not.toHaveBeenCalled();
   });
 });
+
+describe('QueryOperationToggleAction', () => {
+  function setup(active: boolean) {
+    const props: ComponentPropsWithoutRef<typeof QueryOperationToggleAction> = {
+      icon: 'panel-add',
+      title: 'test',
+      onClick: () => {},
+      active,
+    };
+
+    return render(<QueryOperationToggleAction {...props} />);
+  }
+
+  it('should correctly set pressed state', () => {
+    setup(false);
+
+    expect(
+      screen.getByRole('button', {
+        name: selectors.components.QueryEditorRow.actionButton('test'),
+        pressed: false,
+      })
+    ).toBeInTheDocument();
+
+    setup(true);
+
+    expect(
+      screen.getByRole('button', {
+        name: selectors.components.QueryEditorRow.actionButton('test'),
+        pressed: true,
+      })
+    ).toBeInTheDocument();
+  });
+});

public/app/core/components/QueryOperationRow/QueryOperationAction.tsx

@@ -5,33 +5,43 @@ import { GrafanaTheme2 } from '@grafana/data';
 import { selectors } from '@grafana/e2e-selectors';
 import { IconButton, IconName, useStyles2 } from '@grafana/ui';
 
-export interface QueryOperationActionProps {
+interface BaseQueryOperationActionProps {
   icon: IconName;
   title: string;
   onClick: (e: React.MouseEvent) => void;
   disabled?: boolean;
-  active?: boolean;
 }
 
-export const QueryOperationAction = ({ icon, active, disabled, title, onClick }: QueryOperationActionProps) => {
+function BaseQueryOperationAction(props: QueryOperationActionProps | QueryOperationToggleActionProps) {
   const styles = useStyles2(getStyles);
 
   return (
-    <div className={cx(styles.icon, active && styles.active)}>
+    <div className={cx(styles.icon, 'active' in props && props.active && styles.active)}>
       <IconButton
-        name={icon}
-        tooltip={title}
+        name={props.icon}
+        tooltip={props.title}
         className={styles.icon}
-        disabled={!!disabled}
-        onClick={onClick}
+        disabled={!!props.disabled}
+        onClick={props.onClick}
         type="button"
-        aria-label={selectors.components.QueryEditorRow.actionButton(title)}
+        aria-label={selectors.components.QueryEditorRow.actionButton(props.title)}
+        {...('active' in props && { 'aria-pressed': props.active })}
       />
     </div>
   );
-};
+}
+
+interface QueryOperationActionProps extends BaseQueryOperationActionProps {}
+export function QueryOperationAction(props: QueryOperationActionProps) {
+  return <BaseQueryOperationAction {...props} />;
+}
 
-QueryOperationAction.displayName = 'QueryOperationAction';
+interface QueryOperationToggleActionProps extends BaseQueryOperationActionProps {
+  active: boolean;
+}
+export const QueryOperationToggleAction = (props: QueryOperationToggleActionProps) => {
+  return <BaseQueryOperationAction {...props} />;
+};
 
 const getStyles = (theme: GrafanaTheme2) => {
   return {

public/app/features/dashboard/components/TransformationsEditor/TransformationOperationRow.tsx

@@ -5,7 +5,10 @@ import { DataFrame, DataTransformerConfig, TransformerRegistryItem, FrameMatcher
 import { reportInteraction } from '@grafana/runtime';
 import { HorizontalGroup } from '@grafana/ui';
 import { OperationRowHelp } from 'app/core/components/QueryOperationRow/OperationRowHelp';
-import { QueryOperationAction } from 'app/core/components/QueryOperationRow/QueryOperationAction';
+import {
+  QueryOperationAction,
+  QueryOperationToggleAction,
+} from 'app/core/components/QueryOperationRow/QueryOperationAction';
 import {
   QueryOperationRow,
   QueryOperationRowRenderProps,
@@ -37,7 +40,7 @@ export const TransformationOperationRow = ({
 }: TransformationOperationRowProps) => {
   const [showDebug, toggleDebug] = useToggle(false);
   const [showHelp, toggleHelp] = useToggle(false);
-  const disabled = configs[index].transformation.disabled;
+  const disabled = !!configs[index].transformation.disabled;
   const filter = configs[index].transformation.filter != null;
   const showFilter = filter || data.length > 1;
 
@@ -85,29 +88,29 @@ export const TransformationOperationRow = ({
     return (
       <HorizontalGroup align="center" width="auto">
         {uiConfig.state && <PluginStateInfo state={uiConfig.state} />}
-        <QueryOperationAction
-          title="Show/hide transform help"
+        <QueryOperationToggleAction
+          title="Show transform help"
           icon="info-circle"
           onClick={instrumentToggleCallback(toggleHelp, 'help', showHelp)}
           active={showHelp}
         />
         {showFilter && (
-          <QueryOperationAction
+          <QueryOperationToggleAction
             title="Filter"
             icon="filter"
             onClick={instrumentToggleCallback(toggleFilter, 'filter', filter)}
             active={filter}
           />
         )}
-        <QueryOperationAction
+        <QueryOperationToggleAction
           title="Debug"
           disabled={!isOpen}
           icon="bug"
           onClick={instrumentToggleCallback(toggleDebug, 'debug', showDebug)}
           active={showDebug}
         />
-        <QueryOperationAction
-          title="Disable/Enable transformation"
+        <QueryOperationToggleAction
+          title="Disable transformation"
           icon={disabled ? 'eye-slash' : 'eye'}
           onClick={instrumentToggleCallback(() => onDisableToggle(index), 'disabled', disabled)}
           active={disabled}

public/app/features/query/components/QueryEditorRow.tsx

@@ -25,7 +25,10 @@ import { selectors } from '@grafana/e2e-selectors';
 import { AngularComponent, getAngularLoader, getDataSourceSrv } from '@grafana/runtime';
 import { Badge, ErrorBoundaryAlert, HorizontalGroup } from '@grafana/ui';
 import { OperationRowHelp } from 'app/core/components/QueryOperationRow/OperationRowHelp';
-import { QueryOperationAction } from 'app/core/components/QueryOperationRow/QueryOperationAction';
+import {
+  QueryOperationAction,
+  QueryOperationToggleAction,
+} from 'app/core/components/QueryOperationRow/QueryOperationAction';
 import {
   QueryOperationRow,
   QueryOperationRowRenderProps,
@@ -430,15 +433,15 @@ export class QueryEditorRow<TQuery extends DataQuery> extends PureComponent<Prop
   renderActions = (props: QueryOperationRowRenderProps) => {
     const { query, hideDisableQuery = false } = this.props;
     const { hasTextEditMode, datasource, showingHelp } = this.state;
-    const isDisabled = query.hide;
+    const isDisabled = !!query.hide;
 
     const hasEditorHelp = datasource?.components?.QueryEditorHelp;
 
     return (
       <HorizontalGroup width="auto">
         {hasEditorHelp && (
-          <QueryOperationAction
-            title="Toggle data source help"
+          <QueryOperationToggleAction
+            title="Show data source help"
             icon="question-circle"
             onClick={this.onToggleHelp}
             active={showingHelp}
@@ -456,8 +459,8 @@ export class QueryEditorRow<TQuery extends DataQuery> extends PureComponent<Prop
         {this.renderExtraActions()}
         <QueryOperationAction title="Duplicate query" icon="copy" onClick={this.onCopyQuery} />
         {!hideDisableQuery ? (
-          <QueryOperationAction
-            title="Disable/enable query"
+          <QueryOperationToggleAction
+            title="Disable query"
             icon={isDisabled ? 'eye-slash' : 'eye'}
             active={isDisabled}
             onClick={this.onDisableQuery}

public/app/features/query/components/QueryEditorRows.test.tsx

@@ -114,10 +114,7 @@ describe('QueryEditorRows', () => {
     renderScenario({ onAddQuery, onQueryCopied });
     const queryEditorRows = await screen.findAllByTestId('query-editor-row');
     queryEditorRows.map(async (childQuery) => {
-      const duplicateQueryButton = queryByLabelText(
-        childQuery,
-        'Duplicate query query operation action'
-      ) as HTMLElement;
+      const duplicateQueryButton = queryByLabelText(childQuery, 'Duplicate query') as HTMLElement;
 
       expect(duplicateQueryButton).toBeInTheDocument();
 
@@ -135,7 +132,7 @@ describe('QueryEditorRows', () => {
 
     const queryEditorRows = await screen.findAllByTestId('query-editor-row');
     queryEditorRows.map(async (childQuery) => {
-      const deleteQueryButton = queryByLabelText(childQuery, 'Remove query query operation action') as HTMLElement;
+      const deleteQueryButton = queryByLabelText(childQuery, 'Remove query') as HTMLElement;
 
       expect(deleteQueryButton).toBeInTheDocument();