Skip to content

fix(components): addresses the modal closing when toasts appear bug #1829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ari-launchdarkly merged 2 commits into from
Dec 18, 2025
Merged

fix(components): addresses the modal closing when toasts appear bug #1829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d28dd38
fix(components): addresses the modal closing when toasts appear bug
ari-launchdarkly Dec 16, 2025
4809ea9
chore(components): add a changeset
ari-launchdarkly Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fruity-points-agree.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@launchpad-ui/components": patch
---

fix a bug around autoclose behavior in modals when a toast is present
13 changes: 13 additions & 0 deletions packages/components/src/Modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,19 @@ const ModalOverlay = ({ isDismissable = true, ref, ...props }: ModalOverlayProps
return (
<AriaModalOverlay
isDismissable={isDismissable}
shouldCloseOnInteractOutside={(element) => {
Copy link
Contributor

@nhironaka nhironaka Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cspath1 will this negatively interact with toolbar?

Also curious if this will have a negative impact on public consumers (who may be rendering any number of iframes etc)

Copy link

@cspath1 cspath1 Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this change will negatively interact with the toolbar thankfully! As far as Modal usage within the toolbar is concerned, we aren't using the modal component (we have a popup window for the auth flow, since storing cookies via an iframe can cause issues).

More generally speaking though, I do think there is an existing issue with any sort of auto-close functionality when it comes to interacting with the toolbar (i.e. you wouldn't be able to keep the modal open and also interact with the toolbar without the modal closing). This isn't too big of a deal, unless you want to use the toolbar specifically to update flag values/track events that occur within said modal.

I'm curious if there's anything we (FM Next) can do to mitigate that sort of issue on our end.

Copy link
Contributor

@nhironaka nhironaka Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ari-launchdarkly any input on the second point about downstream effects to consumers + iframes?

Copy link
Contributor Author

@ari-launchdarkly ari-launchdarkly Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add a data-tag attribute onto the elements and exclude them in those cases. More specifically, if we set data-dev-toolbar as a property on the trigger, we could add a case for excluding that button click like we are for the current HTML or BODY one. Same might apply for the content of it (so after it's open, applying a specific data tag that we can exclude). I am curious though if event.preventDefault or event.stopPropagation wrapping the container might be just as effective. Are those components available in launchpad @nhironaka @cspath1 ?

// Don't close when the element being checked is HTML or BODY.
// This indicates a portal interaction (e.g., spawning a toast from a button).
//
// Why this is safe:
// - When users click the modal backdrop, the element is the overlay <div>, NOT HTML/BODY
// - HTML/BODY only appear here during React portal rendering edge cases
// - This prevents the modal from incorrectly closing when triggering toasts from modal buttons
// - Legitimate outside clicks (backdrop, other UI) still close the modal as expected
const isRootElement = element.tagName === 'HTML' || element.tagName === 'BODY';

return !isRootElement;
}}
{...props}
ref={ref}
className={composeRenderProps(props.className, (className, renderProps) =>
Expand Down
71 changes: 71 additions & 0 deletions packages/components/stories/Modal.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Heading } from '../src/Heading';
import { IconButton } from '../src/IconButton';
import { Modal, ModalOverlay } from '../src/Modal';
import { Text } from '../src/Text';
import { ToastRegion, toastQueue } from '../src/Toast';

const meta: Meta<typeof Modal> = {
component: Modal,
Expand Down Expand Up @@ -110,3 +111,73 @@ export const Drawer: Story = {
},
},
};

/**
* Bug reproduction: Dialog closes when clicking a button inside it while Toast is active.
*
* Steps to reproduce:
* 1. Click "Show Toast" to display a toast notification
* 2. Click "Open Dialog" to open the modal
* 3. Click "Click Me" button inside the dialog
*
* Expected: Button click should work normally, dialog stays open
* Actual: Dialog closes unexpectedly
*/
export const DialogWithActiveToast: Story = {
render: (args) => {
return (
<>
<ToastRegion />
<div style={{ display: 'flex', gap: '1rem', marginBottom: '1rem' }}>
<DialogTrigger>
<Button variant="primary">Open Dialog</Button>
<ModalOverlay>
<Modal {...args}>
<Dialog>
{({ close }) => (
<>
<div slot="header">
<Heading slot="title">Dialog with Toast Active</Heading>
<IconButton
aria-label="close"
icon="cancel"
size="small"
variant="minimal"
onPress={close}
/>
<Text slot="subtitle">Try clicking the button below with toast active</Text>
</div>
<div slot="body">
<p>
When a toast is visible, clicking the button below should NOT close the
dialog.
</p>
<div style={{ marginTop: '1rem' }}>
<Button
onPress={() => {
toastQueue.add({
title: 'Toast is active',
description: 'Now try clicking the button inside the dialog',
status: 'info',
});
}}
>
Show Toast
</Button>{' '}
</div>
</div>
<div slot="footer">
<Button slot="close">Cancel</Button>
<Button variant="primary">Confirm</Button>
</div>
</>
)}
</Dialog>
</Modal>
</ModalOverlay>
</DialogTrigger>
</div>
</>
);
},
};
Loading