adobe / adobe/react-spectrum

Tailwind starter Modal forwards overlay props to nested Modal

Open
#10,026 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

# 🐛 Bug Report

## Provide a general summary of the issue here

The Tailwind starter `Modal` wrapper composes `ModalOverlay` and `Modal` together, but currently spreads the same props onto both components:

https://github.com/adobe/react-spectrum/blob/a2b69fbc3f291e51219e07ec95eabedc49350901/starters/tailwind/src/Modal.tsx#L30-L35

When overlay-control props such as `isOpen`, `onOpenChange`, `isDismissable`, or `shouldCloseOnInteractOutside` are passed, they are forwarded to the inner `Modal` even though it is already inside a `ModalOverlay`.

React Aria’s `Modal` implementation warns about this nested usage:

https://github.com/adobe/react-spectrum/blob/a2b69fbc3f291e51219e07ec95eabedc49350901/packages/react-aria-components/src/Modal.tsx#L94-L101

## 🤔 Expected Behavior?

The starter wrapper should keep overlay-control props on `ModalOverlay` and pass only modal content props to the inner `Modal`, matching React Aria’s own standalone `Modal` implementation:

https://github.com/adobe/react-spectrum/blob/a2b69fbc3f291e51219e07ec95eabedc49350901/packages/react-aria-components/src/Modal.tsx#L104-L132

## 😯 Current Behavior

The starter forwards the same props to both `ModalOverlay` and the inner `Modal`. In development mode, React Aria can warn that the modal is already wrapped in a `ModalOverlay` and that overlay props should be placed on the `ModalOverlay` instead.

Warning text from React Aria:

```text
This modal is already wrapped in a ModalOverlay, props [...] should be placed on the ModalOverlay instead.
```

## 💁 Possible Solution

Destructure the overlay-control props in the starter `Modal` wrapper. Keep them on `ModalOverlay`, and pass the remaining modal content props to the inner `Modal`. Render `children` explicitly inside the inner `Modal`.

## 🔦 Context

I noticed this while using a local Modal wrapper derived from the Tailwind starter. The warning appears because the starter uses the nested `ModalOverlay > Modal` composition but forwards props as if the inner `Modal` were being used standalone.

## 🖥️ Steps to Reproduce

1. Use the Tailwind starter `Modal` wrapper from current `main`.
2. Pass overlay-control props such as `isOpen`, `onOpenChange`, and `isDismissable` to the starter `Modal`.
3. Render it in development mode.
4. Observe that those props are also forwarded to the inner React Aria `Modal`, triggering the nested `ModalOverlay` warning.

## 🌍 Your Environment

### Version

Current `main` branch at a2b69fbc3f291e51219e07ec95eabedc49350901.

### What browsers are you seeing the problem on?

Browser-independent; this follows from the starter wrapper implementation and React Aria development warning path.

### If other, please specify.

Not applicable.

### What operating system are you using?

macOS.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.