focus fixes

This commit is contained in:
Pavel committed 2026-09-27 11:58:51 +04:00
1 parent 420b4d33b9
commit 4e6f1bf9e4
8 files changed
+212 -23

No files matched your search

@@ -26,6 +26,11 @@ afterEach(async () => {
container.remove();
});
const settle = () =>
act(async () => {
await new Promise((resolve) => setTimeout(resolve, 20));
});
const render = async (element: React.ReactElement) => {
await act(async () => root.render(element));
};
@@ -127,3 +132,31 @@ describe('DialogContent layout', () => {
expect(header).not.toContain('text-center');
});
});
describe('DialogContent focus return', () => {
const opener = () =>
document.querySelector<HTMLButtonElement>('[data-testid="opener"]')!;
const renderDialog = (open: boolean) =>
render(
<>
<button type="button" data-testid="opener">
Search
</button>
<Dialog open={open}>
<DialogContent aria-describedby={undefined}>
<DialogTitle>Search</DialogTitle>
</DialogContent>
</Dialog>
</>,
);
it('returns focus to the element that had it on open', async () => {
await renderDialog(false);
opener().focus();
await renderDialog(true);
await renderDialog(false);
await settle();
expect(document.activeElement).toBe(opener());
});
});
+5
View File
@@ -2,6 +2,7 @@ import { XIcon } from 'lucide-react';
import * as React from 'react';
import { Button } from '@/components/ui/button';
import { useFocusReturn } from '@/components/ui/use-focus-return';
import { cn, overlayScrim } from '@/lib/utils';
import { Dialog as DialogPrimitive } from 'radix-ui';
@@ -49,10 +50,13 @@ function DialogContent({
className,
children,
showCloseButton = true,
onOpenAutoFocus,
onCloseAutoFocus,
...props
}: React.ComponentProps<typeof DialogPrimitive.Content> & {
showCloseButton?: boolean;
}) {
const focusReturn = useFocusReturn(onOpenAutoFocus, onCloseAutoFocus);
return (
<DialogPortal data-slot="dialog-portal">
<DialogOverlay />
@@ -63,6 +67,7 @@ function DialogContent({
className,
)}
{...props}
{...focusReturn}
>
{children}
{showCloseButton && (
+39
View File
@@ -30,6 +30,11 @@ const render = async (element: React.ReactElement) => {
await act(async () => root.render(element));
};
const settle = () =>
act(async () => {
await new Promise((resolve) => setTimeout(resolve, 20));
});
const content = () =>
document.querySelector<HTMLElement>('[data-slot="modal-content"]')!;
@@ -251,3 +256,37 @@ describe('Modal mobile sheet', () => {
expect(content().querySelector('[data-slot="sheet-handle"]')).toBeNull();
});
});
describe('Modal focus return', () => {
const opener = () =>
document.querySelector<HTMLButtonElement>('[data-testid="opener"]')!;
const renderModal = (open: boolean) =>
render(
<>
<button type="button" data-testid="opener">
Open
</button>
<Modal open={open} onOpenChange={() => undefined} title="Upload">
Body
</Modal>
</>,
);
it('returns focus to the element that had it on open', async () => {
await renderModal(false);
opener().focus();
await renderModal(true);
await renderModal(false);
await settle();
expect(document.activeElement).toBe(opener());
});
it('moves focus nowhere when nothing had it on open', async () => {
await renderModal(false);
await renderModal(true);
await renderModal(false);
await settle();
expect(document.activeElement).toBe(document.body);
});
});
+6 -3
View File
@@ -12,6 +12,7 @@ import {
DialogTitle,
} from '@/components/ui/dialog';
import { SheetHandle, sheetBottomShape } from '@/components/ui/sheet';
import { useFocusReturn } from '@/components/ui/use-focus-return';
import { useMediaQuery } from '@/hooks';
import { cn } from '@/lib/utils';
@@ -63,6 +64,10 @@ const Modal = React.forwardRef<HTMLDivElement, ModalProps>(function Modal(
const { isMobile } = useMediaQuery();
const isMobileSheet = mobileVariant === 'sheet' && isMobile;
const shouldShowCloseButton = showCloseButton && !isPerformingTask;
// The phone sheet opens without autofocus so the keyboard stays down.
const focusReturn = useFocusReturn(
isMobileSheet ? (event) => event.preventDefault() : undefined,
);
// When a task is performing, block click-outside / pointer-outside to
// mirror the legacy WrapperModal lock. Esc remains enabled (Radix default).
@@ -113,9 +118,7 @@ const Modal = React.forwardRef<HTMLDivElement, ModalProps>(function Modal(
data-mobile-sheet={isMobileSheet ? '' : undefined}
onPointerDownOutside={blockOutsideInteractions}
onInteractOutside={blockOutsideInteractions}
onOpenAutoFocus={
isMobileSheet ? (event) => event.preventDefault() : undefined
}
{...focusReturn}
// Radix portals this to <body> in the DOM, but React still bubbles
// synthetic events through the JSX tree. Stop the bubble at the
// modal boundary so consumers mounted inside clickable cards (e.g.
+64 -1
View File
@@ -2,7 +2,7 @@ import { act } from 'react';
import { createRoot, type Root } from 'react-dom/client';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { Sheet, SheetContent, SheetTitle } from './sheet';
import { Sheet, SheetContent, SheetTitle, SheetTrigger } from './sheet';
Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true });
@@ -153,3 +153,66 @@ describe('SheetOverlay', () => {
expect(title?.className).toContain('text-xl leading-tight font-semibold');
});
});
describe('SheetContent focus', () => {
const pressEscape = async () => {
await act(async () => {
document.dispatchEvent(
new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }),
);
});
// Radix moves focus once the closed content has unmounted.
await act(async () => {
await new Promise((resolve) => setTimeout(resolve, 20));
});
};
const renderWithTrigger = () =>
render(
<Sheet>
<SheetTrigger data-testid="trigger">Tools</SheetTrigger>
{/* Like the phone pickers: no autofocus, so the keyboard stays down. */}
<SheetContent
side="bottom"
title="Tools"
aria-describedby={undefined}
onOpenAutoFocus={(event) => event.preventDefault()}
>
<button type="button">Inside</button>
</SheetContent>
</Sheet>,
);
const trigger = () =>
document.querySelector<HTMLButtonElement>('[data-testid="trigger"]')!;
it('never draws the browser outline around the panel', async () => {
await render(
<Sheet open>
<SheetContent
side="bottom"
title="Tools"
aria-describedby={undefined}
/>
</Sheet>,
);
expect(content().className.split(' ')).toContain('outline-none');
});
it('leaves the trigger unfocused when a tap opened the sheet', async () => {
await renderWithTrigger();
// iOS doesn't focus a tapped button: the click lands with body focused.
await act(async () => trigger().click());
expect(content()).not.toBeNull();
await pressEscape();
expect(document.activeElement).not.toBe(trigger());
});
it('returns focus to the trigger when it had focus on open', async () => {
await renderWithTrigger();
trigger().focus();
await act(async () => trigger().click());
await pressEscape();
expect(document.activeElement).toBe(trigger());
});
});
+6 -1
View File
@@ -5,6 +5,7 @@ import { cva } from 'class-variance-authority';
import { Button } from '@/components/ui/button';
import { cn, overlayScrim } from '@/lib/utils';
import { useFocusReturn } from '@/components/ui/use-focus-return';
function Sheet({ ...props }: React.ComponentProps<typeof SheetPrimitive.Root>) {
return <SheetPrimitive.Root data-slot="sheet" {...props} />;
@@ -69,7 +70,7 @@ function SheetHandle({ className, ...props }: React.ComponentProps<'div'>) {
}
const sheetContentVariants = cva(
'bg-background data-[state=open]:animate-in data-[state=closed]:animate-out fixed z-50 flex flex-col gap-4 shadow-lg transition ease-in-out data-[state=closed]:duration-300 data-[state=open]:duration-500',
'bg-background data-[state=open]:animate-in data-[state=closed]:animate-out fixed z-50 flex flex-col gap-4 shadow-lg outline-none transition ease-in-out data-[state=closed]:duration-300 data-[state=open]:duration-500',
{
variants: {
side: {
@@ -95,6 +96,8 @@ function SheetContent({
// handle is only a cue; it doesn't drag).
showCloseButton = !handle,
title,
onOpenAutoFocus,
onCloseAutoFocus,
...props
}: React.ComponentProps<typeof SheetPrimitive.Content> & {
side?: 'top' | 'right' | 'bottom' | 'left';
@@ -106,6 +109,7 @@ function SheetContent({
// heading of its own (omit it if the children already render a SheetTitle).
title?: string;
}) {
const focusReturn = useFocusReturn(onOpenAutoFocus, onCloseAutoFocus);
return (
<SheetPortal>
<SheetOverlay />
@@ -114,6 +118,7 @@ function SheetContent({
data-side={side}
className={cn(sheetContentVariants({ side }), className)}
{...props}
{...focusReturn}
>
{title ? <SheetTitle className="sr-only">{title}</SheetTitle> : null}
{handle && <SheetHandle />}
@@ -0,0 +1,58 @@
import * as React from 'react';
type AutoFocusHandler = (event: Event) => void;
/**
* Focus return for Radix dialogs (Sheet, Modal, DialogContent): on close,
* focus goes back to whatever had it when the dialog opened, and nowhere if
* nothing did.
*
* Radix focuses the trigger on close even when it never had focus. A tap on
* iOS (and a click in Safari) doesn't focus a button, so after a phone sheet
* the trigger would light up with a focus-visible ring out of nowhere. After a
* keyboard open the trigger did have focus and gets it back as before, and a
* dialog opened without a trigger (a Modal from a menu row) now returns focus
* to where the user was instead of dropping it on <body>.
*
* Returns the two handlers to pass to the Radix Content. The caller's own
* handlers run first; a caller that calls `preventDefault()` on close keeps
* control of focus.
*/
function useFocusReturn(
onOpenAutoFocus?: AutoFocusHandler,
onCloseAutoFocus?: AutoFocusHandler,
) {
const returnTo = React.useRef<HTMLElement | null>(null);
const handleOpenAutoFocus = React.useCallback(
(event: Event) => {
// Still the element that had focus before the dialog opened.
const active = document.activeElement;
returnTo.current =
active instanceof HTMLElement && active !== document.body
? active
: null;
onOpenAutoFocus?.(event);
},
[onOpenAutoFocus],
);
const handleCloseAutoFocus = React.useCallback(
(event: Event) => {
onCloseAutoFocus?.(event);
const target = returnTo.current;
returnTo.current = null;
if (event.defaultPrevented) return;
event.preventDefault();
if (target?.isConnected) target.focus();
},
[onCloseAutoFocus],
);
return {
onOpenAutoFocus: handleOpenAutoFocus,
onCloseAutoFocus: handleCloseAutoFocus,
};
}
export { useFocusReturn };
+1 -18
View File
@@ -1,4 +1,4 @@
import React, { useEffect, useRef, useState } from 'react';
import React, { useEffect, useState } from 'react';
import { useTranslation } from 'react-i18next';
import { useSelector } from 'react-redux';
@@ -47,9 +47,6 @@ export default function TraceSheet({
const [failed, setFailed] = useState(false);
// Bumped by Retry to re-run the fetch for the same trace.
const [reloadKey, setReloadKey] = useState(0);
// The sheet is opened from a Logs row, not a SheetTrigger, so Radix has no
// trigger to return focus to on close; remember what had focus instead.
const returnFocusRef = useRef<HTMLElement | null>(null);
useEffect(() => {
if (!traceRef) return;
@@ -87,20 +84,6 @@ export default function TraceSheet({
side="right"
title={t('settings.logs.trace.title')}
aria-describedby={undefined}
onOpenAutoFocus={() => {
returnFocusRef.current =
document.activeElement instanceof HTMLElement
? document.activeElement
: null;
}}
onCloseAutoFocus={(event) => {
const target = returnFocusRef.current;
returnFocusRef.current = null;
if (target?.isConnected) {
event.preventDefault();
target.focus();
}
}}
className="w-full overflow-y-auto sm:max-w-3xl"
>
<div className="flex flex-col gap-6 px-4 pt-4 pb-6">