fix(BottomSheet): focus returned after a pointer close gets no focus ring

.changeset/bottom-sheet-focus-return-last-input.md

@@ -0,0 +1,7 @@
+---
+'@astryxdesign/core': patch
+---
+
+[fix] `BottomSheet` returns focus without a focus ring after a pointer close (a tap in WebKit, or a pointer close of a sheet opened from the keyboard) and keeps the ring after a keyboard close, including on a `DropdownMenu` bottom-sheet trigger the caller renders (`renderTrigger`). The sheet now asks the browser not to treat that focus as keyboard focus, the way Dialog's focus moves do.
+
+@vjeux

apps/storybook/stories/DropdownMenu.stories.tsx

@@ -1052,6 +1052,40 @@ export const ActionSheetPresentation: Story = {
   },
 };
 
+export const ActionSheetCustomTrigger: Story = {
+  name: 'Presentation / action sheet, custom trigger',
+  parameters: {
+    layout: 'fullscreen',
+    docs: {
+      story: {inline: false, height: '560px'},
+      description: {
+        story:
+          'A control the caller renders opens the bottom-sheet presentation. When the sheet closes, focus returns to that control, with a focus ring only after keyboard input.',
+      },
+    },
+  },
+  globals: {viewport: {value: 'mobile1', isRotated: false}},
+  render: () => (
+    <div {...stylex.props(readinessStyles.viewportStoryCanvas)}>
+      <DropdownMenu
+        presentation="bottom-sheet"
+        renderTrigger={props => (
+          <IconButton
+            icon={<EllipsisHorizontalIcon />}
+            label="Project actions"
+            variant="ghost"
+            {...props}
+          />
+        )}
+        items={PROJECT_ACTIONS.map(action => ({
+          ...action,
+          onClick: () => console.log(`${action.label} selected`),
+        }))}
+      />
+    </div>
+  ),
+};
+
 export const SectionedActionSheetPresentation: Story = {
   name: 'Presentation / sectioned action sheet',
   parameters: {

packages/core/src/BottomSheet/BottomSheet.test.tsx

@@ -14,6 +14,7 @@ import {act, render, screen, fireEvent} from '@testing-library/react';
 import {createRef, useState} from 'react';
 import {BottomSheet} from './BottomSheet';
 import {BottomSheetSwitcher} from './BottomSheetSwitcher';
+import {__resetInteractionModalityForTest} from '../utils/interactionModality';
 
 // A sheet has no stops unless its host asks for them, so the tests that
 // exercise detents pass their own. These three — a 14% peek, a half-height
@@ -2351,6 +2352,119 @@ describe('BottomSheet', () => {
 
       expect(document.activeElement).toBe(finalFocusRef.current);
     });
+
+    // The returned focus shows the focus-visible ring only after keyboard
+    // input (`architecture:interaction-modality` INV1). A plain focus() leaves
+    // that to the browser, which rings it after a pointer close: WebKit after
+    // a tap, and Chromium when the sheet was opened from the keyboard.
+    describe('ring on the returned focus', () => {
+      beforeEach(() => {
+        __resetInteractionModalityForTest();
+      });
+
+      afterEach(() => {
+        vi.restoreAllMocks();
+      });
+
+      function openThenClose(closeWith: 'pointer' | 'keyboard') {
+        render(<Harness />);
+        const opener = screen.getByRole('button', {name: 'Open sheet'});
+        opener.focus();
+        fireEvent.click(opener);
+        const done = screen.getByRole('button', {name: 'Done'});
+        if (closeWith === 'pointer') {
+          fireEvent.pointerDown(done);
+        } else {
+          fireEvent.keyDown(done, {key: 'Enter'});
+        }
+        fireEvent.click(done);
+        return opener;
+      }
+
+      it('asks the browser for no ring after a pointer close', () => {
+        const opener = openThenClose('pointer');
+        const focus = vi.spyOn(opener, 'focus');
+        finishSheetExit();
+
+        expect(opener).toHaveFocus();
+        expect(focus.mock.calls).toEqual([[{focusVisible: false}]]);
+      });
+
+      it('leaves the ring to the browser after a keyboard close', () => {
+        const opener = openThenClose('keyboard');
+        const focus = vi.spyOn(opener, 'focus');
+        finishSheetExit();
+
+        expect(opener).toHaveFocus();
+        expect(focus.mock.calls).toEqual([[]]);
+      });
+
+      it('clears a ring the native close already drew before refocusing', () => {
+        const opener = openThenClose('pointer');
+        // The browser's own dialog close restores focus to the opener, with a
+        // ring, before the sheet moves focus, and focusing the focused element
+        // changes nothing.
+        HTMLDialogElement.prototype.close = vi.fn(function (
+          this: HTMLDialogElement,
+        ) {
+          this.removeAttribute('open');
+          opener.focus();
+        });
+        const matches = HTMLElement.prototype.matches;
+        vi.spyOn(HTMLElement.prototype, 'matches').mockImplementation(function (
+          this: HTMLElement,
+          selector: string,
+        ) {
+          return selector === ':focus-visible'
+            ? this === opener
+            : matches.call(this, selector);
+        });
+        const blur = vi.spyOn(opener, 'blur');
+        const focus = vi.spyOn(opener, 'focus');
+        finishSheetExit();
+
+        expect(blur).toHaveBeenCalledTimes(1);
+        expect(focus.mock.calls.at(-1)).toEqual([{focusVisible: false}]);
+        expect(blur.mock.invocationCallOrder[0]).toBeLessThan(
+          focus.mock.invocationCallOrder.at(-1) ?? 0,
+        );
+        expect(opener).toHaveFocus();
+      });
+
+      it('asks for no ring on an explicit final focus target too', () => {
+        const finalFocusRef = createRef<HTMLButtonElement>();
+        function ExplicitFocusHarness() {
+          const [isOpen, setIsOpen] = useState(true);
+          return (
+            <>
+              <button ref={finalFocusRef} type="button">
+                Adaptive trigger
+              </button>
+              <BottomSheet
+                isOpen={isOpen}
+                onOpenChange={setIsOpen}
+                finalFocusRef={finalFocusRef}
+                label="Filters">
+                <button type="button" onClick={() => setIsOpen(false)}>
+                  Done
+                </button>
+              </BottomSheet>
+            </>
+          );
+        }
+
+        render(<ExplicitFocusHarness />);
+        const done = screen.getByRole('button', {name: 'Done'});
+        fireEvent.pointerDown(done);
+        fireEvent.click(done);
+        const target = finalFocusRef.current!;
+        const focus = vi.spyOn(target, 'focus');
+        finishSheetExit();
+
+        expect(target).toHaveFocus();
+        expect(focus.mock.calls).toEqual([[{focusVisible: false}]]);
+      });
+    });
   });
 
   describe('initial focus', () => {

packages/core/src/BottomSheet/BottomSheet.tsx

@@ -4,7 +4,7 @@
 
 /**
  * @file BottomSheet.tsx
- * @input Uses React, StyleX, core hooks, shared layer dismissal, named BottomSheetPanel, BottomSheetSwitcherContext
+ * @input Uses React, StyleX, core hooks, shared layer dismissal, the shared interaction modality, named BottomSheetPanel, BottomSheetSwitcherContext
  * @output Exports BottomSheet component and BottomSheetProps
  * @position Public BottomSheet router plus private standalone/switcher hosts
  *
@@ -46,6 +46,8 @@ import {colorVars, durationVars, easeVars} from '../theme/tokens.stylex';
 import {useDevWarning, useScrollLock} from '../hooks';
 import {LayerDepthProvider} from '../Layer/LayerDepthContext';
 import {useLayerDismissal} from '../Layer/useLayerDismissal';
+import {focusForLastInput} from '../utils/focusReturn';
+import {useInteractionModalityTracking} from '../utils/interactionModality';
 import {
   BottomSheetPanel,
   type BottomSheetPanelMotion,
@@ -320,12 +322,20 @@ function StandaloneBottomSheet({
         dialog.close();
       }
       setIsPresented(false);
-      (finalFocusRef?.current ?? triggerRef.current)?.focus();
+      // The returned focus shows the focus-visible ring only after keyboard
+      // input (`architecture:interaction-modality` INV1). The native close
+      // may already have restored focus to the target with a ring; the helper
+      // clears it first.
+      const returnTarget = finalFocusRef?.current ?? triggerRef.current;
+      if (returnTarget != null) {
+        focusForLastInput(returnTarget);
+      }
       triggerRef.current = null;
     },
     [finalFocusRef, isOpen],
   );
 
+  useInteractionModalityTracking();
   useScrollLock(shouldPresent && hasScrim);
   useDevWarning(
     'BottomSheet',

packages/core/src/BottomSheet/__tests__/BottomSheetFocusReturn.a11y.browser.spec.ts

@@ -0,0 +1,96 @@
+// Copyright (c) Meta Platforms, Inc. and affiliates.
+
+/**
+ * @file BottomSheetFocusReturn.a11y.browser.spec.ts
+ * @input The `Core/DropdownMenu` action-sheet story with a trigger the caller
+ *   renders, in a built Storybook, and real Chromium/WebKit pointer and
+ *   keyboard input.
+ * @output Browser evidence that the focus a BottomSheet returns when it closes
+ *   follows the last input (docs/architecture/interaction-modality.md INV1):
+ *   no ring after a pointer close, the ring after a keyboard close.
+ * @position Cross-browser proof; jsdom only proves which focus call the sheet
+ *   makes. The menu's sheet returns focus through BottomSheet's
+ *   `finalFocusRef`, and a control the caller renders carries no style of its
+ *   own that would hide a ring, so the ring shown is the browser's answer.
+ *   Chromium rings it when a keyboard-opened sheet is closed by a pointer;
+ *   WebKit also rings it after a tap open and a tap close.
+ */
+
+import {expect, test, type Locator, type Page} from '@playwright/test';
+import {
+  DEFAULT_STORYBOOK_DIR,
+  serveStorybook,
+  type StaticServer,
+} from '@astryxdesign/a11y-spec/storybook';
+
+let storybook: StaticServer;
+test.beforeAll(async () => {
+  storybook = await serveStorybook(
+    process.env.ASTRYX_STORYBOOK_DIR ?? DEFAULT_STORYBOOK_DIR,
+  );
+});
+test.afterAll(async () => {
+  await storybook?.close();
+});
+
+async function mount(page: Page): Promise<Locator> {
+  await page.goto(
+    `${storybook.origin}/iframe.html?id=core-dropdownmenu--action-sheet-custom-trigger&viewMode=story`,
+  );
+  const trigger = page.getByRole('button', {
+    name: 'Project actions',
+    exact: true,
+  });
+  await expect(trigger).toBeVisible();
+  return trigger;
+}
+
+async function openFromKeyboard(page: Page, trigger: Locator): Promise<void> {
+  await trigger.focus();
+  await page.keyboard.press('Enter');
+  await expect(page.getByRole('dialog')).toBeVisible();
+}
+
+function row(page: Page): Locator {
+  return page.getByRole('dialog').getByRole('button', {name: /Edit project/});
+}
+
+async function expectReturnedFocus(
+  page: Page,
+  trigger: Locator,
+  ring: boolean,
+): Promise<void> {
+  await expect(page.getByRole('dialog')).toBeHidden();
+  await expect(trigger).toBeFocused();
+  expect(
+    await trigger.evaluate(element => element.matches(':focus-visible')),
+  ).toBe(ring);
+}
+
+test('a keyboard-opened sheet closed by a pointer pick returns focus without a ring', async ({
+  page,
+}) => {
+  const trigger = await mount(page);
+  await openFromKeyboard(page, trigger);
+  await row(page).click();
+  await expectReturnedFocus(page, trigger, false);
+});
+
+test('a pointer open and pointer pick return focus without a ring', async ({
+  page,
+}) => {
+  const trigger = await mount(page);
+  await trigger.click();
+  await expect(page.getByRole('dialog')).toBeVisible();
+  await row(page).click();
+  await expectReturnedFocus(page, trigger, false);
+});
+
+test('a keyboard-opened sheet closed by Escape keeps the ring', async ({
+  page,
+}) => {
+  const trigger = await mount(page);
+  await openFromKeyboard(page, trigger);
+  await page.keyboard.press('Escape');
+  await expectReturnedFocus(page, trigger, true);
+});

packages/core/src/Dialog/Dialog.tsx

@@ -70,38 +70,10 @@ import {DialogContext} from './DialogContext';
 import {themeProps} from '../utils/themeProps';
 import type {DialogVariantMap} from './index';
 import {focusOutlineProps} from '../utils/focusOutline.stylex';
-import {
-  getInteractionModality,
-  useInteractionModalityTracking,
-} from '../utils/interactionModality';
+import {useInteractionModalityTracking} from '../utils/interactionModality';
+import {focusForLastInput} from '../utils/focusReturn';
 
 import {useMergedRefs} from '../hooks/useMergedRefs';
-/**
- * Move focus programmatically so its ring follows the last input. Dialog does
- * not render the element that opened it or the consumer's autofocus target, so
- * the browser's focus option is the one way to reach them.
- */
-function focusForLastInput(element: HTMLElement): void {
-  // A text-entry field paints its own focus treatment to show where typing
-  // goes, and an extra blur there can run validation or mark it touched.
-  const isTextEntry =
-    element instanceof HTMLInputElement ||
-    element instanceof HTMLTextAreaElement ||
-    element instanceof HTMLSelectElement ||
-    element.isContentEditable;
-  if (isTextEntry || getInteractionModality() !== 'pointer') {
-    element.focus();
-    return;
-  }
-  // Focusing the element that already has focus does nothing, so a ring the
-  // browser already drew would stay. WebKit draws one when showModal() or its
-  // native close moves focus itself, so clear that focus first.
-  if (element.matches(':focus-visible')) {
-    element.blur();
-  }
-  element.focus({focusVisible: false});
-}
-
 /**
  * Calculate a directional translate offset for dialog entry animation.
  * Returns a normalized vector from the trigger element toward the viewport