Skip to content
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

Jump to bottom

React dismissable layer @1.0.5 bug #2597

Closed
ugokoli opened this issue Dec 18, 2023 · 4 comments · May be fixed by #3087
Closed

React dismissable layer @1.0.5 bug #2597

ugokoli opened this issue Dec 18, 2023 · 4 comments · May be fixed by #3087

Comments

@ugokoli
Copy link

ugokoli commented Dec 18, 2023

Bug report

The dom body element receives the pointer-events: none when AlertDialog is opened and remains after closing. This causes the whole page to freeze and not respond to any user interaction.

Current Behavior

I have tested it to be happening on AlertDialog component only when using the api to open and close the dialog ie. open and onOpenChanged props. It works fine when using Trigger component

Expected behavior

Let user interaction not be blocked after interacting with the following components AlertdDialog, HoverCard, Popover and Toast

Suggested solution

I traced the issue to come from the @radix-ui/react-dismissable-layer library, version 1.0.5 Precisely.
I have tried version 1.0.3 of @radix-ui/react-dismissable-layer and the bug does not happen on that version.
I had to downgrade other components that use @radix-ui/react-dismissable-layer like AlertdDialog, HoverCard, Popover and Toast to previous versions that depend on 1.0.3 to have it working again.

Can the issue be found and fixed in version 1.0.6 of @radix-ui/react-dismissable-layer and maintained to never happen again?

Additional context

It relates to this other issue #1241
@robintown
Copy link

See #1241 (comment): this is probably a case of mismatched versions of @radix-ui/react-dismissable-layer, rather than a bug specific to version 1.0.5.

@StephenHaney
Copy link
Collaborator

Going to close this one, canonical issue: #1088

@danieltroger
Copy link

danieltroger commented Aug 27, 2024 •
edited
Loading

We do not have duplicate versions of @radix-ui/react-dismissable-layer and are also suffering from this bug

(venv) daniel@mmmmmmmmmm app % yarn why @radix-ui/react-dismissable-layer        
├─ @radix-ui/react-dialog@npm:1.1.1
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 (via npm:1.1.0)
│
├─ @radix-ui/react-dialog@npm:1.1.1 [358d6]
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 [9dd81] (via npm:1.1.0 [9dd81])
│
├─ @radix-ui/react-dialog@npm:1.1.1 [a4987]
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 [34378] (via npm:1.1.0 [34378])
│
├─ @radix-ui/react-dialog@npm:1.1.1 [bad1d]
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 [a9d43] (via npm:1.1.0 [a9d43])
│
├─ @radix-ui/react-hover-card@npm:1.1.1
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 (via npm:1.1.0)
│
├─ @radix-ui/react-hover-card@npm:1.1.1 [0312d]
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 [34378] (via npm:1.1.0 [34378])
│
├─ @radix-ui/react-menu@npm:2.1.1
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 (via npm:1.1.0)
│
├─ @radix-ui/react-menu@npm:2.1.1 [f82dc]
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 [34378] (via npm:1.1.0 [34378])
│
├─ @radix-ui/react-navigation-menu@npm:1.2.0
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 (via npm:1.1.0)
│
├─ @radix-ui/react-navigation-menu@npm:1.2.0 [a4987]
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 [34378] (via npm:1.1.0 [34378])
│
├─ @radix-ui/react-popover@npm:1.1.1
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 (via npm:1.1.0)
│
├─ @radix-ui/react-popover@npm:1.1.1 [a4987]
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 [34378] (via npm:1.1.0 [34378])
│
├─ @radix-ui/react-select@npm:2.1.1
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 (via npm:1.1.0)
│
├─ @radix-ui/react-select@npm:2.1.1 [a4987]
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 [34378] (via npm:1.1.0 [34378])
│
├─ @radix-ui/react-tooltip@npm:1.1.1
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 (via npm:1.1.0)
│
├─ @radix-ui/react-tooltip@npm:1.1.2
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 (via npm:1.1.0)
│
├─ @radix-ui/react-tooltip@npm:1.1.1 [0312d]
│  └─ @radix-ui/react-dismissable-layer@npm:1.1.0 [34378] (via npm:1.1.0 [34378])
│
└─ @radix-ui/react-tooltip@npm:1.1.2 [a4987]
   └─ @radix-ui/react-dismissable-layer@npm:1.1.0 [34378] (via npm:1.1.0 [34378])

