radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
15.87k stars 823 forks source link

[Popover] [Dialog] When nesting `Popover` in `Dialog`, clicks in the `Popover` are closing it #2121

Closed williamhzo closed 1 year ago

williamhzo commented 1 year ago

Bug report

Current Behavior

When nesting Popover in Dialog, clicks in the Popover are closing it (considered outside interaction) even with the appropriate z-indices.

Expected behavior

When opening the Popover from the Dialog, interaction within the Popover shouldn't trigger an outside click and therefore shouldn't close it.

Reproducible example

https://codesandbox.io/s/mystifying-banzai-v59705?file=/src/App.tsx

Suggested solution

Adding the modal prop to the Popover root seems to fix it, but then outside content remains blocked and un-interactive when dismissing the Dialog + Popover by an outside click.

Additional context

i assume it's a portal and z-index issue, but what i understood is that radix is supposed to natively handle such nesting?

i'm certainly missing something here, as this use-case seems pretty common, i tried searching in other issues but didn't find an answer to this.

thanks in advance for the support! you (the whole team + maintainers) are doing an amazing job 🙏

Your environment

Software Name(s) Version
Radix Package(s) react-popover, react-dialog resp. 1.0.5, 1.0.3
React n/a 18.2.0
Browser Arc 0.101.2
Assistive tech n/a n/a
Node n/a 16.17
yarn n/a 1.22.19
Operating System macOS 13.3.1
joaom00 commented 1 year ago

Hey @williamhzo,

It's an issue when components use different versions of the DismissableLayer component under the hood. https://github.com/radix-ui/primitives/issues/1088

In your example, updating the Dialog to the latest version fixes the issue.

williamhzo commented 1 year ago

hi @joaom00!

thanks for the explanation and support, that fixes the reproduction CodeSandbox mentioned in the issue description but it doesn't seem to fix in my actual project, even when just copy/pasting the content of the CodeSandbox (styles included) in an empty page.

i just checked the versions and both packages are at their latest:

and when running yarn info on both packages both have the @radix-ui/react-dismissable-layer to 1.0.3.

i tried pinning all my radix dependency versions to keep them in sync, but maybe another dependency needs to be updated?

i know it's a long shot since you can't reproduce on your end but do you know of any issues that might cause this? i tried re-installing the deps and upgrading to latest but wasn't successful.

thanks in advance!

williamhzo commented 1 year ago

for anyone having the issue, i think i've may've found the cause of the bug: another 3rd party lib is using an outdated version of @radix-ui/react-dismissable-layer under the hood, causing the version mismatch.

enforcing the correct version with yarn resolutions doesn't seem to work, apparently due to my yarn version being version 1.x.x (it seems that only v2 and v3 supports "/").

williamhzo commented 1 year ago

peer dependency was upgraded to latest version, closing the issue as it's now fixed.

thanks again @joaom00!

taoliujun commented 1 year ago

I can't solve it even I put resolutions in package.json.

"dependencies": {
  "@radix-ui/react-dialog": "1.0.5-rc.5",
  "@radix-ui/react-popover": "1.0.7-rc.5",
},
 "resolutions": {
        "@radix-ui/react-dismissable-layer": "1.0.5-rc.5"
    },
williamhzo commented 1 year ago

hey @taoliujun, you appear to be using the release candidate versions of @radix-ui/react-dialog and @radix-ui/react-popover that contained this bug, if you update to latest stable version of both packages the issue should be fixed, with no resolution needed

zain-adtracktiv commented 4 months ago

Hey, this is still happening for me with the latest version (1.1.1) of both the @radix-ui/react-popover and @radix-ui/react-dialog

roodiroot commented 4 months ago

Set the modal prop to true on the Popover root. https://www.radix-ui.com/primitives/docs/components/popover#api-reference

Hey, this is still happening for me with the latest version (1.1.1) of both the @radix-ui/react-popover and @radix-ui/react-dialog

ASgmbv commented 3 months ago

Try this

const [container, setContainer] = useState(null);

return (
      <Dialog>
        <DialogTrigger asChild>
          <Button>Logic Logic</Button>
        </DialogTrigger>
        <DialogContent>
         <div ref={setContainer}></div>

          <Popover>
            <PopoverTrigger asChild>
              <Button>
                Set condition
              </Button>
            </PopoverTrigger>
            <PopoverContent container={container}> // need to make it accept container prop
              <Input />
            </PopoverContent>
          </Popover>
        </DialogContent>
      </Dialog>
)
fgatti675 commented 3 months ago

Hi, facing the same issue. This seems not to be a valid prop?

<PopoverContent container={container}>

Thank you

fgatti675 commented 3 months ago

Btw, making the PopOver modal fixed the issue for me :) <PopoverPrimitive.Root modal={true}>

Ssalvador221 commented 2 months ago

Btw, making the PopOver modal fixed the issue for me :) <PopoverPrimitive.Root modal={true}>

This worked for me too! Thanks, @fgatti675.