RadioButton: Make description appear in a Tooltip component (#78010)

2024-12-24 08:00:08 -06:00 · 2023-11-27 13:54:07 +01:00 · 2023-11-27 13:54:07 +01:00 · e422a92eae
commit e422a92eae
parent 1c270b1dc2
10 changed files with 44 additions and 29 deletions
--- 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
--- 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...';
--- 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');
--- 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');
--- 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:
--- 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');
--- 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`,
  },
--- 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<HTMLInputElement, RadioButtonProps>(
  ) => {
    const styles = useStyles2(getRadioButtonStyles, size, fullWidth);

-    return (
-      <div className={styles.radioOption}>
-        <input
-          type="radio"
-          className={styles.radio}
-          onChange={onChange}
-          onClick={onClick}
-          disabled={disabled}
-          id={id}
-          checked={active}
-          name={name}
-          aria-label={ariaLabel || description}
-          ref={ref}
-        />
+    const inputRadioButton = (
+      <input
+        type="radio"
+        className={styles.radio}
+        onChange={onChange}
+        onClick={onClick}
+        disabled={disabled}
+        id={id}
+        checked={active}
+        name={name}
+        aria-label={ariaLabel}
+        ref={ref}
+      />
+    );
+    return description ? (
+      <div className={styles.radioOption} data-testid={selectors.components.RadioButton.container}>
+        <Tooltip content={description} placement="bottom">
+          {inputRadioButton}
+        </Tooltip>
+        <label className={styles.radioLabel} htmlFor={id} title={description || ariaLabel}>
+          {children}
+        </label>
+      </div>
+    ) : (
+      <div className={styles.radioOption} data-testid={selectors.components.RadioButton.container}>
+        {inputRadioButton}
        <label className={styles.radioLabel} htmlFor={id} title={description || ariaLabel}>
          {children}
        </label>
@ -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,
--- 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<T>({
    <div
      role="radiogroup"
      aria-label={ariaLabel}
-      className={cx(styles.radioGroup, fullWidth && styles.fullWidth, className)}
+      className={cx(styles.radioGroup, fullWidth && styles.fullWidth, invalid && styles.invalid, className)}
    >
      {options.map((opt, i) => {
        const isItemDisabled = disabledOptions && opt.value && disabledOptions.includes(opt.value);
--- 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<HTMLElement, TooltipProps>(
      [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 && (
          <Portal>