From b99e9668a91669029339bdcfbd0616ff574e1af8 Mon Sep 17 00:00:00 2001 From: Giordano Ricci Date: Wed, 10 May 2023 11:57:08 +0100 Subject: [PATCH] QueryRow: Make toggle actions screen-readers accessible (#67998) --- e2e/panels-suite/panelEdit_queries.spec.ts | 4 +- .../src/selectors/components.ts | 2 +- .../QueryOperationAction.test.tsx | 61 ++++++++++++++----- .../QueryOperationAction.tsx | 32 ++++++---- .../TransformationOperationRow.tsx | 19 +++--- .../query/components/QueryEditorRow.tsx | 15 +++-- .../query/components/QueryEditorRows.test.tsx | 7 +-- 7 files changed, 93 insertions(+), 47 deletions(-) diff --git a/e2e/panels-suite/panelEdit_queries.spec.ts b/e2e/panels-suite/panelEdit_queries.spec.ts index da1e8ef66b6..5f80d931f05 100644 --- a/e2e/panels-suite/panelEdit_queries.spec.ts +++ b/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; diff --git a/packages/grafana-e2e-selectors/src/selectors/components.ts b/packages/grafana-e2e-selectors/src/selectors/components.ts index 1e303539e32..7b13bd96fcc 100644 --- a/packages/grafana-e2e-selectors/src/selectors/components.ts +++ b/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}`, }, diff --git a/public/app/core/components/QueryOperationRow/QueryOperationAction.test.tsx b/public/app/core/components/QueryOperationRow/QueryOperationAction.test.tsx index 7c964e958b1..b7d583a01a2 100644 --- a/public/app/core/components/QueryOperationRow/QueryOperationAction.test.tsx +++ b/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'; - -const setup = (propOverrides?: Partial) => { - const props: QueryOperationActionProps = { - icon: 'panel-add', - title: 'test', - onClick: jest.fn(), - disabled: false, - ...propOverrides, - }; - - render(); -}; +import { QueryOperationAction, QueryOperationToggleAction } from './QueryOperationAction'; describe('QueryOperationAction tests', () => { + function setup(propOverrides?: Partial>) { + const props: ComponentPropsWithoutRef = { + icon: 'panel-add', + title: 'test', + onClick: jest.fn(), + disabled: false, + ...propOverrides, + }; + + render(); + } + 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 = { + icon: 'panel-add', + title: 'test', + onClick: () => {}, + active, + }; + + return render(); + } + + 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(); + }); +}); diff --git a/public/app/core/components/QueryOperationRow/QueryOperationAction.tsx b/public/app/core/components/QueryOperationRow/QueryOperationAction.tsx index 5918d6067e9..fe69509ac4a 100644 --- a/public/app/core/components/QueryOperationRow/QueryOperationAction.tsx +++ b/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 ( -
+
); -}; +} -QueryOperationAction.displayName = 'QueryOperationAction'; +interface QueryOperationActionProps extends BaseQueryOperationActionProps {} +export function QueryOperationAction(props: QueryOperationActionProps) { + return ; +} + +interface QueryOperationToggleActionProps extends BaseQueryOperationActionProps { + active: boolean; +} +export const QueryOperationToggleAction = (props: QueryOperationToggleActionProps) => { + return ; +}; const getStyles = (theme: GrafanaTheme2) => { return { diff --git a/public/app/features/dashboard/components/TransformationsEditor/TransformationOperationRow.tsx b/public/app/features/dashboard/components/TransformationsEditor/TransformationOperationRow.tsx index fce34b37f12..dd4cde96514 100644 --- a/public/app/features/dashboard/components/TransformationsEditor/TransformationOperationRow.tsx +++ b/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 ( {uiConfig.state && } - {showFilter && ( - )} - - onDisableToggle(index), 'disabled', disabled)} active={disabled} diff --git a/public/app/features/query/components/QueryEditorRow.tsx b/public/app/features/query/components/QueryEditorRow.tsx index 7127fef7677..36d3091649a 100644 --- a/public/app/features/query/components/QueryEditorRow.tsx +++ b/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 extends PureComponent { 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 ( {hasEditorHelp && ( - extends PureComponent {!hideDisableQuery ? ( - { 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();