[CLOV-1606][BpkModalV3] Fix infinite focus redirect loop when BpkModalV3 and BpkDrawer coexist

[Faye (Faye-Xiao)]

Description

When a BpkDrawer (which uses the legacy withScrim/focusScope focus management system) is open at the same time as a BpkModalV3 (which uses ark-ui's @zag-js/focus-trap), both systems simultaneously listen to focusin on document. This causes an infinite focus redirect loop that overflows the call stack.

Root cause: focusScope registers a bubble-phase focusin listener that synchronously calls element.focus(). @zag-js/focus-trap registers a capture-phase listener but does not call stopImmediatePropagation when focus is already inside the modal. The result is a recursive focusin dispatch loop (~8 frames per iteration) that overflows the call stack after ~100–200 iterations.

Fix: When BpkModalV3Root opens, it calls focusScope.unscopeFocus() to deactivate the legacy listener before ark-ui's focus trap takes over. This is a compatibility patch that keeps both systems stable while BpkDrawer has not yet been migrated to ark-ui.

Changes

• BpkModalV3Root.tsx: Call focusScope.unscopeFocus() in a useEffect when isOpen becomes true, with a comment explaining why.
• BpkModalV3-test.tsx: Mock focusScope and add 3 test cases covering: modal opens (unscopeFocus called), modal closed (unscopeFocus not called), and closed → open transition.
• BpkModalV3.stories.tsx: Add WithDrawer story demonstrating BpkModalV3 opened from within a BpkDrawer.

Test Screenshots

Tested it in local carhire

Platform	Before	After
mWeb
destop

• no error in mWeb
• no change in desktop, no issue with keyboard management in desktop

Checklist

• Ensure the PR title includes the name of the component you are changing so it's clear in the release notes for consumers of the changes in the version e.g [CLOV-1606][BpkModalV3] Fix infinite focus redirect loop when BpkModalV3 and BpkDrawer coexist
• README.md (If you have created a new component) — N/A, no new component
• Component README.md — N/A
• Tests
• Accessibility tests
  • The following checks were performed:
    • Ability to navigate using a keyboard only — this fix restores correct keyboard focus when both components coexist
    • Zoom functionality (Deque University explanation):
      • The page SHOULD be functional AND readable when only the text is magnified to 200% of its initial size
      • Pages must reflow as zoom increases up to 400% so that content continues to be presented in only one column i.e. Content MUST NOT require scrolling in two directions (both vertically and horizontally)
    • Ability to navigate using a screen reader only
• Storybook examples created/updated — WithDrawer story added to bpk-component-modal-v3
• For breaking changes or deprecating components/properties, migration guides added to the description of the PR — N/A, this is a bug fix

[skyscanner-backpack-bot]

Visit https://backpack.github.io/storybook-prs/4635 to see this build running in a browser.

[skyscanner-backpack-bot]

	Warnings
⚠️	Package source files (e.g. packages/package-name/src/Component.js) were updated, but snapshots weren't. Have you checked that the tests still pass?

Browser support

If this is a visual change, make sure you've tested it in multiple browsers.

Generated by 🚫 dangerJS against a91f499

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[skyscanner-backpack-bot]

Visit https://backpack.github.io/storybook-prs/4635 to see this build running in a browser.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[skyscanner-backpack-bot]

Visit https://backpack.github.io/storybook-prs/4635 to see this build running in a browser.

[skyscanner-backpack-bot]

Visit https://backpack.github.io/storybook-prs/4635 to see this build running in a browser.

[Ezreal Yang (Supremeyh)]

Check on story, it seems cannot close drawer with keyboard Esc and when close the modal, it would flash fast, is it expected?

Screen.Recording.2026-05-27.at.11.43.04.mov

[Ezreal Yang (Supremeyh)]

This is the main concern with the current implementation.

Looking at withScrim.tsx:114-115, the drawer only calls focusScope.scopeFocus(this.dialogElement) once — in componentDidMount. It has no mechanism to re-register after an external caller clears the global state.

When BpkModalV3 closes (drawer still mounted), the sequence is:

1. BpkModalV3Root opens → focusScope.unscopeFocus() called ✅
2. ark-ui focus trap takes over the modal ✅
3. User closes BpkModalV3 → BpkModalV3Root unmounts / isOpen becomes false
4. focusScope listener is gone and never restored ❌
5. The drawer is still visually open but focus can now escape it ❌
   The PR trades a crash for a silent a11y regression. On desktop this matters most: keyboard users can now Tab out of the drawer after the modal closes.

Suggested fix — save and restore the scope element:

// BpkModalV3Root.tsx
const scopedElementRef = useRef<HTMLElement | null>(null);

useEffect(() => {
  if (isOpen) {
    // Save current focus scope element before clearing it
    // (focusScope doesn't expose its element directly — see note below)
    focusScope.unscopeFocus();
  } else if (scopedElementRef.current) {
    // Re-activate the saved scope when modal closes
    focusScope.scopeFocus(scopedElementRef.current);
    scopedElementRef.current = null;
  }
}, [isOpen]);

However, focusScope.ts doesn't expose the currently scoped element, so this would require either:

• Option A: Add getScopedElement(): HTMLElement | null to focusScope API, or
• Option B: Add pauseFocus() / resumeFocus() to focusScope that internally preserves teardownFn, or
• Option C (minimal): withScrim re-calls scopeFocus in componentDidUpdate when the drawer is still open

[Faye (Faye-Xiao)]

Good catch. Fixed in the latest commit by replacing unscopeFocus() with pauseFocus() / resumeFocus() pattern.

When the modal opens, pauseFocus() removes the focusin listener but keeps the element reference. When the modal closes, resumeFocus() re-registers the listener without calling focus() immediately, so ark-ui's own focus-return logic runs first and there is no focus jump.

The key change is in focusScope.ts: extracted a registerListener() helper that only attaches the listener (no initial focus steal), which resumeFocus() uses.

[Ezreal Yang (Supremeyh)]

It imports focusScope directly from a sibling package's src/ path. This creates a hard coupling between bpk-component-modal and the internals of bpk-scrim-utils. The import path ../../../../bpk-scrim-utils/src/focusScope bypasses the package boundary. If bpk-scrim-utils reorganizes its internals, BpkModalV3Root breaks silently.

Minor suggestion: at minimum, export focusScope from bpk-scrim-utils' package index so this reads as an inter-package dependency, not an internal-path hack.

[Faye (Faye-Xiao)]

Fixed. Added focusScope as a named export in bpk-scrim-utils/index.ts, and updated the import in BpkModalV3Root to use the package index instead of the src path.

[Ezreal Yang (Supremeyh)]

Tests are correct for what they test. Three concerns:

• There is no test for the close path: the tests verify unscopeFocus is called on open, but there's no test asserting behaviour when the modal closes while a drawer is active. This is precisely the gap where the a11y regression lives.

• scopeFocus mock is registered but never asserted in any test — suggests the author considered testing restore behavior but didn't implement it.

• The beforeEach clears only unscopeFocus but not scopeFocus. If a future test for restore is added, the mock should clear both:

beforeEach(() => {
  (focusScope.unscopeFocus as jest.Mock).mockClear();
  (focusScope.scopeFocus as jest.Mock).mockClear(); // add this
});

[Faye (Faye-Xiao)]

All three points fixed:

• Added a test for the close path: should call focusScope.resumeFocus when modal transitions from open to closed
• The scopeFocus mock is now also cleared in beforeEach alongside pauseFocus and resumeFocus
• Updated all existing tests from unscopeFocus to pauseFocus / resumeFocus to match the new implementation

[Ezreal Yang (Supremeyh)]

The WithDrawerExample story is excellent — it clearly demonstrates the exact failure scenario and serves as a regression canary. One nit: the story doesn't close the modal and verify the drawer still traps focus, which would expose the regression mentioned above.

[Faye (Faye-Xiao)]

Added verification steps to the modal body text: after closing the modal, press Tab inside the drawer and check that focus stays trapped. This makes it easy to test the post-close behaviour manually in Storybook.

[skyscanner-backpack-bot]

Visit https://backpack.github.io/storybook-prs/4635 to see this build running in a browser.

[Faye (Faye-Xiao)]

Thanks for the review! The Esc key and flash issue — I will verify manually in the Storybook. Will update here after testing.

[skyscanner-backpack-bot]

Visit https://backpack.github.io/storybook-prs/4635 to see this build running in a browser.

[skyscanner-backpack-bot]

Visit https://backpack.github.io/storybook-prs/4635 to see this build running in a browser.

[skyscanner-backpack-bot]

Visit https://backpack.github.io/storybook-prs/4635 to see this build running in a browser.

[skyscanner-backpack-bot]

Visit https://backpack.github.io/storybook-prs/4635 to see this build running in a browser.