[ER-49165] Fix focus recursion between Drawer and Modal

.changeset/drawer-modal-focus-recursion.md

@@ -0,0 +1,8 @@
+---
+'@toptal/picasso-drawer': patch
+'@toptal/picasso-modal': patch
+'@toptal/picasso-utils': minor
+'@toptal/picasso': patch
+---
+
+- fix `Maximum call stack size exceeded` when a `Drawer` is opened inside an open `Modal`. `Modal` opts out of MUI's `FocusTrap` and uses its own document-level focus listener while `Drawer` keeps MUI's, so the two mechanisms recurse on `Element.focus()`. `Drawer` now registers with the shared `ModalManager` singleton, letting the underlying `Modal` bail out of its focus listener when a `Drawer` is on top.

packages/base/Drawer/package.json

@@ -47,8 +47,10 @@
     ".": "./dist-package/src/index.js"
   },
   "devDependencies": {
+    "@toptal/picasso-modal": "4.0.0",
     "@toptal/picasso-provider": "5.0.2",
-    "@toptal/picasso-tailwind-merge": "2.0.4"
+    "@toptal/picasso-tailwind-merge": "2.0.4",
+    "@toptal/picasso-test-utils": "2.0.0"
   },
   "files": [
     "dist-package/**",

packages/base/Drawer/src/Drawer/Drawer.tsx

@@ -1,4 +1,4 @@
-import React, { useRef } from 'react'
+import React, { useEffect, useRef } from 'react'
 import { Modal } from '@mui/base/Modal'
 import type { BaseProps, TransitionProps } from '@toptal/picasso-shared'
 import { useDrawer, usePicassoRoot } from '@toptal/picasso-provider'
@@ -9,6 +9,8 @@ import { Slide } from '@toptal/picasso-slide'
 import { Backdrop } from '@toptal/picasso-backdrop'
 import { Container } from '@toptal/picasso-container'
 import {
+  defaultModalManager,
+  generateModalId,
   useIsomorphicLayoutEffect,
   usePageScrollLock,
 } from '@toptal/picasso-utils'
@@ -81,6 +83,7 @@ export const Drawer = ({
   const { setHasDrawer } = useDrawer()
   const container = usePicassoRoot()
   const ref = useRef<HTMLDivElement>(null)
+  const drawerId = useRef(generateModalId())
 
   usePageScrollLock(Boolean(maintainBodyScrollLock && open))
 
@@ -94,6 +97,20 @@ export const Drawer = ({
     return cleanup
   }, [open, setHasDrawer])
 
+  // Register with the shared overlay manager so a Picasso Modal below
+  // yields focus enforcement to this Drawer (see ER-49165).
+  useEffect(() => {
+    const id = drawerId.current
+
+    if (open) {
+      defaultModalManager.add(id)
+    }
+
+    return () => {
+      defaultModalManager.remove(id)
+    }
+  }, [open])
+
   const handleOnClose = () => {
     if (onClose) {
       onClose()

packages/base/Drawer/src/Drawer/test.tsx

@@ -0,0 +1,147 @@
+import React from 'react'
+import { afterEach, describe, expect, it, jest } from '@jest/globals'
+import { render, cleanup } from '@toptal/picasso-test-utils'
+import { Modal } from '@toptal/picasso-modal'
+import { defaultModalManager } from '@toptal/picasso-utils'
+
+import Drawer from './Drawer'
+
+describe('Drawer', () => {
+  afterEach(() => {
+    cleanup()
+    defaultModalManager.reset()
+  })
+
+  it('renders without crashing', () => {
+    const { baseElement } = render(<Drawer open>content</Drawer>)
+
+    expect(baseElement).toBeInTheDocument()
+  })
+
+  describe('overlay registration', () => {
+    it('registers with defaultModalManager when open', () => {
+      render(<Drawer open>content</Drawer>)
+      expect(defaultModalManager.modals).toHaveLength(1)
+    })
+
+    it('does not register when closed', () => {
+      render(<Drawer open={false}>content</Drawer>)
+      expect(defaultModalManager.modals).toHaveLength(0)
+    })
+
+    it('removes itself from defaultModalManager on unmount', () => {
+      const { unmount } = render(<Drawer open>content</Drawer>)
+
+      expect(defaultModalManager.modals).toHaveLength(1)
+      unmount()
+      expect(defaultModalManager.modals).toHaveLength(0)
+    })
+
+    it('updates registration when open prop toggles', () => {
+      const { rerender } = render(<Drawer open>content</Drawer>)
+
+      expect(defaultModalManager.modals).toHaveLength(1)
+
+      rerender(<Drawer open={false}>content</Drawer>)
+      expect(defaultModalManager.modals).toHaveLength(0)
+
+      rerender(<Drawer open>content</Drawer>)
+      expect(defaultModalManager.modals).toHaveLength(1)
+    })
+  })
+
+  // ER-49165: Picasso Modal uses a custom document focus listener (because it
+  // sets disableEnforceFocus on MUI's Modal) while Drawer relies on MUI's
+  // FocusTrap. With both active, the two focus mechanisms recursed on
+  // Element.focus() and raised "Maximum call stack size exceeded".
+  describe('when nested inside a Picasso Modal (ER-49165)', () => {
+    it('takes over as topmost overlay in the shared manager', () => {
+      // Mount Modal first so its id is unambiguous in the manager — relying
+      // on the stack position after a simultaneous mount of both would be
+      // fragile against MUI's internal mount lifecycle.
+      const { rerender } = render(<Modal open>modal content</Modal>)
+      const modalId = defaultModalManager.modals[0]
+
+      expect(defaultModalManager.isTopModal(modalId)).toBe(true)
+
+      rerender(
+        <Modal open>
+          <Drawer open>drawer content</Drawer>
+        </Modal>
+      )
+
+      // Modal stayed mounted, only Drawer fired its add, so the stack is
+      // [modalId, drawerId] with the Drawer on top.
+      expect(defaultModalManager.modals).toHaveLength(2)
+      expect(defaultModalManager.modals[0]).toBe(modalId)
+      const drawerId = defaultModalManager.modals[1]
+
+      expect(defaultModalManager.isTopModal(drawerId)).toBe(true)
+      expect(defaultModalManager.isTopModal(modalId)).toBe(false)
+    })
+
+    it('makes the Modal listener bail out on document focus events', () => {
+      const focusSpy = jest.spyOn(HTMLElement.prototype, 'focus')
+      // Spying on isTopModal lets us prove the Modal listener actually ran —
+      // without this, a harness regression (e.g. the listener not attached or
+      // the synthetic event not reaching it) would silently pass the
+      // negative .focus() assertion below.
+      const isTopModalSpy = jest.spyOn(defaultModalManager, 'isTopModal')
+
+      render(
+        <Modal open>
+          <input data-testid='modal-input' />
+          <Drawer open>
+            <input data-testid='drawer-input' />
+          </Drawer>
+        </Modal>
+      )
+
+      // Discard focus calls fired during mount (auto-focus, MUI FocusTrap).
+      focusSpy.mockClear()
+      isTopModalSpy.mockClear()
+
+      // Re-fire the document focus listener path that previously recursed.
+      document.dispatchEvent(new FocusEvent('focus', { bubbles: true }))
+
+      // Proof the Modal listener actually fired and queried the manager.
+      // isTopModal is only called from inside Modal's focus handler, so a
+      // call here means the handler ran end-to-end up to the bailout check.
+      expect(isTopModalSpy).toHaveBeenCalled()
+      // And at least one of those calls returned false (the Modal's own id,
+      // since Drawer is on top), exercising the bailout branch.
+      expect(isTopModalSpy.mock.results.map(result => result.value)).toContain(
+        false
+      )
+
+      // With the bailout taken, Modal's handler does not invoke
+      // focusFirstFocusableElement, so no .focus() calls originate from it.
+      expect(focusSpy).not.toHaveBeenCalled()
+
+      focusSpy.mockRestore()
+      isTopModalSpy.mockRestore()
+    })
+
+    it('restores Modal as topmost when the Drawer closes', () => {
+      const { rerender } = render(<Modal open>modal content</Modal>)
+      const modalId = defaultModalManager.modals[0]
+
+      rerender(
+        <Modal open>
+          <Drawer open>drawer content</Drawer>
+        </Modal>
+      )
+
+      expect(defaultModalManager.isTopModal(modalId)).toBe(false)
+
+      rerender(
+        <Modal open>
+          <Drawer open={false}>drawer content</Drawer>
+        </Modal>
+      )
+
+      expect(defaultModalManager.modals).toHaveLength(1)
+      expect(defaultModalManager.isTopModal(modalId)).toBe(true)
+    })
+  })
+})

packages/base/Modal/src/Modal/Modal.tsx

@@ -16,7 +16,8 @@ import { usePicassoRoot, RootContext } from '@toptal/picasso-provider'
 import { CloseMinor16 } from '@toptal/picasso-icons'
 import {
   useCombinedRefs,
-  ModalManager,
+  defaultModalManager,
+  generateModalId,
   usePageScrollLock,
 } from '@toptal/picasso-utils'
 import { ButtonCircular } from '@toptal/picasso-button'
@@ -66,8 +67,6 @@ export interface Props extends BaseProps, HTMLAttributes<HTMLDivElement> {
   }
 }
 
-const defaultManager = new ModalManager()
-
 // https://github.com/udacity/ud891/blob/gh-pages/lesson2-focus/07-modals-and-keyboard-traps/solution/modal.js#L25
 // found in https://developers.google.com/web/fundamentals/accessibility/focus/using-tabindex
 const focusableElementsString =
@@ -112,12 +111,6 @@ const isFocusInsideTooltip = () => {
   return false
 }
 
-const generateKey = (() => {
-  let count = 0
-
-  return () => ++count
-})()
-
 // eslint-disable-next-line react/display-name
 export const Modal = forwardRef<HTMLDivElement, Props>(function Modal(
   {
@@ -150,7 +143,7 @@ export const Modal = forwardRef<HTMLDivElement, Props>(function Modal(
     ref,
     useRef<HTMLDivElement>(null)
   )
-  const modalId = useRef(generateKey())
+  const modalId = useRef(generateModalId())
   const { rootRef } = useContext(RootContext)
 
   useEffect(() => {
@@ -161,7 +154,7 @@ export const Modal = forwardRef<HTMLDivElement, Props>(function Modal(
         )
       }
 
-      if (!defaultManager.isTopModal(modalId.current)) {
+      if (!defaultModalManager.isTopModal(modalId.current)) {
         return
       }
 
@@ -195,11 +188,11 @@ export const Modal = forwardRef<HTMLDivElement, Props>(function Modal(
     const currentModalId = modalId.current
 
     if (open) {
-      defaultManager.add(currentModalId)
+      defaultModalManager.add(currentModalId)
     }
 
     return () => {
-      defaultManager.remove(currentModalId)
+      defaultModalManager.remove(currentModalId)
     }
   }, [open])
 

packages/base/Utils/src/utils/Modal/index.ts

@@ -1,2 +1,6 @@
-export { ModalManager } from './modal-manager'
+export {
+  ModalManager,
+  defaultModalManager,
+  generateModalId,
+} from './modal-manager'
 export { useModal } from './use-modal'

packages/base/Utils/src/utils/Modal/modal-manager.ts

@@ -24,4 +24,18 @@ export class ModalManager {
       this.modals.length > 0 && this.modals[this.modals.length - 1] === modalId
     )
   }
+
+  /** @internal Test-only helper to reset state between specs. */
+  reset() {
+    this.modals.length = 0
+  }
 }
+
+// Shared across Picasso overlay components (Modal, Drawer) so the topmost
+// overlay's focus mechanism wins and nested overlays don't fight each other.
+// See: https://toptal-core.atlassian.net/browse/ER-49165
+export const defaultModalManager = new ModalManager()
+
+let modalIdCounter = 0
+
+export const generateModalId = () => ++modalIdCounter