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
This commit is contained in:
Ashley Harrison 2023-01-30 13:32:16 +00:00 committed by GitHub
parent 8593ca245a
commit f19b07c0bc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 15 additions and 13 deletions

View File

@ -32,7 +32,7 @@ describe('QueryOperationRow', () => {
const onCloseSpy = jest.fn(); const onCloseSpy = jest.fn();
setup({ isOpen: false, onOpen: onOpenSpy, onClose: onCloseSpy }); setup({ isOpen: false, onOpen: onOpenSpy, onClose: onCloseSpy });
const queryRow = screen.getByText(/^test-title$/); const queryRow = screen.getByRole('button', { name: /^test-title$/ });
expect(queryRow).toBeInTheDocument(); expect(queryRow).toBeInTheDocument();
// open row on click // open row on click

View File

@ -90,7 +90,6 @@ export function QueryOperationRow({
}, },
}; };
const titleElement = title && ReactUtils.renderOrCallToRender(title, renderPropArgs);
const actionsElement = actions && ReactUtils.renderOrCallToRender(actions, renderPropArgs); const actionsElement = actions && ReactUtils.renderOrCallToRender(actions, renderPropArgs);
const headerElementRendered = headerElement && ReactUtils.renderOrCallToRender(headerElement, renderPropArgs); const headerElementRendered = headerElement && ReactUtils.renderOrCallToRender(headerElement, renderPropArgs);
@ -112,7 +111,7 @@ export function QueryOperationRow({
isContentVisible={isContentVisible} isContentVisible={isContentVisible}
onRowToggle={onRowToggle} onRowToggle={onRowToggle}
reportDragMousePosition={reportDragMousePosition} reportDragMousePosition={reportDragMousePosition}
titleElement={titleElement} title={title}
/> />
</div> </div>
{isContentVisible && <div className={styles.content}>{children}</div>} {isContentVisible && <div className={styles.content}>{children}</div>}
@ -135,7 +134,7 @@ export function QueryOperationRow({
isContentVisible={isContentVisible} isContentVisible={isContentVisible}
onRowToggle={onRowToggle} onRowToggle={onRowToggle}
reportDragMousePosition={reportDragMousePosition} reportDragMousePosition={reportDragMousePosition}
titleElement={titleElement} title={title}
/> />
{isContentVisible && <div className={styles.content}>{children}</div>} {isContentVisible && <div className={styles.content}>{children}</div>}
</div> </div>

View File

@ -1,4 +1,5 @@
import { css, cx } from '@emotion/css'; import { css, cx } from '@emotion/css';
import { useId } from '@react-aria/utils';
import React, { MouseEventHandler } from 'react'; import React, { MouseEventHandler } from 'react';
import { DraggableProvidedDragHandleProps } from 'react-beautiful-dnd'; import { DraggableProvidedDragHandleProps } from 'react-beautiful-dnd';
@ -14,7 +15,7 @@ interface QueryOperationRowHeaderProps {
isContentVisible: boolean; isContentVisible: boolean;
onRowToggle: () => void; onRowToggle: () => void;
reportDragMousePosition: MouseEventHandler<HTMLDivElement>; reportDragMousePosition: MouseEventHandler<HTMLDivElement>;
titleElement?: React.ReactNode; title?: string;
id: string; id: string;
} }
@ -27,10 +28,11 @@ export const QueryOperationRowHeader: React.FC<QueryOperationRowHeaderProps> = (
isContentVisible, isContentVisible,
onRowToggle, onRowToggle,
reportDragMousePosition, reportDragMousePosition,
titleElement, title,
id, id,
}: QueryOperationRowHeaderProps) => { }: QueryOperationRowHeaderProps) => {
const styles = useStyles2(getStyles); const styles = useStyles2(getStyles);
const titleId = useId();
return ( return (
<div className={styles.header}> <div className={styles.header}>
@ -44,10 +46,13 @@ export const QueryOperationRowHeader: React.FC<QueryOperationRowHeaderProps> = (
type="button" type="button"
aria-expanded={isContentVisible} aria-expanded={isContentVisible}
aria-controls={id} aria-controls={id}
aria-labelledby={titleId}
/> />
{titleElement && ( {title && (
<div className={styles.titleWrapper} onClick={onRowToggle} aria-label="Query operation row title"> <div className={styles.titleWrapper}>
<div className={cx(styles.title, disabled && styles.disabled)}>{titleElement}</div> <div id={titleId} className={cx(styles.title, disabled && styles.disabled)}>
{title}
</div>
</div> </div>
)} )}
{headerElement} {headerElement}
@ -109,13 +114,11 @@ const getStyles = (theme: GrafanaTheme2) => ({
display: flex; display: flex;
align-items: center; align-items: center;
flex-grow: 1; flex-grow: 1;
cursor: pointer;
overflow: hidden; overflow: hidden;
margin-right: ${theme.spacing(0.5)}; margin-right: ${theme.spacing(0.5)};
`, `,
title: css` title: css`
font-weight: ${theme.typography.fontWeightBold}; font-weight: ${theme.typography.fontWeightBold};
color: ${theme.colors.text.link};
margin-left: ${theme.spacing(0.5)}; margin-left: ${theme.spacing(0.5)};
overflow: hidden; overflow: hidden;
text-overflow: ellipsis; text-overflow: ellipsis;

View File

@ -50,14 +50,14 @@ describe('InspectDataTab', () => {
}); });
it('should show available options', async () => { it('should show available options', async () => {
render(<InspectDataTab {...createProps()} />); render(<InspectDataTab {...createProps()} />);
const dataOptions = screen.getByText(/Data options/i); const dataOptions = screen.getByRole('button', { name: /Data options/i });
await userEvent.click(dataOptions); await userEvent.click(dataOptions);
expect(screen.getByText(/Show data frame/i)).toBeInTheDocument(); expect(screen.getByText(/Show data frame/i)).toBeInTheDocument();
expect(screen.getByText(/Download for Excel/i)).toBeInTheDocument(); expect(screen.getByText(/Download for Excel/i)).toBeInTheDocument();
}); });
it('should show available dataFrame options', async () => { it('should show available dataFrame options', async () => {
render(<InspectDataTab {...createProps()} />); render(<InspectDataTab {...createProps()} />);
const dataOptions = screen.getByText(/Data options/i); const dataOptions = screen.getByRole('button', { name: /Data options/i });
await userEvent.click(dataOptions); await userEvent.click(dataOptions);
const dataFrameInput = screen.getByRole('combobox', { name: /Select dataframe/i }); const dataFrameInput = screen.getByRole('combobox', { name: /Select dataframe/i });
await userEvent.click(dataFrameInput); await userEvent.click(dataFrameInput);