From 1021f05e32f1f9b7a4a75d0752ac52643060ae54 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Tue, 5 Aug 2025 12:04:35 +0100 Subject: [PATCH] Accessibility: Make either `tooltip` or `aria-label` required when no button children present (#109131) * ensure button must have tooltip/aria-label when no children * fix violations * restore some unnecessarily removed labels * use types instead of interfaces as before * further fixes * fix unit test * commit translations and fix unit test * upgrade plugin-ui to 0.10.8 * set aria-label + unit tests --- jest.config.js | 2 +- package.json | 2 +- .../grafana-o11y-ds-frontend/package.json | 2 +- .../TraceToMetrics/TraceToMetricsSettings.tsx | 2 +- packages/grafana-prometheus/package.json | 2 +- .../src/configuration/ExemplarSetting.tsx | 2 +- .../querybuilder/shared/OperationEditor.tsx | 2 +- .../querybuilder/shared/OperationHeader.tsx | 7 ++- .../shared/OperationInfoButton.tsx | 4 +- .../shared/OperationList.test.tsx | 2 +- .../shared/OperationParamEditorRegistry.tsx | 2 +- packages/grafana-sql/package.json | 2 +- .../AwesomeQueryBuilder.tsx | 2 +- .../visual-query-builder/GroupByRow.tsx | 2 +- .../SelectCustomFunctionParameters.tsx | 4 +- .../visual-query-builder/SelectRow.tsx | 4 +- .../src/components/Actions/ActionButton.tsx | 2 +- .../src/components/Button/Button.test.tsx | 22 ++++++++ .../src/components/Button/Button.tsx | 30 ++++++++--- .../ButtonCascader/ButtonCascader.tsx | 4 +- .../ClipboardButton/ClipboardButton.story.tsx | 4 +- .../ClipboardButton/ClipboardButton.tsx | 5 +- .../components/DataLinks/DataLinkButton.tsx | 2 +- .../Table/TableNG/Filter/FilterPopup.tsx | 1 + .../PageActionBar/PageActionBar.tsx | 5 +- .../unified/components/MoreButton.tsx | 2 +- .../ReceiverMetadataBadge.tsx | 10 +++- .../components/rule-viewer/RuleViewer.tsx | 9 +++- .../unified/components/rules/ActionButton.tsx | 8 ++- .../unified/components/rules/ActionIcon.tsx | 54 +++++++++---------- .../alerting/unified/group-details/Title.tsx | 8 ++- .../unified/rule-list/RuleListPageTitle.tsx | 10 ++-- .../panel-edit/PanelVizTypePicker.tsx | 2 +- .../PanelEditor/OptionsPaneCategory.tsx | 6 +++ .../dimensions/editors/ResourcePicker.tsx | 13 ++++- .../editors/TextDimensionEditor.tsx | 11 +++- .../ContentOutlineItemButton.tsx | 4 ++ .../explore/Logs/LogsVolumePanelList.tsx | 9 +++- .../TracePageHeader/TracePageHeader.tsx | 8 ++- .../TraceView/components/common/CopyIcon.tsx | 8 ++- .../logs/components/LogDetailsRow.test.tsx | 2 +- .../logs/components/LogDetailsRow.tsx | 2 +- .../components/panel/LogLineDetailsFields.tsx | 2 +- .../provisioning/Shared/CodeBlock.tsx | 10 +++- .../features/query/components/QueryGroup.tsx | 2 +- .../FilterByValueFilterEditor.tsx | 7 ++- .../LogsQueryBuilder/FilterItem.tsx | 7 ++- .../LogsQueryBuilder/FilterSection.tsx | 8 ++- .../LogsQueryBuilder/FuzzySearch.tsx | 14 ++++- .../LogsQueryBuilder/GroupBySection.tsx | 1 + .../LogsQueryBuilder/OrderBySection.tsx | 15 +++++- .../LogsQueryBuilder/TableSection.tsx | 7 ++- .../grafana-azure-monitor-datasource.json | 11 +++- .../datasource/azuremonitor/package.json | 2 +- .../datasource/cloud-monitoring/package.json | 2 +- .../elasticsearch/configuration/DataLink.tsx | 2 +- .../package.json | 2 +- .../components/CSVWaveEditor.tsx | 7 ++- .../plugins/datasource/jaeger/package.json | 2 +- .../loki/components/LokiContextUi.tsx | 51 +++++++++--------- .../loki/configuration/DerivedField.tsx | 2 +- .../loki/configuration/DerivedFields.test.tsx | 2 +- .../app/plugins/datasource/mssql/package.json | 2 +- .../app/plugins/datasource/mysql/package.json | 2 +- .../app/plugins/datasource/tempo/package.json | 2 +- .../plugins/datasource/zipkin/package.json | 2 +- .../geomap/components/MeasureOverlay.tsx | 9 +++- .../panel/nodeGraph/NodeGraph.test.tsx | 4 +- .../plugins/panel/nodeGraph/ViewControls.tsx | 4 +- .../nodeGraph/editor/ArcOptionsEditor.tsx | 2 +- public/locales/en-US/grafana.json | 27 +++++++++- yarn.lock | 32 +++++------ 72 files changed, 367 insertions(+), 159 deletions(-) diff --git a/jest.config.js b/jest.config.js index a576f4ec0c4..ef107c8cc24 100644 --- a/jest.config.js +++ b/jest.config.js @@ -41,7 +41,7 @@ module.exports = { verbose: false, testEnvironment: 'jsdom', transform: { - '^.+\\.(ts|tsx|js|jsx)$': [require.resolve('ts-jest'), { isolatedModules: true }], + '^.+\\.(ts|tsx|js|jsx)$': [require.resolve('ts-jest')], }, transformIgnorePatterns: [ `/node_modules/(?!${esModules})`, // exclude es modules to prevent TS complaining diff --git a/package.json b/package.json index 71c62b5c442..5a2b36d0472 100644 --- a/package.json +++ b/package.json @@ -287,7 +287,7 @@ "@grafana/llm": "0.22.1", "@grafana/monaco-logql": "^0.0.8", "@grafana/o11y-ds-frontend": "workspace:*", - "@grafana/plugin-ui": "0.10.7", + "@grafana/plugin-ui": "0.10.8", "@grafana/prometheus": "workspace:*", "@grafana/runtime": "workspace:*", "@grafana/scenes": "6.29.1", diff --git a/packages/grafana-o11y-ds-frontend/package.json b/packages/grafana-o11y-ds-frontend/package.json index aee2372aa14..85c25758adc 100644 --- a/packages/grafana-o11y-ds-frontend/package.json +++ b/packages/grafana-o11y-ds-frontend/package.json @@ -20,7 +20,7 @@ "@emotion/css": "11.13.5", "@grafana/data": "12.2.0-pre", "@grafana/e2e-selectors": "12.2.0-pre", - "@grafana/plugin-ui": "0.10.7", + "@grafana/plugin-ui": "0.10.8", "@grafana/runtime": "12.2.0-pre", "@grafana/schema": "12.2.0-pre", "@grafana/ui": "12.2.0-pre", diff --git a/packages/grafana-o11y-ds-frontend/src/TraceToMetrics/TraceToMetricsSettings.tsx b/packages/grafana-o11y-ds-frontend/src/TraceToMetrics/TraceToMetricsSettings.tsx index 966c1703b54..fbb68ab9a51 100644 --- a/packages/grafana-o11y-ds-frontend/src/TraceToMetrics/TraceToMetricsSettings.tsx +++ b/packages/grafana-o11y-ds-frontend/src/TraceToMetrics/TraceToMetricsSettings.tsx @@ -162,7 +162,7 @@ export function TraceToMetricsSettings({ options, onOptionsChange }: Props) { ); + expect(screen.queryByRole('button', { name: 'Tooltip text' })).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Child text' })).toBeInTheDocument(); + }); + + it('should prioritise the aria-label if it is present', () => { + setup( + + ); + expect(screen.queryByRole('button', { name: 'Child text' })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Tooltip text' })).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Aria label' })).toBeInTheDocument(); + }); }); diff --git a/packages/grafana-ui/src/components/Button/Button.tsx b/packages/grafana-ui/src/components/Button/Button.tsx index f9500fc196d..83b1c5825c0 100644 --- a/packages/grafana-ui/src/components/Button/Button.tsx +++ b/packages/grafana-ui/src/components/Button/Button.tsx @@ -18,28 +18,43 @@ export const allButtonVariants: ButtonVariant[] = ['primary', 'secondary', 'dest export type ButtonFill = 'solid' | 'outline' | 'text'; export const allButtonFills: ButtonFill[] = ['solid', 'outline', 'text']; -type CommonProps = { +type BaseProps = { size?: ComponentSize; variant?: ButtonVariant; fill?: ButtonFill; icon?: IconName | React.ReactElement; className?: string; - children?: React.ReactNode; fullWidth?: boolean; type?: string; - /** Tooltip content to display on hover */ tooltip?: PopoverContent; - /** Position of the tooltip */ tooltipPlacement?: TooltipPlacement; /** Position of the icon */ iconPlacement?: 'left' | 'right'; }; +// either aria-label or tooltip is required for buttons without children +type NoChildrenAriaLabel = BaseProps & { + children?: never; + 'aria-label': string; +}; +type NoChildrenTooltip = BaseProps & { + children?: never; + tooltip: PopoverContent; + tooltipPlacement?: TooltipPlacement; +}; + +type BasePropsWithChildren = BaseProps & { + children: React.ReactNode; +}; + +type CommonProps = BasePropsWithChildren | NoChildrenTooltip | NoChildrenAriaLabel; + export type ButtonProps = CommonProps & ButtonHTMLAttributes; export const Button = React.forwardRef( ( { + 'aria-label': ariaLabel, variant = 'primary', size = 'md', fill = 'solid', @@ -92,6 +107,7 @@ export const Button = React.forwardRef( aria-disabled={hasTooltip && disabled} disabled={!hasTooltip && disabled} ref={tooltip ? undefined : ref} + aria-label={ariaLabel ?? (!children && typeof tooltip === 'string' ? tooltip : undefined)} > {iconPlacement === 'left' && iconComponent} {children && {children}} @@ -113,13 +129,12 @@ export const Button = React.forwardRef( Button.displayName = 'Button'; -export type ButtonLinkProps = CommonProps & - ButtonHTMLAttributes & - AnchorHTMLAttributes; +export type ButtonLinkProps = ButtonProps & Omit, 'aria-label'>; export const LinkButton = React.forwardRef( ( { + 'aria-label': ariaLabel, variant = 'primary', size = 'md', fill = 'solid', @@ -164,6 +179,7 @@ export const LinkButton = React.forwardRef( tabIndex={disabled ? -1 : 0} aria-disabled={disabled} ref={tooltip ? undefined : ref} + aria-label={ariaLabel ?? (!children && typeof tooltip === 'string' ? tooltip : undefined)} > {children && {children}} diff --git a/packages/grafana-ui/src/components/ButtonCascader/ButtonCascader.tsx b/packages/grafana-ui/src/components/ButtonCascader/ButtonCascader.tsx index 6e6ba1fd725..43aaff2bee2 100644 --- a/packages/grafana-ui/src/components/ButtonCascader/ButtonCascader.tsx +++ b/packages/grafana-ui/src/components/ButtonCascader/ButtonCascader.tsx @@ -14,7 +14,7 @@ import { Icon } from '../Icon/Icon'; export interface ButtonCascaderProps { options: CascaderOption[]; - children?: string; + children: string; icon?: IconName; disabled?: boolean; value?: string[]; @@ -24,7 +24,7 @@ export interface ButtonCascaderProps { onPopupVisibleChange?: (visible: boolean) => void; className?: string; variant?: ButtonProps['variant']; - buttonProps?: ButtonProps; + buttonProps?: Omit; hideDownIcon?: boolean; } diff --git a/packages/grafana-ui/src/components/ClipboardButton/ClipboardButton.story.tsx b/packages/grafana-ui/src/components/ClipboardButton/ClipboardButton.story.tsx index 64c0489590a..588eb0af231 100644 --- a/packages/grafana-ui/src/components/ClipboardButton/ClipboardButton.story.tsx +++ b/packages/grafana-ui/src/components/ClipboardButton/ClipboardButton.story.tsx @@ -19,10 +19,10 @@ const meta: Meta = { }, }; -interface StoryProps extends Partial { +type StoryProps = Partial & { inputText: string; buttonText: string; -} +}; export const ClipboardButton: StoryFn = (args) => { const shareUrl = 'https://grafana.com/d/abcDEF-34t'; diff --git a/packages/grafana-ui/src/components/ClipboardButton/ClipboardButton.tsx b/packages/grafana-ui/src/components/ClipboardButton/ClipboardButton.tsx index dc8bf238ed8..a42871dcde0 100644 --- a/packages/grafana-ui/src/components/ClipboardButton/ClipboardButton.tsx +++ b/packages/grafana-ui/src/components/ClipboardButton/ClipboardButton.tsx @@ -10,14 +10,14 @@ import { Button, ButtonProps } from '../Button/Button'; import { Icon } from '../Icon/Icon'; import { InlineToast } from '../InlineToast/InlineToast'; -export interface Props extends ButtonProps { +export type Props = ButtonProps & { /** A function that returns text to be copied */ getText(): string; /** Callback when the text has been successfully copied */ onClipboardCopy?(copiedText: string): void; /** Callback when there was an error copying the text */ onClipboardError?(copiedText: string, error: unknown): void; -} +}; const SHOW_SUCCESS_DURATION = 2 * 1000; @@ -73,7 +73,6 @@ export function ClipboardButton({ onClick={copyTextCallback} icon={icon} variant={showCopySuccess ? 'success' : variant} - aria-label={showCopySuccess ? copiedText : undefined} {...buttonProps} className={cx(styles.button, showCopySuccess && styles.successButton, buttonProps.className)} ref={buttonRef} diff --git a/packages/grafana-ui/src/components/DataLinks/DataLinkButton.tsx b/packages/grafana-ui/src/components/DataLinks/DataLinkButton.tsx index 44ea08e36ac..4338129f4a9 100644 --- a/packages/grafana-ui/src/components/DataLinks/DataLinkButton.tsx +++ b/packages/grafana-ui/src/components/DataLinks/DataLinkButton.tsx @@ -4,7 +4,7 @@ import { ButtonProps, Button } from '../Button/Button'; type DataLinkButtonProps = { link: LinkModel; - buttonProps?: ButtonProps; + buttonProps?: Omit; }; /** diff --git a/packages/grafana-ui/src/components/Table/TableNG/Filter/FilterPopup.tsx b/packages/grafana-ui/src/components/Table/TableNG/Filter/FilterPopup.tsx index 3d3103be741..d55da76386e 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/Filter/FilterPopup.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/Filter/FilterPopup.tsx @@ -135,6 +135,7 @@ export const FilterPopup = ({ value={searchFilter} /> + ); }; export const getStyle = (theme: GrafanaTheme2) => ({ diff --git a/public/app/features/alerting/unified/components/rules/ActionIcon.tsx b/public/app/features/alerting/unified/components/rules/ActionIcon.tsx index 5a3dbcc9870..0d0a67395ed 100644 --- a/public/app/features/alerting/unified/components/rules/ActionIcon.tsx +++ b/public/app/features/alerting/unified/components/rules/ActionIcon.tsx @@ -27,32 +27,32 @@ export const ActionIcon = ({ }: Props) => { const ariaLabel = typeof tooltip === 'string' ? tooltip : undefined; - return ( - - {to ? ( - - ) : ( - + {onRemove && (