fix(components): addresses the modal closing when toasts appear bug #1829
Conversation
🦋 Changeset detectedLatest commit: 4809ea9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
|
Size Change: +150 B (+0.03%) Total Size: 533 kB
ℹ️ View Unchanged
|
| return ( | ||
| <AriaModalOverlay | ||
| isDismissable={isDismissable} | ||
| shouldCloseOnInteractOutside={(element) => { |
There was a problem hiding this comment.
@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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@ari-launchdarkly any input on the second point about downstream effects to consumers + iframes?
There was a problem hiding this comment.
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 ?
ari-launchdarkly
deleted the
ari-launchdarkly/REL-11286-fix-toast-click-bug
branch
Summary
Problem
The crux of the problem here is that react-aria uses a portal mechanism to set toasts onto when they are appended onto the DOM. When someone triggers a toast, a HTML tag is appended to the DOM via the portal, so spamming the same button results in the second click (after the toast is initialized) from being set onto the HTML tag, which is "outside" the dialog and causes it to collapse. This solution works by listening to the element that's being clicked on and preventing the collapse when that's a BODY or HTML tag.
CURRENT BUG:
https://www.loom.com/share/98745d81ab324abeb91cdf522b5c47c5
SOLUTION:
https://www.loom.com/share/6a0880c893184250a6a0b671eb4d5a66
Screenshots (if appropriate):
Testing approaches
Related Jira issue: REL-11286: BUG: Clicking a dialog when a toast is up closes the dialog