A11y: Fix toggletip predictable focus for keyboard users (#72100)

* Updated toggletip to use strategy fixed, added tests and temporary story so it can easily be checked * Added FocusScope restoreFocus and appropriate tests to toggletip * Open toggletip in test for making sure focus remains when using escape * Add aria-expanded to toggletip toggle child * Added to temp story for Toggletip * Remove focusScope for toggletip and handle focus restoration manually * Remove toggletip temp story and add toggletip long content story --------- Co-authored-by: joshhunt <josh@trtr.co>
2024-11-27 11:20:27 -06:00 · 2023-08-25 16:00:58 +01:00 · 2023-08-25 16:00:58 +01:00 · 31e29de024
commit 31e29de024
parent cb040a72bd
4 changed files with 217 additions and 50 deletions
--- a/packages/grafana-ui/src/components/Toggletip/Toggletip.mdx
+++ b/packages/grafana-ui/src/components/Toggletip/Toggletip.mdx
@ -64,7 +64,7 @@ function onClose() {
 }

 return (
-  <Toogletip
+  <Toggletip
    content="Toggletip body"
    title="This is the title of the Toggletip"
    footer="Toggletip footer text"
@ -72,7 +72,7 @@ return (
    onClose={onClose}
  >
    <IconButton name="question-circle" tooltip="IconButton containing a Toggletip" />
-  </Toogletip>
+  </Toggletip>
 );
 ```

--- a/packages/grafana-ui/src/components/Toggletip/Toggletip.story.tsx
+++ b/packages/grafana-ui/src/components/Toggletip/Toggletip.story.tsx
@ -5,6 +5,7 @@ import { SelectableValue } from '@grafana/data';

 import { withCenteredStory } from '../../utils/storybook/withCenteredStory';
 import { Button } from '../Button';
+import { CustomScrollbar } from '../CustomScrollbar/CustomScrollbar';
 import { ButtonSelect } from '../Dropdown/ButtonSelect';
 import { InlineField } from '../Forms/InlineField';
 import { Icon } from '../Icon/Icon';
@ -175,4 +176,54 @@ HostingMultiElements.args = {
  theme: 'info',
 };

+export const LongContent: StoryFn<typeof Toggletip> = ({
+  title,
+  content,
+  footer,
+  theme,
+  closeButton,
+  placement,
+  ...args
+}) => {
+  return (
+    <Toggletip
+      title={<h2>Toggletip with scrollable content and no interactive controls</h2>}
+      content={
+        <CustomScrollbar autoHeightMax="500px">
+          {/* one of the few documented cases we can turn this rule off */}
+          {/* https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/docs/rules/no-noninteractive-tabindex.md#case-shouldnt-i-add-a-tabindex-so-that-users-can-navigate-to-this-item */}
+          {/* eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex */}
+          <div tabIndex={0}>
+            <p>
+              If for any reason you have to use a Toggletip with a lot of content with no interactive controls, set a{' '}
+              <code>tabIndex=0</code> attribute to the container so keyboard users are able to focus the content and
+              able to scroll up and down it.
+            </p>
+            {new Array(15).fill(undefined).map((_, i) => (
+              <p key={i}>This is some content repeated over and over again to ensure it is scrollable.</p>
+            ))}
+          </div>
+        </CustomScrollbar>
+      }
+      footer={footer}
+      theme={theme}
+      placement={placement}
+      {...args}
+    >
+      <Button>Click to show Toggletip with long content!</Button>
+    </Toggletip>
+  );
+};
+LongContent.args = {
+  placement: 'auto',
+  theme: 'info',
+};
+
+LongContent.parameters = {
+  controls: {
+    hideNoControlsWarning: true,
+    exclude: ['title', 'content', 'children'],
+  },
+};
+
 export default meta;
--- a/packages/grafana-ui/src/components/Toggletip/Toggletip.test.tsx
+++ b/packages/grafana-ui/src/components/Toggletip/Toggletip.test.tsx
@ -1,4 +1,4 @@
-import { render, screen } from '@testing-library/react';
+import { act, render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import React from 'react';

@ -7,7 +7,7 @@ import { Button } from '../Button';
 import { Toggletip } from './Toggletip';

 describe('Toggletip', () => {
-  it('should display toogletip after click on "Click me!" button', async () => {
+  it('should display toggletip after click on "Click me!" button', async () => {
    render(
      <Toggletip placement="auto" content="Tooltip text">
        <Button type="button" data-testid="myButton">
@ -22,7 +22,7 @@ describe('Toggletip', () => {
    expect(screen.getByTestId('toggletip-content')).toBeInTheDocument();
  });

-  it('should close toogletip after click on close button', async () => {
+  it('should close toggletip after click on close button', async () => {
    const closeSpy = jest.fn();
    render(
      <Toggletip placement="auto" content="Tooltip text" onClose={closeSpy}>
@ -43,7 +43,7 @@ describe('Toggletip', () => {
    expect(closeSpy).toHaveBeenCalledTimes(1);
  });

-  it('should close toogletip after press ESC', async () => {
+  it('should close toggletip after press ESC', async () => {
    const closeSpy = jest.fn();
    render(
      <Toggletip placement="auto" content="Tooltip text" onClose={closeSpy}>
@ -62,7 +62,7 @@ describe('Toggletip', () => {
    expect(closeSpy).toHaveBeenCalledTimes(1);
  });

-  it('should display the toogletip after press ENTER', async () => {
+  it('should display the toggletip after press ENTER', async () => {
    const closeSpy = jest.fn();
    render(
      <Toggletip placement="auto" content="Tooltip text" onClose={closeSpy}>
@ -81,4 +81,109 @@ describe('Toggletip', () => {

    expect(screen.getByTestId('toggletip-content')).toBeInTheDocument();
  });
+
+  it('should be able to focus toggletip content next in DOM order - forwards and backwards', async () => {
+    const closeSpy = jest.fn();
+    const afterInDom = 'Outside of toggletip';
+
+    render(
+      <>
+        <Toggletip placement="auto" content="Tooltip text" onClose={closeSpy}>
+          <Button type="button" data-testid="myButton">
+            Click me!
+          </Button>
+        </Toggletip>
+        <button>{afterInDom}</button>
+      </>
+    );
+
+    expect(screen.queryByTestId('toggletip-content')).not.toBeInTheDocument();
+
+    const button = screen.getByTestId('myButton');
+    const afterButton = screen.getByText(afterInDom);
+    await userEvent.click(button);
+    await userEvent.tab();
+    const closeButton = screen.getByTestId('toggletip-header-close');
+    expect(closeButton).toHaveFocus();
+
+    // focus after
+    await userEvent.tab();
+    expect(afterButton).toHaveFocus();
+
+    // focus backwards
+    await userEvent.tab({ shift: true });
+    expect(closeButton).toHaveFocus();
+
+    // focus back to togglebutton
+    await userEvent.tab({ shift: true });
+    expect(button).toHaveFocus();
+  });
+
+  describe('Focus state', () => {
+    let user: ReturnType<typeof userEvent.setup>;
+
+    beforeEach(() => {
+      jest.useFakeTimers();
+      // Need to use delay: null here to work with fakeTimers
+      // see https://github.com/testing-library/user-event/issues/833
+      user = userEvent.setup({ delay: null });
+    });
+
+    afterEach(() => {
+      jest.useRealTimers();
+    });
+
+    it('should restore focus to the button that opened the toggletip when closed from within the toggletip', async () => {
+      const closeSpy = jest.fn();
+      render(
+        <Toggletip placement="auto" content="Tooltip text" onClose={closeSpy}>
+          <Button type="button" data-testid="myButton">
+            Click me!
+          </Button>
+        </Toggletip>
+      );
+
+      const button = screen.getByTestId('myButton');
+      await user.click(button);
+      const closeButton = await screen.findByTestId('toggletip-header-close');
+      expect(closeButton).toBeInTheDocument();
+      await user.click(closeButton);
+      act(() => {
+        jest.runAllTimers();
+      });
+
+      expect(button).toHaveFocus();
+    });
+
+    it('should NOT restore focus to the button that opened the toggletip when closed from outside the toggletip', async () => {
+      const closeSpy = jest.fn();
+      const afterInDom = 'Outside of toggletip';
+
+      render(
+        <>
+          <Toggletip placement="auto" content="Tooltip text" onClose={closeSpy}>
+            <Button type="button" data-testid="myButton">
+              Click me!
+            </Button>
+          </Toggletip>
+          <button>{afterInDom}</button>
+        </>
+      );
+
+      const button = screen.getByTestId('myButton');
+      await user.click(button);
+      const closeButton = await screen.findByTestId('toggletip-header-close');
+
+      expect(closeButton).toBeInTheDocument();
+      const afterButton = screen.getByText(afterInDom);
+      afterButton.focus();
+
+      await user.keyboard('{escape}');
+      act(() => {
+        jest.runAllTimers();
+      });
+
+      expect(afterButton).toHaveFocus();
+    });
+  });
 });
--- a/packages/grafana-ui/src/components/Toggletip/Toggletip.tsx
+++ b/packages/grafana-ui/src/components/Toggletip/Toggletip.tsx
@ -8,7 +8,6 @@ import { GrafanaTheme2 } from '@grafana/data';
 import { useStyles2 } from '../../themes/ThemeContext';
 import { buildTooltipTheme } from '../../utils/tooltipUtils';
 import { IconButton } from '../IconButton/IconButton';
-import { Portal } from '../Portal/Portal';

 import { ToggletipContent } from './types';

@ -50,16 +49,43 @@ export const Toggletip = React.memo(
    const contentRef = useRef(null);
    const [controlledVisible, setControlledVisible] = React.useState(false);

-    const closeToggletip = useCallback(() => {
-      setControlledVisible(false);
-      onClose?.();
-    }, [onClose]);
+    const { getArrowProps, getTooltipProps, setTooltipRef, setTriggerRef, visible, update, tooltipRef, triggerRef } =
+      usePopperTooltip(
+        {
+          visible: controlledVisible,
+          placement: placement,
+          interactive: true,
+          offset: [0, 8],
+          trigger: 'click',
+          onVisibleChange: (value: boolean) => {
+            setControlledVisible(value);
+            if (!value) {
+              onClose?.();
+            }
+          },
+        },
+        {
+          strategy: 'fixed',
+        }
+      );
+
+    const closeToggletip = useCallback(
+      (event: KeyboardEvent | React.MouseEvent<HTMLButtonElement, MouseEvent>) => {
+        setControlledVisible(false);
+        onClose?.();
+
+        if (event.target instanceof Node && tooltipRef?.contains(event.target)) {
+          triggerRef?.focus();
+        }
+      },
+      [onClose, tooltipRef, triggerRef]
+    );

    useEffect(() => {
      if (controlledVisible) {
        const handleKeyDown = (enterKey: KeyboardEvent) => {
          if (enterKey.key === 'Escape') {
-            closeToggletip();
+            closeToggletip(enterKey);
          }
        };
        document.addEventListener('keydown', handleKeyDown);
@ -70,52 +96,37 @@ export const Toggletip = React.memo(
      return;
    }, [controlledVisible, closeToggletip]);

-    const { getArrowProps, getTooltipProps, setTooltipRef, setTriggerRef, visible, update } = usePopperTooltip({
-      visible: controlledVisible,
-      placement: placement,
-      interactive: true,
-      offset: [0, 8],
-      trigger: 'click',
-      onVisibleChange: (value: boolean) => {
-        setControlledVisible(value);
-        if (!value) {
-          onClose?.();
-        }
-      },
-    });
-
    return (
      <>
        {React.cloneElement(children, {
          ref: setTriggerRef,
          tabIndex: 0,
+          'aria-expanded': visible,
        })}
        {visible && (
-          <Portal>
-            <div
-              data-testid="toggletip-content"
-              ref={setTooltipRef}
-              {...getTooltipProps({ className: cx(style.container, fitContent && styles.fitContent) })}
-            >
-              {Boolean(title) && <div className={style.header}>{title}</div>}
-              {closeButton && (
-                <div className={style.headerClose}>
-                  <IconButton
-                    tooltip="Close"
-                    name="times"
-                    data-testid="toggletip-header-close"
-                    onClick={closeToggletip}
-                  />
-                </div>
-              )}
-              <div ref={contentRef} {...getArrowProps({ className: style.arrow })} />
-              <div className={style.body}>
-                {(typeof content === 'string' || React.isValidElement(content)) && content}
-                {typeof content === 'function' && update && content({ update })}
+          <div
+            data-testid="toggletip-content"
+            ref={setTooltipRef}
+            {...getTooltipProps({ className: cx(style.container, fitContent && styles.fitContent) })}
+          >
+            {Boolean(title) && <div className={style.header}>{title}</div>}
+            {closeButton && (
+              <div className={style.headerClose}>
+                <IconButton
+                  tooltip="Close"
+                  name="times"
+                  data-testid="toggletip-header-close"
+                  onClick={closeToggletip}
+                />
              </div>
-              {Boolean(footer) && <div className={style.footer}>{footer}</div>}
+            )}
+            <div ref={contentRef} {...getArrowProps({ className: style.arrow })} />
+            <div className={style.body}>
+              {(typeof content === 'string' || React.isValidElement(content)) && content}
+              {typeof content === 'function' && update && content({ update })}
            </div>
-          </Portal>
+            {Boolean(footer) && <div className={style.footer}>{footer}</div>}
+          </div>
        )}
      </>
    );