adobe / adobe/react-spectrum

DialogContainer drops disableFocusManagement - not forwarded to underlying Modal

Open Beginner friendly
#10,280 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
15.9k
Forks
1.6k
Avg merge
3d 9m
Merged PRs (30d)
59

Description

### Provide a general summary of the feature here

`DialogContainer` accepts and forwards `isDismissable` and `isKeyboardDismissDisabled` to the `Modal` overlay it renders, but it does not destructure or forward `disableFocusManagement`. As a result, a consumer that needs to opt out of the modal's internal `FocusScope` has no supported way to do so through `DialogContainer` — the prop is silently dropped, the `FocusScope` activates, and it steals focus on open.

`disableFocusManagement` is already supported on `Modal`/the underlying overlay; the only missing link is `DialogContainer` not passing it through. It is also absent from the `SpectrumDialogContainerProps` type, so there's no type-safe way to pass it even if it were forwarded.

### 🤔 Expected Behavior?

`DialogContainer` forwards `disableFocusManagement` to its `Modal`, mirroring how it already forwards `isDismissable` and `isKeyboardDismissDisabled`, and the prop is declared as an optional `boolean` on `SpectrumDialogContainerProps`.

### 😯 Current Behavior

The prop is neither typed nor forwarded. Passing `disableFocusManagement` to `DialogContainer` is a no-op: the value never reaches `Modal`, the `FocusScope` always runs, and focus is moved into the dialog on mount regardless.

### 💁 Possible Solution

Two small changes (this is exactly what we currently apply as a `patch-package` patch):

In `@react-spectrum/dialog` `DialogContainer`, destructure and forward the prop:

```tsx
let {
children,
type = 'modal',
onDismiss,
isDismissable,
isKeyboardDismissDisabled,
disableFocusManagement // add
} = props;

// ...
// add
```

In `@react-types/dialog`, add it to `SpectrumDialogContainerProps`:

```ts
export interface SpectrumDialogContainerProps {
// ...
isKeyboardDismissDisabled?: boolean,
/** Whether to disable focus management for the dialog (e.g. when an embedding host
manages focus itself). */
disableFocusManagement?: boolean
}
```

### 🔦 Context

We embed a host SDK that renders its own dialogs via `DialogContainer` and manages focus itself; it passes `disableFocusManagement={true}` so the dialog does not trap/steal focus on open. Because `DialogContainer` drops the prop, keyboard navigation (Tab / Enter / Space / Arrow keys) is broken for actions inside the dialog — a WCAG 2.1 AA keyboard-accessibility regression. We are working around it with a `patch-package` patch today and would like a native fix so we can remove the patch.

### 💻 Examples

Minimal repro:

```tsx
{}}>
content

// FocusScope still activates and moves focus into the dialog; disableFocusManagement is ignored.
```

### 🧢 Your Company/Team

Adobe/PandoraUI

### 🕷 Tracking Issue

_No response_

Contributor guide

Open the contributing guide

Research direction

Start in the @react-spectrum/dialog DialogContainer implementation and the @react-types/dialog SpectrumDialogContainerProps definition. Check how isDismissable and isKeyboardDismissDisabled are typed and passed to Modal, then make disableFocusManagement follow the same path and verify that the prop is optional and reaches the underlying overlay.

Written by the indexing model from the issue text.

Assessment

Tech stack
react, typescript
Domain
accessibility, frontend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.