Focus trap doesn't seem to be working in Modal in React Aria Components

[MarekBodingerBA]

Hello,

I didn't want to create the issue, because there might be some easily catchable issue in the code I am not able to see. Basically, my modal doesn't trap the focus. I cannot really say whether it's an library issue or my code issue.

Modal component:
https://github.com/bratislava/konto.bratislava.sk/blob/56a7ea961480d2b19f69de19d1440a865b7efe8e/next/components/forms/simple-components/ModalV2.tsx#L19

Modal component usage:
https://github.com/bratislava/konto.bratislava.sk/blob/56a7ea961480d2b19f69de19d1440a865b7efe8e/next/components/forms/segments/SkipStepModal/SkipStepModal.tsx#L16
https://github.com/bratislava/konto.bratislava.sk/blob/56a7ea961480d2b19f69de19d1440a865b7efe8e/next/components/styleguide/showcases/ModalShowCase.tsx#L284

Reproduction:
https://city-account-next.staging.bratislava.sk/styleguide#modal -> Open skip step modal

Thank you for help.

[LFDanLu]

From a glance, nothing really jumps out to me from your implementation. Mind logging out what value contain is returning after you open the Modal? I'm expecting it to be true after the Modal renders since it should be called by useDialog

[MarekBodingerBA]

This seems to be the reason, the hook which tries to get the OverlayContext is above the context itself in the component tree.

[MarekBodingerBA]

Context get:

react-spectrum/packages/react-aria-components/src/Modal.tsx

Line 131 in be50125

let {modalProps, underlayProps} = useModalOverlay(props, state, modalRef);

->

react-spectrum/packages/@react-aria/overlays/src/useModalOverlay.ts

Line 57 in be50125

useOverlayFocusContain();

->

react-spectrum/packages/@react-aria/overlays/src/Overlay.tsx

Line 78 in be50125

let ctx = useContext(OverlayContext);

Context set:

react-spectrum/packages/react-aria-components/src/Modal.tsx

Line 151 in be50125

<Overlay isExiting={props.isExiting}>

[LFDanLu]

Odd that the context is always null, useOverlayFocusContain should be also called by the Dialog which should have access to the OverlayContext.Provider you show in the screenshot above since it is a child of the Modal...

[LFDanLu]

Ah I checked your yarn.lock, it could be due to multiple versions of @react-aria/overlay, resulting in multiple versions of the OverlayContext. This means the OverlayContext setup by the Modal's Overlay might be a different one that the Dialog within tries to access, hence the null value for ctx in useOverlayFocusContain. Could you confirm if this might be the case?

[MarekBodingerBA]

Oh, thank you! I deleted the unnecessary @react-aria/overlays package and it works. 💪

I would like to use the thread to also ask a related question. Is this needed for modal in SSR?

const isSSR = useIsSSR()

if (isSSR) {
  return null
}

The error is missing window, isn't there an internal check for SSR?

ReferenceError: window is not defined
    at $5df64b3807dc15ee$var$getViewportSize (file:///C:/Projects/konto.bratislava.sk/next/node_modules/@react-aria/utils/dist/import.mjs:811:172)
    at file:///C:/Projects/konto.bratislava.sk/next/node_modules/@react-aria/utils/dist/import.mjs:790:52
    at useReducer (C:\Projects\konto.bratislava.sk\next\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5079:57)
    at Object.useState (C:\Projects\konto.bratislava.sk\next\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5017:10)

[MarekBodingerBA]

Also, is this onClose needed? Shouldn't it propagate to the modal?

[LFDanLu]

I would like to use the thread to also ask a related question. Is this needed for modal in SSR?

Oh interesting, our react-aria Overlay already has SSR handling built in but RAC Modal still has a hook call that relies on the window existing which is a problem for SSR. We should either change Modal so that hook call + the div it renders is in a child component of ModalOverlayInner so that Overlay wouldn't render it server side and/or make useViewportSize have a useIsSSR check within it as well. Mind filing an issue for that? Ideally, we would handle is useIsSSR check in as many places as necessary so you wouldn't have to do so.

Also, is this onClose needed? Shouldn't it propagate to the modal?

From a glance, that onClose is needed since you have a button that uses the close from the Dialog's render props which comes from either the Dialog's props.onClose applied on the Dialog directly or through a DialogContext that a DialogTrigger provides. Since you are directly controlling the Modal's open state without a DialogTrigger, what you are doing is correct. Perhaps we could have the Dialog access a Modal/Popover's context as a fallback though, will have to think about that.

[MarekBodingerBA]

Thank you, I've created the issue:
#4921