Also tried adding to vite.config.mjs like explained here

resolve: {
      dedupe: ["@radix-ui/react-dismissable-layer"],
    },

And clearing vite cache, but no dice

@danieltroger
Copy link

danieltroger commented Aug 27, 2024 •
edited
Loading

Ok so what happened for us was that these conditions never were met

primitives/packages/react/dismissable-layer/src/DismissableLayer.tsx

Lines 124 to 129 in 660060a

if (
disableOutsidePointerEvents &&
context.layersWithOutsidePointerEventsDisabled.size === 1
) {
ownerDocument.body.style.pointerEvents = originalBodyPointerEvents;
}
so the pointer events aren't being unset even though they should.

The reason being that we nested two <DropdownMenuPrimitive.Root>s, both with modal set to true, and selecting an option in the topmost menu triggered some kind of race condition where the process of dismissing both modals changed the props sent to <DismissableLayer> to have disableOutsidePointerEvents set to false, see the logs in this screenshot
Screenshot 2024-08-27 at 16 51 48.

Wwhere setOpenDropDown and setIsHovered get set to false is where the state passed to open on both DropdownMenuPrimitive.Root changes to false.

This in turn made this code:
Screenshot 2024-08-27 at 16 36 38
set disableOutsidePointerEvents to false which then made this hook

primitives/packages/react/dismissable-layer/src/DismissableLayer.tsx

Lines 124 to 129 in 660060a

if (
disableOutsidePointerEvents &&
context.layersWithOutsidePointerEventsDisabled.size === 1
) {
ownerDocument.body.style.pointerEvents = originalBodyPointerEvents;
}
not actually remove the pointer-events: none.

I really need to work on other stuff so sorry for not having a repro, but maybe this helps more than not saying anything? Here's the yarn patch we applied to fix the issue for us (we use yarn berry as package manager)

@radix-ui-react-dismissable-layer-npm-1.1.0-6cdf2af8cf.patch

diff --git a/dist/index.mjs b/dist/index.mjs
index 55f6c96529982098820fa8072c5069591d3abf36..b6441d8e7091022b842f359e4fc4ff1655fb6e85 100644
--- a/dist/index.mjs
+++ b/dist/index.mjs
@@ -86,7 +86,11 @@ var DismissableLayer = React.forwardRef(
       return () => {
         if (!node) return;
         context.layers.delete(node);
-        context.layersWithOutsidePointerEventsDisabled.delete(node);
+        const deletedAPointerEventsLayer = context.layersWithOutsidePointerEventsDisabled.delete(node);
+        if(!context.layersWithOutsidePointerEventsDisabled.size && deletedAPointerEventsLayer) {
+          // Work around https://github.com/radix-ui/primitives/issues/2597#issuecomment-2312839713
+          ownerDocument.body.style.pointerEvents = originalBodyPointerEvents;
+        }
         dispatchUpdate();
       };
     }, [node, context]);

Then add to package.json for anyone else running into this:

 "resolutions": {
 "@radix-ui/react-dismissable-layer@npm:1.1.0": "patch:@radix-ui/react-dismissable-layer@npm%3A1.1.0#~/.yarn/patches/@radix-ui-react-dismissable-layer-npm-1.1.0-6cdf2af8cf.patch"
}

danieltroger added a commit to danieltroger/primitives that referenced this issue Aug 27, 2024
@danieltroger
Work around radix-ui#2597 (comment)
4a379ef
@danieltroger danieltroger mentioned this issue Aug 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Work around https://github.com/radix-ui/primitives/issues/2597#issuec… danieltroger/primitives
4 participants
@StephenHaney @danieltroger @ugokoli @robintown