From e422a92eaec1edee5ae3a1631e7901f594468e51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Fern=C3=A1ndez?= Date: Mon, 27 Nov 2023 13:54:07 +0100 Subject: [PATCH] RadioButton: Make description appear in a Tooltip component (#78010) --- e2e/various-suite/exemplars.spec.ts | 2 +- e2e/various-suite/loki-editor.spec.ts | 2 +- e2e/various-suite/loki-query-builder.spec.ts | 3 +- e2e/various-suite/mysql.spec.ts | 3 +- e2e/various-suite/query-editor.spec.ts | 2 +- .../visualization-suggestions.spec.ts | 2 +- .../src/selectors/components.ts | 3 ++ .../Forms/RadioButtonGroup/RadioButton.tsx | 49 ++++++++++++------- .../RadioButtonGroup/RadioButtonGroup.tsx | 2 +- .../src/components/Tooltip/Tooltip.tsx | 5 +- 10 files changed, 44 insertions(+), 29 deletions(-) diff --git a/e2e/various-suite/exemplars.spec.ts b/e2e/various-suite/exemplars.spec.ts index 97da7f3aabd..62d1ce43ea8 100644 --- a/e2e/various-suite/exemplars.spec.ts +++ b/e2e/various-suite/exemplars.spec.ts @@ -55,7 +55,7 @@ describe('Exemplars', () => { cy.contains(dataSourceName).scrollIntoView().should('be.visible').click(); // Switch to code editor - cy.contains('label', 'Code').click(); + e2e.components.RadioButton.container().filter(':contains("Code")').click(); // we need to wait for the query-field being lazy-loaded, in two steps: // 1. first we wait for the text 'Loading...' to appear diff --git a/e2e/various-suite/loki-editor.spec.ts b/e2e/various-suite/loki-editor.spec.ts index 8ba7d4647f6..189c9d5a131 100644 --- a/e2e/various-suite/loki-editor.spec.ts +++ b/e2e/various-suite/loki-editor.spec.ts @@ -37,7 +37,7 @@ describe('Loki Query Editor', () => { e2e.components.DataSourcePicker.container().should('be.visible').click(); cy.contains(dataSourceName).scrollIntoView().should('be.visible').click(); - cy.contains('Code').click(); + e2e.components.RadioButton.container().filter(':contains("Code")').click(); // Wait for lazy loading const monacoLoadingText = 'Loading...'; diff --git a/e2e/various-suite/loki-query-builder.spec.ts b/e2e/various-suite/loki-query-builder.spec.ts index 348a4d92ab5..40141bb7c2b 100644 --- a/e2e/various-suite/loki-query-builder.spec.ts +++ b/e2e/various-suite/loki-query-builder.spec.ts @@ -78,7 +78,8 @@ describe('Loki query builder', () => { cy.contains(finalQuery).should('be.visible'); // Change to code editor - cy.contains('label', 'Code').click(); + e2e.components.RadioButton.container().filter(':contains("Code")').click(); + // We need to test this manually because the final query is split into separate DOM elements using cy.contains(finalQuery).should('be.visible'); does not detect the query. cy.contains('rate').should('be.visible'); cy.contains('instance1|instance2').should('be.visible'); diff --git a/e2e/various-suite/mysql.spec.ts b/e2e/various-suite/mysql.spec.ts index 336573d556a..99746b008ca 100644 --- a/e2e/various-suite/mysql.spec.ts +++ b/e2e/various-suite/mysql.spec.ts @@ -35,7 +35,8 @@ describe('MySQL datasource', () => { }); it('code editor autocomplete should handle table name escaping/quoting', () => { - cy.get("label[for^='option-code']").should('be.visible').click(); + e2e.components.RadioButton.container().filter(':contains("Code")').click(); + cy.get('textarea').type('S{downArrow}{enter}'); cy.wait('@tables'); cy.get('.suggest-widget').contains(tableNameWithSpecialCharacter).should('be.visible'); diff --git a/e2e/various-suite/query-editor.spec.ts b/e2e/various-suite/query-editor.spec.ts index de383f23940..2263f1ffacf 100644 --- a/e2e/various-suite/query-editor.spec.ts +++ b/e2e/various-suite/query-editor.spec.ts @@ -12,7 +12,7 @@ describe('Query editor', () => { cy.contains('gdev-prometheus').scrollIntoView().should('be.visible').click(); const queryText = `rate(http_requests_total{job="grafana"}[5m])`; - cy.contains('label', 'Code').click(); + e2e.components.RadioButton.container().filter(':contains("Code")').click(); // we need to wait for the query-field being lazy-loaded, in two steps: // it is a two-step process: diff --git a/e2e/various-suite/visualization-suggestions.spec.ts b/e2e/various-suite/visualization-suggestions.spec.ts index 100953980f5..aef309ea452 100644 --- a/e2e/various-suite/visualization-suggestions.spec.ts +++ b/e2e/various-suite/visualization-suggestions.spec.ts @@ -10,7 +10,7 @@ describe('Visualization suggestions', () => { // Try visualization suggestions e2e.components.PanelEditor.toggleVizPicker().click(); - cy.contains('Suggestions').click(); + e2e.components.RadioButton.container().filter(':contains("Suggestions")').click(); // Verify we see suggestions e2e.components.VisualizationPreview.card('Line chart').should('be.visible'); diff --git a/packages/grafana-e2e-selectors/src/selectors/components.ts b/packages/grafana-e2e-selectors/src/selectors/components.ts index 8e3a87ef3bf..f692860c395 100644 --- a/packages/grafana-e2e-selectors/src/selectors/components.ts +++ b/packages/grafana-e2e-selectors/src/selectors/components.ts @@ -10,6 +10,9 @@ * @alpha */ export const Components = { + RadioButton: { + container: 'data-testid radio-button', + }, Breadcrumbs: { breadcrumb: (title: string) => `data-testid ${title} breadcrumb`, }, diff --git a/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx b/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx index 9360b560686..b3318923934 100644 --- a/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx +++ b/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx @@ -2,10 +2,11 @@ import { css } from '@emotion/css'; import React from 'react'; import { GrafanaTheme2 } from '@grafana/data'; -import { StringSelector } from '@grafana/e2e-selectors'; +import { StringSelector, selectors } from '@grafana/e2e-selectors'; import { useStyles2 } from '../../../themes'; import { getFocusStyles, getMouseFocusStyles } from '../../../themes/mixins'; +import { Tooltip } from '../../Tooltip/Tooltip'; import { getPropertiesForButtonSize } from '../commonStyles'; export type RadioButtonSize = 'sm' | 'md'; @@ -43,20 +44,32 @@ export const RadioButton = React.forwardRef( ) => { const styles = useStyles2(getRadioButtonStyles, size, fullWidth); - return ( -
- + const inputRadioButton = ( + + ); + return description ? ( +
+ + {inputRadioButton} + + +
+ ) : ( +
+ {inputRadioButton} @@ -86,15 +99,16 @@ const getRadioButtonStyles = (theme: GrafanaTheme2, size: RadioButtonSize, fullW radio: css({ position: 'absolute', opacity: 0, - zIndex: -1000, + zIndex: 2, width: '100% !important', height: '100%', + cursor: 'pointer', '&:checked + label': { color: theme.colors.text.primary, fontWeight: theme.typography.fontWeightMedium, background: theme.colors.action.selected, - zIndex: 3, + zIndex: 1, }, '&:focus + label, &:focus-visible + label': getFocusStyles(theme), @@ -119,7 +133,6 @@ const getRadioButtonStyles = (theme: GrafanaTheme2, size: RadioButtonSize, fullW borderRadius: theme.shape.radius.default, background: theme.colors.background.primary, cursor: 'pointer', - zIndex: 1, userSelect: 'none', whiteSpace: 'nowrap', flexGrow: 1, diff --git a/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButtonGroup.tsx b/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButtonGroup.tsx index 8c5153180d4..1ac45002e6a 100644 --- a/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButtonGroup.tsx +++ b/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButtonGroup.tsx @@ -76,7 +76,7 @@ export function RadioButtonGroup({
{options.map((opt, i) => { const isItemDisabled = disabledOptions && opt.value && disabledOptions.includes(opt.value); diff --git a/packages/grafana-ui/src/components/Tooltip/Tooltip.tsx b/packages/grafana-ui/src/components/Tooltip/Tooltip.tsx index b3060829095..1cc90e3cc58 100644 --- a/packages/grafana-ui/src/components/Tooltip/Tooltip.tsx +++ b/packages/grafana-ui/src/components/Tooltip/Tooltip.tsx @@ -81,15 +81,12 @@ export const Tooltip = React.forwardRef( [forwardedRef, setTriggerRef] ); - // if the child has an aria-label, this should take precedence over the tooltip content - const childHasAriaLabel = 'aria-label' in children.props; - return ( <> {React.cloneElement(children, { ref: handleRef, tabIndex: 0, // tooltip trigger should be keyboard focusable - 'aria-describedby': !childHasAriaLabel && visible ? tooltipId : undefined, + 'aria-describedby': visible ? tooltipId : undefined, })} {visible && (