From 1441c90178a8699fcbaff2b2580a7618f32b97f0 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Thu, 22 Jun 2023 09:31:55 +0100 Subject: [PATCH] Accessibility: use `Collapse` in `QueryOptionGroup` (#70446) * use collapse in QueryOptionGroup * fix unit tests * fix tests after merging main * adjust padding to match --- .../LokiQueryBuilderOptions.test.tsx | 20 +++--- .../PromQueryBuilderOptions.test.tsx | 10 +-- .../querybuilder/shared/QueryOptionGroup.tsx | 70 ++++++++----------- 3 files changed, 46 insertions(+), 54 deletions(-) diff --git a/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.test.tsx b/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.test.tsx index b8f400459ca..ab194e91d43 100644 --- a/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.test.tsx +++ b/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.test.tsx @@ -10,7 +10,7 @@ describe('LokiQueryBuilderOptions', () => { it('can change query type', async () => { const { props } = setup(); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); expect(screen.getByLabelText('Range')).toBeChecked(); await userEvent.click(screen.getByLabelText('Instant')); @@ -24,7 +24,7 @@ describe('LokiQueryBuilderOptions', () => { it('can change legend format', async () => { const { props } = setup(); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); // First autosize input is a Legend const element = screen.getAllByTestId('autosize-input')[0]; @@ -40,7 +40,7 @@ describe('LokiQueryBuilderOptions', () => { it('can change line limit to valid value', async () => { const { props } = setup({ expr: '{foo="bar"}' }); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); // Second autosize input is a Line limit const element = screen.getAllByTestId('autosize-input')[1]; await userEvent.type(element, '10'); @@ -57,7 +57,7 @@ describe('LokiQueryBuilderOptions', () => { // We need to start with some value to be able to change it props.query.maxLines = 10; - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); // Second autosize input is a Line limit const element = screen.getAllByTestId('autosize-input')[1]; await userEvent.type(element, '-10'); @@ -74,7 +74,7 @@ describe('LokiQueryBuilderOptions', () => { // We need to start with some value to be able to change it props.query.maxLines = 10; - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); // Second autosize input is a Line limit const element = screen.getAllByTestId('autosize-input')[1]; await userEvent.type(element, 'asd'); @@ -102,19 +102,19 @@ describe('LokiQueryBuilderOptions', () => { it('does not shows resolution field if resolution is not set', async () => { setup({ expr: 'rate({foo="bar"}[5m]' }); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); expect(screen.queryByText('Resolution')).not.toBeInTheDocument(); }); it('does not shows resolution field if resolution is set to default value 1', async () => { setup({ expr: 'rate({foo="bar"}[5m]', resolution: 1 }); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); expect(screen.queryByText('Resolution')).not.toBeInTheDocument(); }); it('does shows resolution field with warning if resolution is set to non-default value', async () => { setup({ expr: 'rate({foo="bar"}[5m]', resolution: 2 }); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); expect(screen.getByText('Resolution')).toBeInTheDocument(); expect( screen.getByText("The 'Resolution' is deprecated. Use 'Step' editor instead to change step parameter.") @@ -130,13 +130,13 @@ describe('LokiQueryBuilderOptions', () => { it('shows error when invalid value in step', async () => { setup({ expr: 'rate({foo="bar"}[5m]', step: 'a' }); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); expect(screen.getByText(/Invalid step/)).toBeInTheDocument(); }); it('does not shows error when valid value in step', async () => { setup({ expr: 'rate({foo="bar"}[5m]', step: '1m' }); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); expect(screen.queryByText(/Invalid step/)).not.toBeInTheDocument(); }); }); diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderOptions.test.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderOptions.test.tsx index 9270f32c6a6..4f9e1384980 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderOptions.test.tsx +++ b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryBuilderOptions.test.tsx @@ -14,7 +14,7 @@ describe('PromQueryBuilderOptions', () => { it('Can change query type', async () => { const { props } = setup(); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); expect(screen.getByLabelText('Range')).toBeChecked(); await userEvent.click(screen.getByLabelText('Instant')); @@ -30,7 +30,7 @@ describe('PromQueryBuilderOptions', () => { it('Can set query type to "Both" on render for PanelEditor', async () => { setup({ instant: true, range: true }); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); expect(screen.getByLabelText('Both')).toBeChecked(); }); @@ -38,7 +38,7 @@ describe('PromQueryBuilderOptions', () => { it('Can set query type to "Both" on render for Explorer', async () => { setup({ instant: true, range: true }, CoreApp.Explore); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); expect(screen.getByLabelText('Both')).toBeChecked(); }); @@ -51,7 +51,7 @@ describe('PromQueryBuilderOptions', () => { it('Can change legend format to verbose', async () => { const { props } = setup(); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); let legendModeSelect = screen.getByText('Auto').parentElement!; await userEvent.click(legendModeSelect); @@ -67,7 +67,7 @@ describe('PromQueryBuilderOptions', () => { it('Can change legend format to custom', async () => { const { props } = setup(); - await userEvent.click(screen.getByTitle('Click to edit options')); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); let legendModeSelect = screen.getByText('Auto').parentElement!; await userEvent.click(legendModeSelect); diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryOptionGroup.tsx b/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryOptionGroup.tsx index fe6fd638ec4..d88ff69ff39 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryOptionGroup.tsx +++ b/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryOptionGroup.tsx @@ -4,7 +4,7 @@ import { useToggle } from 'react-use'; import { getValueFormat, GrafanaTheme2 } from '@grafana/data'; import { Stack } from '@grafana/experimental'; -import { Icon, useStyles2 } from '@grafana/ui'; +import { Collapse, useStyles2 } from '@grafana/ui'; import { QueryStats } from 'app/plugins/datasource/loki/types'; export interface Props { @@ -25,22 +25,26 @@ export function QueryOptionGroup({ title, children, collapsedInfo, queryStats }: return (
- -
-
- -
-
{title}
- {!isOpen && ( -
- {collapsedInfo.map((x, i) => ( - {x} - ))} -
- )} -
- {isOpen &&
{children}
} - + +
{title}
+ {!isOpen && ( +
+ {collapsedInfo.map((x, i) => ( + {x} + ))} +
+ )} + + } + > +
{children}
+ {queryStats &&

This query will process approximately {convertUnits()}.

}
); @@ -48,29 +52,21 @@ export function QueryOptionGroup({ title, children, collapsedInfo, queryStats }: const getStyles = (theme: GrafanaTheme2) => { return { + collapse: css({ + backgroundColor: 'unset', + border: 'unset', + marginBottom: 0, + + ['> button']: { + padding: theme.spacing(0, 1), + }, + }), wrapper: css({ width: '100%', display: 'flex', justifyContent: 'space-between', alignItems: 'baseline', }), - switchLabel: css({ - color: theme.colors.text.secondary, - cursor: 'pointer', - fontSize: theme.typography.bodySmall.fontSize, - '&:hover': { - color: theme.colors.text.primary, - }, - }), - header: css({ - display: 'flex', - cursor: 'pointer', - alignItems: 'baseline', - color: theme.colors.text.primary, - '&:hover': { - background: theme.colors.emphasize(theme.colors.background.primary, 0.03), - }, - }), title: css({ flexGrow: 1, overflow: 'hidden', @@ -81,20 +77,16 @@ const getStyles = (theme: GrafanaTheme2) => { description: css({ color: theme.colors.text.secondary, fontSize: theme.typography.bodySmall.fontSize, + fontWeight: theme.typography.bodySmall.fontWeight, paddingLeft: theme.spacing(2), gap: theme.spacing(2), display: 'flex', }), body: css({ display: 'flex', - paddingTop: theme.spacing(2), gap: theme.spacing(2), flexWrap: 'wrap', }), - toggle: css({ - color: theme.colors.text.secondary, - marginRight: `${theme.spacing(1)}`, - }), stats: css({ margin: '0px', color: theme.colors.text.secondary,