Modal/Drawer: Switch to use floating-ui's focus trapping (#116017)

* Add awareness of a parent when toggletip is rendered to work inside other modals

* switch modal + drawer to use floating-ui's focus trapping

* remove outdated docs

* fix some unit tests

* fix scopes tests

* remove duplicate aria-label

* kick CI

* fix e2e tests

---------

Co-authored-by: tdbishop <thomas.bishop@grafana.com>
This commit is contained in:
Ashley Harrison 2026-01-09 16:47:25 +00:00 committed by GitHub
parent 9cd811b9e6
commit ca6ab973b4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 207 additions and 122 deletions

View file

@ -51,6 +51,8 @@ test.describe('Dashboard keybindings with new layouts', { tag: ['@dashboards'] }
await expect(dashboardPage.getByGrafanaSelector(selectors.components.PanelInspector.Json.content)).toBeVisible();
// Press Escape to close tooltip on the close button
await page.keyboard.press('Escape');
// Press Escape to close inspector
await page.keyboard.press('Escape');

View file

@ -58,6 +58,8 @@ test.describe(
await expect(dashboardPage.getByGrafanaSelector(selectors.components.PanelInspector.Json.content)).toBeVisible();
// Press Escape to close tooltip on the close button
await page.keyboard.press('Escape');
// Press Escape to close inspector
await page.keyboard.press('Escape');

View file

@ -82,9 +82,9 @@ test.describe('Panels test: Table - Kitchen Sink', { tag: ['@panels', '@table']
await expect(getCellHeight(page, 1, longTextColIdx)).resolves.toBeLessThan(100);
// click cell inspect, check that cell inspection pops open in the side as we'd expect.
await loremIpsumCell.getByLabel('Inspect value').click();
const loremIpsumText = await loremIpsumCell.textContent();
expect(loremIpsumText).toBeDefined();
await loremIpsumCell.getByLabel('Inspect value').click();
await expect(page.getByRole('dialog').getByText(loremIpsumText!)).toBeVisible();
});

View file

@ -1,9 +1,7 @@
import { css, cx } from '@emotion/css';
import { FloatingFocusManager, useFloating } from '@floating-ui/react';
import RcDrawer from '@rc-component/drawer';
import { useDialog } from '@react-aria/dialog';
import { FocusScope } from '@react-aria/focus';
import { useOverlay } from '@react-aria/overlays';
import { ReactNode, useCallback, useEffect, useState } from 'react';
import { ReactNode, useCallback, useEffect, useId, useState } from 'react';
import * as React from 'react';
import { GrafanaTheme2 } from '@grafana/data';
@ -81,17 +79,16 @@ export function Drawer({
const styles = useStyles2(getStyles);
const wrapperStyles = useStyles2(getWrapperStyles, size);
const dragStyles = useStyles2(getDragStyles);
const titleId = useId();
const overlayRef = React.useRef(null);
const { dialogProps, titleProps } = useDialog({}, overlayRef);
const { overlayProps } = useOverlay(
{
isDismissable: false,
isOpen: true,
onClose,
const { context, refs } = useFloating({
open: true,
onOpenChange: (open) => {
if (!open) {
onClose?.();
}
},
overlayRef
);
});
// Adds body class while open so the toolbar nav can hide some actions while drawer is open
useBodyClassWhileOpen();
@ -117,6 +114,8 @@ export function Drawer({
minWidth,
},
}}
aria-label={typeof title === 'string' ? selectors.components.Drawer.General.title(title) : undefined}
aria-labelledby={typeof title !== 'string' ? titleId : undefined}
width={''}
motion={{
motionAppear: true,
@ -129,18 +128,8 @@ export function Drawer({
motionName: styles.maskMotion,
}}
>
<FocusScope restoreFocus contain autoFocus>
<div
aria-label={
typeof title === 'string'
? selectors.components.Drawer.General.title(title)
: selectors.components.Drawer.General.title('no title')
}
className={styles.container}
{...overlayProps}
{...dialogProps}
ref={overlayRef}
>
<FloatingFocusManager context={context} modal>
<div className={styles.container} ref={refs.setFloating}>
{/* eslint-disable-next-line jsx-a11y/no-static-element-interactions */}
<div
className={cx(dragStyles.dragHandleVertical, styles.resizer)}
@ -159,7 +148,7 @@ export function Drawer({
</div>
{typeof title === 'string' ? (
<Stack direction="column">
<Text element="h3" truncate {...titleProps}>
<Text element="h3" truncate>
{title}
</Text>
{subtitle && (
@ -169,13 +158,13 @@ export function Drawer({
)}
</Stack>
) : (
title
<div id={titleId}>{title}</div>
)}
{tabs && <div className={styles.tabsWrapper}>{tabs}</div>}
</div>
{!scrollableContent ? content : <ScrollContainer showScrollIndicators>{content}</ScrollContainer>}
</div>
</FocusScope>
</FloatingFocusManager>
</RcDrawer>
);
}

View file

@ -1,9 +1,7 @@
import { cx } from '@emotion/css';
import { useDialog } from '@react-aria/dialog';
import { FocusScope } from '@react-aria/focus';
import { OverlayContainer, useOverlay } from '@react-aria/overlays';
import { PropsWithChildren, useRef, type JSX } from 'react';
import * as React from 'react';
import { FloatingFocusManager, useDismiss, useFloating, useInteractions, useRole } from '@floating-ui/react';
import { OverlayContainer } from '@react-aria/overlays';
import { PropsWithChildren, ReactNode, useId, type JSX } from 'react';
import { t } from '@grafana/i18n';
@ -66,23 +64,26 @@ export function Modal(props: PropsWithChildren<Props>) {
trapFocus = true,
} = props;
const styles = useStyles2(getModalStyles);
const titleId = useId();
const ref = useRef<HTMLDivElement>(null);
// Handle interacting outside the dialog and pressing
// the Escape key to close the modal.
const { overlayProps, underlayProps } = useOverlay(
{ isKeyboardDismissDisabled: !closeOnEscape, isOpen, onClose: onDismiss },
ref
);
// Get props for the dialog and its title
const { dialogProps, titleProps } = useDialog(
{
'aria-label': ariaLabel,
const { context, refs } = useFloating({
open: isOpen,
onOpenChange: (open) => {
if (!open) {
onDismiss?.();
}
},
ref
);
});
const dismiss = useDismiss(context, {
enabled: closeOnEscape,
});
const role = useRole(context, {
role: 'dialog',
});
const { getFloatingProps } = useInteractions([dismiss, role]);
if (!isOpen) {
return null;
@ -96,12 +97,17 @@ export function Modal(props: PropsWithChildren<Props>) {
role="presentation"
className={styles.modalBackdrop}
onClick={onClickBackdrop || (closeOnBackdropClick ? onDismiss : undefined)}
{...underlayProps}
/>
<FocusScope contain={trapFocus} autoFocus restoreFocus>
<div className={cx(styles.modal, className)} ref={ref} {...overlayProps} {...dialogProps}>
<FloatingFocusManager context={context} modal={trapFocus}>
<div
className={cx(styles.modal, className)}
ref={refs.setFloating}
aria-label={ariaLabel}
aria-labelledby={typeof title === 'string' ? titleId : undefined}
{...getFloatingProps()}
>
<div className={headerClass}>
{typeof title === 'string' && <DefaultModalHeader {...props} title={title} id={titleProps.id} />}
{typeof title === 'string' && <DefaultModalHeader {...props} title={title} id={titleId} />}
{
// FIXME: custom title components won't get an accessible title.
// Do we really want to support them or shall we just limit this ModalTabsHeader?
@ -118,12 +124,12 @@ export function Modal(props: PropsWithChildren<Props>) {
</div>
<div className={cx(styles.modalContent, contentClassName)}>{children}</div>
</div>
</FocusScope>
</FloatingFocusManager>
</OverlayContainer>
);
}
function ModalButtonRow({ leftItems, children }: { leftItems?: React.ReactNode; children: React.ReactNode }) {
function ModalButtonRow({ leftItems, children }: { leftItems?: ReactNode; children: ReactNode }) {
const styles = useStyles2(getModalStyles);
if (leftItems) {

View file

@ -75,5 +75,3 @@ return (
</Toggletip>
);
```
<ArgTypes of={Toggletip} />

View file

@ -1,6 +1,11 @@
import { Meta, StoryFn } from '@storybook/react';
import { useState } from 'react';
import { Button } from '../Button/Button';
import { Drawer } from '../Drawer/Drawer';
import { Field } from '../Forms/Field';
import { Input } from '../Input/Input';
import { Modal } from '../Modal/Modal';
import { ScrollContainer } from '../ScrollContainer/ScrollContainer';
import mdx from '../Toggletip/Toggletip.mdx';
@ -133,4 +138,86 @@ LongContent.parameters = {
},
};
export const InsideDrawer: StoryFn<typeof Toggletip> = () => {
const [isDrawerOpen, setIsDrawerOpen] = useState(false);
return (
<>
<Button onClick={() => setIsDrawerOpen(true)}>Open Drawer</Button>
{isDrawerOpen && (
<Drawer title="Drawer with Toggletip" onClose={() => setIsDrawerOpen(false)}>
<div>
<p style={{ marginBottom: '16px' }}>This demonstrates using Toggletip inside a Drawer.</p>
<Toggletip
title="Interactive Form"
content={
<div style={{ display: 'flex', flexDirection: 'column', gap: '8px' }}>
<Field label="Name">
<Input placeholder="Enter your name" />
</Field>
<Button variant="primary" size="sm">
Submit
</Button>
</div>
}
footer="Focus should work correctly within this Toggletip"
placement="bottom-start"
>
<Button>Click to show Toggletip</Button>
</Toggletip>
</div>
</Drawer>
)}
</>
);
};
InsideDrawer.parameters = {
controls: {
hideNoControlsWarning: true,
exclude: ['title', 'content', 'footer', 'children', 'placement', 'theme', 'closeButton', 'portalRoot'],
},
};
export const InsideModal: StoryFn<typeof Toggletip> = () => {
const [isModalOpen, setIsModalOpen] = useState(false);
return (
<>
<Button onClick={() => setIsModalOpen(true)}>Open Modal</Button>
<Modal title="Modal with Toggletip" isOpen={isModalOpen} onDismiss={() => setIsModalOpen(false)}>
<div>
<p style={{ marginBottom: '16px' }}>This demonstrates using Toggletip inside a Modal.</p>
<Modal.ButtonRow>
<Toggletip
title="Interactive Form"
content={
<div style={{ display: 'flex', flexDirection: 'column', gap: '8px' }}>
<Field label="Name">
<Input placeholder="Enter your name" />
</Field>
<Button variant="primary" size="sm">
Submit
</Button>
</div>
}
footer="Focus should work correctly within this Toggletip"
placement="bottom-start"
>
<Button>Click to show Toggletip</Button>
</Toggletip>
</Modal.ButtonRow>
</div>
</Modal>
</>
);
};
InsideDrawer.parameters = {
controls: {
hideNoControlsWarning: true,
exclude: ['title', 'content', 'footer', 'children', 'placement', 'theme', 'closeButton', 'portalRoot'],
},
};
export default meta;

View file

@ -79,6 +79,9 @@ describe('new receiver', () => {
// click test
await user.click(ui.testContactPoint.get());
// close the modal
await user.click(screen.getByRole('button', { name: 'Close' }));
// we shouldn't be testing implementation details but when the request is successful
// it can't seem to assert on the success toast
await user.click(ui.saveContactButton.get());

View file

@ -1,4 +1,4 @@
import { act, fireEvent, render, testWithFeatureToggles } from 'test/test-utils';
import { render, testWithFeatureToggles, userEvent, waitFor } from 'test/test-utils';
import { ExpressionQuery, ExpressionQueryType } from '../../types';
@ -72,12 +72,12 @@ describe('SqlExpr', () => {
const refIds = [{ value: 'A' }];
const query = { refId: 'expr1', type: 'sql', expression: '' } as ExpressionQuery;
await act(async () => {
render(<SqlExpr onChange={onChange} refIds={refIds} query={query} queries={[]} />);
});
render(<SqlExpr onChange={onChange} refIds={refIds} query={query} queries={[]} />);
// Verify onChange was called
expect(onChange).toHaveBeenCalled();
await waitFor(() => {
expect(onChange).toHaveBeenCalled();
});
// Verify essential SQL structure without exact string matching
const updatedQuery = onChange.mock.calls[0][0];
@ -90,19 +90,12 @@ describe('SqlExpr', () => {
const existingExpression = 'SELECT 1 AS foo';
const query = { refId: 'expr1', type: 'sql', expression: existingExpression } as ExpressionQuery;
await act(async () => {
render(<SqlExpr onChange={onChange} refIds={refIds} query={query} queries={[]} />);
});
// Check if onChange was called
if (onChange.mock.calls.length > 0) {
// If called, ensure it didn't change the expression value
const updatedQuery = onChange.mock.calls[0][0];
expect(updatedQuery.expression).toBe(existingExpression);
}
render(<SqlExpr onChange={onChange} refIds={refIds} query={query} queries={[]} />);
// The SQLEditor should receive the existing expression
expect(query.expression).toBe(existingExpression);
await waitFor(() => {
expect(query.expression).toBe(existingExpression);
});
});
it('adds alerting format when alerting prop is true', async () => {
@ -110,40 +103,12 @@ describe('SqlExpr', () => {
const refIds = [{ value: 'A' }];
const query = { refId: 'expr1', type: 'sql' } as ExpressionQuery;
await act(async () => {
render(<SqlExpr onChange={onChange} refIds={refIds} query={query} alerting queries={[]} />);
render(<SqlExpr onChange={onChange} refIds={refIds} query={query} alerting queries={[]} />);
await waitFor(() => {
const updatedQuery = onChange.mock.calls[0][0];
expect(updatedQuery.format).toBe('alerting');
});
const updatedQuery = onChange.mock.calls[0][0];
expect(updatedQuery.format).toBe('alerting');
});
});
describe('SqlExpr with GenAI features', () => {
const defaultProps: SqlExprProps = {
onChange: jest.fn(),
refIds: [{ value: 'A' }],
query: { refId: 'expression_1', type: ExpressionQueryType.sql, expression: `SELECT * FROM A LIMIT 10` },
queries: [],
};
it('renders suggestions drawer when isDrawerOpen is true', async () => {
const { useSQLSuggestions } = require('./GenAI/hooks/useSQLSuggestions');
useSQLSuggestions.mockImplementation(() => ({
isDrawerOpen: true,
suggestions: ['suggestion1', 'suggestion2'],
}));
const { findByTestId } = render(<SqlExpr {...defaultProps} />);
expect(await findByTestId('suggestions-drawer')).toBeInTheDocument();
});
it('renders explanation drawer when isExplanationOpen is true', async () => {
const { useSQLExplanations } = require('./GenAI/hooks/useSQLExplanations');
useSQLExplanations.mockImplementation(() => ({ isExplanationOpen: true }));
const { findByTestId } = render(<SqlExpr {...defaultProps} />);
expect(await findByTestId('explanation-drawer')).toBeInTheDocument();
});
});
@ -166,10 +131,10 @@ describe('Schema Inspector feature toggle', () => {
});
});
it('renders panel open by default', () => {
const { getByText } = render(<SqlExpr {...defaultProps} />);
it('renders panel open by default', async () => {
const { findByText } = render(<SqlExpr {...defaultProps} />);
expect(getByText('No schema information available')).toBeInTheDocument();
expect(await findByText('No schema information available')).toBeInTheDocument();
});
it('closes panel and shows reopen button when close button clicked', async () => {
@ -178,7 +143,7 @@ describe('Schema Inspector feature toggle', () => {
expect(queryByText('No schema information available')).toBeInTheDocument();
const closeButton = getByText('Schema inspector');
await act(async () => fireEvent.click(closeButton));
await userEvent.click(closeButton);
expect(queryByText('No schema information available')).not.toBeInTheDocument();
expect(await findByText('Schema inspector')).toBeInTheDocument();
@ -188,12 +153,12 @@ describe('Schema Inspector feature toggle', () => {
const { queryByText, getByText } = render(<SqlExpr {...defaultProps} />);
const closeButton = getByText('Schema inspector');
await act(async () => fireEvent.click(closeButton));
await userEvent.click(closeButton);
expect(queryByText('No schema information available')).not.toBeInTheDocument();
const reopenButton = getByText('Schema inspector');
await act(async () => fireEvent.click(reopenButton));
await userEvent.click(reopenButton);
expect(queryByText('No schema information available')).toBeInTheDocument();
});
@ -233,3 +198,33 @@ describe('Schema Inspector feature toggle', () => {
});
});
});
describe('SqlExpr with GenAI features', () => {
const defaultProps: SqlExprProps = {
onChange: jest.fn(),
refIds: [{ value: 'A' }],
query: { refId: 'expression_1', type: ExpressionQueryType.sql, expression: `SELECT * FROM A LIMIT 10` },
queries: [],
};
it('renders suggestions drawer when isDrawerOpen is true', async () => {
// TODO this inline require breaks future tests - do it differently!
const { useSQLSuggestions } = require('./GenAI/hooks/useSQLSuggestions');
useSQLSuggestions.mockImplementation(() => ({
isDrawerOpen: true,
suggestions: ['suggestion1', 'suggestion2'],
}));
const { findByTestId } = render(<SqlExpr {...defaultProps} />);
expect(await findByTestId('suggestions-drawer')).toBeInTheDocument();
});
it('renders explanation drawer when isExplanationOpen is true', async () => {
// TODO this inline require breaks future tests - do it differently!
const { useSQLExplanations } = require('./GenAI/hooks/useSQLExplanations');
useSQLExplanations.mockImplementation(() => ({ isExplanationOpen: true }));
const { findByTestId } = render(<SqlExpr {...defaultProps} />);
expect(await findByTestId('explanation-drawer')).toBeInTheDocument();
});
});

View file

@ -340,14 +340,16 @@ describe('LibraryPanelsSearch', () => {
await user.click(screen.getAllByRole('button', { name: 'Delete' })[1]);
await waitFor(() =>
expect(getLibraryPanelsSpy).toHaveBeenCalledWith({
searchString: '',
folderFilterUIDs: ['wfTJJL5Wz'],
page: 1,
typeFilter: [],
sortDirection: undefined,
perPage: 40,
})
expect(getLibraryPanelsSpy).toHaveBeenCalledWith(
expect.objectContaining({
searchString: '',
folderFilterUIDs: ['wfTJJL5Wz'],
page: 1,
typeFilter: [],
sortDirection: undefined,
perPage: 40,
})
)
);
});
});

View file

@ -105,7 +105,6 @@ describe('Selector', () => {
// Lowercase because we don't have any backend that returns the correct case, then it falls back to the value in the URL
expectScopesSelectorValue('grafana');
await openSelector();
//screen.debug(undefined, 100000);
expectResultApplicationsGrafanaSelected();
jest.spyOn(locationService, 'getLocation').mockRestore();
@ -175,6 +174,7 @@ describe('Selector', () => {
await applyScopes();
// Deselect all scopes
await hoverSelector();
await clearSelector();
// Recent scopes should still be available
@ -197,6 +197,7 @@ describe('Selector', () => {
await selectResultApplicationsMimir();
await applyScopes();
await hoverSelector();
await clearSelector();
// Check recent scopes are updated

View file

@ -47,7 +47,7 @@ const type = async (selector: () => HTMLInputElement, value: string) => {
export const updateScopes = async (service: ScopesService, scopes: string[]) =>
act(async () => service.changeScopes(scopes));
export const openSelector = async () => click(getSelectorInput);
export const hoverSelector = async () => fireEvent.mouseOver(getSelectorInput());
export const hoverSelector = async () => userEvent.hover(getSelectorInput());
export const clearSelector = async () => click(getSelectorClear);
export const applyScopes = async () => {
await click(getSelectorApply);