Skip to content

Commit 2991179

Browse files
devongovettDevon Govett
andauthored
Make RAC overlay open props override DialogTrigger state (#4684)
Co-authored-by: Devon Govett <[email protected]>
1 parent a4f8ec3 commit 2991179

File tree

4 files changed

+58
-5
lines changed

4 files changed

+58
-5
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ export {_Modal as Modal};
9393

9494
function ModalOverlayWithForwardRef(props: ModalOverlayProps, ref: ForwardedRef<HTMLDivElement>) {
9595
let ctx = useContext(ModalContext);
96-
// eslint-disable-next-line react-hooks/rules-of-hooks
97-
let state = ctx?.state ?? useOverlayTriggerState(props);
96+
let localState = useOverlayTriggerState(props);
97+
let state = props.isOpen != null || props.defaultOpen != null || !ctx?.state ? localState : ctx.state;
9898

9999
let objectRef = useObjectRef(ref);
100100
let modalRef = useRef<HTMLDivElement>(null);

packages/react-aria-components/src/Popover.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,8 @@ export const PopoverContext = createContext<ContextValue<PopoverContextValue, HT
5656
function Popover(props: PopoverProps, ref: ForwardedRef<HTMLElement>) {
5757
[props, ref] = useContextProps(props, ref, PopoverContext);
5858
let ctx = props as PopoverContextValue;
59-
// React components cannot be reparented so if there's context there should always be context.
60-
// eslint-disable-next-line react-hooks/rules-of-hooks
61-
let state = ctx?.state ?? useOverlayTriggerState(props);
59+
let localState = useOverlayTriggerState(props);
60+
let state = props.isOpen != null || props.defaultOpen != null || !ctx?.state ? localState : ctx.state;
6261
let isExiting = useExitAnimation(ref, state.isOpen);
6362

6463
if (state && !state.isOpen && !isExiting) {

packages/react-aria-components/test/Dialog.test.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,4 +186,39 @@ describe('Dialog', () => {
186186

187187
expect(dialog).not.toBeInTheDocument();
188188
});
189+
190+
it('should support Modal being used standalone', async () => {
191+
let onOpenChange = jest.fn();
192+
let {getByRole} = render(
193+
<Modal isDismissable isOpen onOpenChange={onOpenChange}>
194+
<Dialog>A modal</Dialog>
195+
</Modal>
196+
);
197+
198+
let dialog = getByRole('dialog');
199+
expect(dialog).toHaveTextContent('A modal');
200+
201+
userEvent.click(document.body);
202+
expect(onOpenChange).toHaveBeenCalledTimes(1);
203+
expect(onOpenChange).toHaveBeenCalledWith(false);
204+
});
205+
206+
it('isOpen and defaultOpen should override state from context', async () => {
207+
let onOpenChange = jest.fn();
208+
let {getByRole} = render(<>
209+
<DialogTrigger>
210+
<Button />
211+
<Modal isDismissable isOpen onOpenChange={onOpenChange}>
212+
<Dialog>A modal</Dialog>
213+
</Modal>
214+
</DialogTrigger>
215+
</>);
216+
217+
let dialog = getByRole('dialog');
218+
expect(dialog).toHaveTextContent('A modal');
219+
220+
userEvent.click(document.body);
221+
expect(onOpenChange).toHaveBeenCalledTimes(1);
222+
expect(onOpenChange).toHaveBeenCalledWith(false);
223+
});
189224
});

packages/react-aria-components/test/Popover.test.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,23 @@ describe('Popover', () => {
9999
expect(onOpenChange).toHaveBeenCalledTimes(1);
100100
expect(onOpenChange).toHaveBeenCalledWith(false);
101101
});
102+
103+
it('isOpen and defaultOpen should override state from context', async () => {
104+
let onOpenChange = jest.fn();
105+
let {getByRole} = render(<>
106+
<DialogTrigger>
107+
<Button />
108+
<Popover isOpen onOpenChange={onOpenChange}>
109+
<Dialog>A popover</Dialog>
110+
</Popover>
111+
</DialogTrigger>
112+
</>);
113+
114+
let dialog = getByRole('dialog');
115+
expect(dialog).toHaveTextContent('A popover');
116+
117+
userEvent.click(document.body);
118+
expect(onOpenChange).toHaveBeenCalledTimes(1);
119+
expect(onOpenChange).toHaveBeenCalledWith(false);
120+
});
102121
});

0 commit comments

Comments
 (0)