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.

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

Shadcn popover not working in Modal Dialog #1511

Closed
coded58 opened this issue Sep 12, 2023 · 31 comments
Closed

Shadcn popover not working in Modal Dialog #1511

coded58 opened this issue Sep 12, 2023 · 31 comments

Comments

@coded58
Copy link

coded58 commented Sep 12, 2023

How can i use shadcn popover in modal correctly? i copied the date-picker component and tried using it in a modal-form but i closes without user interaction
https://github.com/shadcn-ui/ui/assets/99658156/8a08a6de-cca0-4e26-98df-973559637934

@coded58
Copy link
Author

coded58 commented Sep 12, 2023

similar issue on the below codesandbox but it was in bootstrap
https://codesandbox.io/s/react-boostrap-modal-overlay-bm2rdb?file=/src/MyOverlay.js

@Josevictor2
Copy link

I think it happened because the Dialog and Popover components are using the body element as a portal. try changing the portal element to the Dialog itself solves the issue.

@coded58
Copy link
Author

coded58 commented Sep 13, 2023

yeah thanks, this approach worked for me

@coded58 coded58 closed this as completed Sep 13, 2023
@romuloccomp
Copy link

I think it happened because the Dialog and Popover components are using the body element as a portal. try changing the portal element to the Dialog itself solves the issue.

Hi there, how can I do this?
Can you send an example, please?

@ap-1
Copy link

ap-1 commented Sep 27, 2023

@coded58 @Josevictor2 Would you mind sharing how this is done? The closest I've gotten is creating a ref on DialogContent and passing ref.current to PopoverPrimitive.Portal's container prop, but this breaks when the dialog is closed and reopened.

@benomzn
Copy link

benomzn commented Sep 28, 2023

I think it happened because the Dialog and Popover components are using the body element as a portal. try changing the portal element to the Dialog itself solves the issue.

Do you mean remove the portal component from Popover?

@romuloccomp
Copy link

I think it happened because the Dialog and Popover components are using the body element as a portal. try changing the portal element to the Dialog itself solves the issue.

Do you mean remove the portal component from Popover?

It's working for me:
Remove <PopoverPrimitive.Portal> on popover.

cc @ap-1 @benomzn

@AuroraLantean
Copy link

Go inside your Shadcn UI popover.tsx file, inside the PopoverContent, remove <PopoverPrimitive.Portal> and its closing tag. I saved this modifiiled popover.tsx into popoverDialog.tsx file so I can use this new PopoverContent in my Dialog Modals.

@morgan4080
Copy link

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

@Fanoflix
Copy link

Go inside your Shadcn UI popover.tsx file, inside the PopoverContent, remove <PopoverPrimitive.Portal> and its closing tag. I saved this modifiiled popover.tsx into popoverDialog.tsx file so I can use this new PopoverContent in my Dialog Modals.

That worked for me too. But is there any side effect that might occur because of it?

After removing the portal, I anticipated the popover to not overflow correctly (I thought it will start clipping), but its working fine.

@hummify
Copy link

hummify commented Nov 16, 2023

I've had the same concern when I found that removing <DropdownMenuPrimitive.Portal> from dropdown-menu.tsx solved a similar issue I was having with dropdown menu sub-content closing without user interaction. At first glance it seems as though removing the portal elements don't inhibit the components' ability to function, but I may be wrong.

@Winter-wub
Copy link

Winter-wub commented Nov 17, 2023

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

this solution is working for me

@hummify
Copy link

hummify commented Nov 17, 2023

Setting modal={false} will allow users to interact with elements outside of the dialog while it is open which I don't think is usually desirable

@baha-ott
Copy link

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

best solution, worked correctly as expected.

@bikmazer
Copy link

bikmazer commented Jan 4, 2024

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

Worked For Me! Good people, I'm glad you're here.

@mardausdennis
Copy link

Setting modal={false} will allow users to interact with elements outside of the dialog while it is open which I don't think is usually desirable

When using a modal Popover, closing the dialog retains focus on the Popover, blocking interaction with other elements. This happens when clicking outside to close.

How can I ensure the Popover fully closes and releases focus when the dialog closes, especially from clicking outside?

Looking for guidance on the best approach to resolve this issue and allow users to interact with the page freely again after closing the modal Popover.

@klincoder
Copy link

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

This worked for me with or without removing the PopoverPrimitive.Portal inside the popover file. I'm using Datepicker inside a modal/dialog. Hope it helps you.

@Ansub
Copy link

Ansub commented Jan 17, 2024

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

adding modal prop as true to Popover fixed the issue for me, thanks.

<Popover modal={true}>

@sfonua10
Copy link

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

this is the way

@jmccarthy86
Copy link

In my case the modal prop worked to enable a popover in a dialog, but caused inconsistencies with additional components within the popover.

The alternative solution was to use the Portal container prop, pass a ref of a valid HTMLElement to the portal. The result was the same behavior as the modal={true} as suggested in this thread, but the method here also fixed the issues in further nested components; in the popover; in a dialog.

Here's the core parts of the implementation.

//... Component Prep
const formRef = React.useRef<HTMLFormElement>(null);
//... Within my component
<form ref={formRef}>
    <PopoverContent className="w-[200px] p-0" asChild containerRef={formRef}>
    </PopoverContent>
</form>
//.. popover.tsx
const PopoverContent = React.forwardRef<
  React.ElementRef<typeof PopoverPrimitive.Content>,
  React.ComponentPropsWithoutRef<typeof PopoverPrimitive.Content> & {
    containerRef?: React.RefObject<HTMLElement>
  }
>(({ className, align = "center", sideOffset = 4, containerRef, ...props }, ref) => (
  <PopoverPrimitive.Portal container={containerRef?.current}>

@AumeshMisra
Copy link

In my case the modal prop worked to enable a popover in a dialog, but caused inconsistencies with additional components within the popover.

The alternative solution was to use the Portal container prop, pass a ref of a valid HTMLElement to the portal. The result was the same behavior as the modal={true} as suggested in this thread, but the method here also fixed the issues in further nested components; in the popover; in a dialog.

Here's the core parts of the implementation.

//... Component Prep
const formRef = React.useRef<HTMLFormElement>(null);
//... Within my component
<form ref={formRef}>
    <PopoverContent className="w-[200px] p-0" asChild containerRef={formRef}>
    </PopoverContent>
</form>
//.. popover.tsx
const PopoverContent = React.forwardRef<
  React.ElementRef<typeof PopoverPrimitive.Content>,
  React.ComponentPropsWithoutRef<typeof PopoverPrimitive.Content> & {
    containerRef?: React.RefObject<HTMLElement>
  }
>(({ className, align = "center", sideOffset = 4, containerRef, ...props }, ref) => (
  <PopoverPrimitive.Portal container={containerRef?.current}>

@jmccarthy86 I tried the approach with modal={true} and the above, but unfortunately still had issues.
More generally the structure I had was (with the changes above).

<Dialog>
// ... Dialog Trigger
<DialogContent>
<form ref={formRef}>
<Popover>
//.. Popover Trigger
<PopoverContent asChild containerRef={formRef}>
   <Command>
   </Command>
</PopoverContent>

</Popover>
</form>
</DialogContent>
</Dialog>

Unfortunately hovering over the commands do not work and I can only use arrow keys to select anything. Do you have any insight to what I can do here?

@jmccarthy86
Copy link

jmccarthy86 commented Jul 12, 2024

@AumeshMisra

Ah, sorry, I forgot to mention that I also made an adjustment to the className in the CommandItem component.

//command.tsx

const CommandItem = React.forwardRef<
  React.ElementRef<typeof CommandPrimitive.Item>,
  React.ComponentPropsWithoutRef<typeof CommandPrimitive.Item>
>(({ className, ...props }, ref) => (
  <CommandPrimitive.Item
    ref={ref}
    className={cn(
      "relative flex cursor-default select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none aria-selected:bg-accent aria-selected:text-accent-foreground data-[disabled='true']:pointer-events-none data-[disabled='true']:opacity-50",
      className
    )}
    {...props}
  />
))

See where data-[disabled='true'] in data-[disabled='true']:pointer-events-none data-[disabled='true']:opacity-50 , in the current version of shadcn/ui the class is set as data-[disabled]. So replace that and that should sort your issue

@aumeshm
Copy link

aumeshm commented Jul 12, 2024

In my case the modal prop worked to enable a popover in a dialog, but caused inconsistencies with additional components within the popover.
The alternative solution was to use the Portal container prop, pass a ref of a valid HTMLElement to the portal. The result was the same behavior as the modal={true} as suggested in this thread, but the method here also fixed the issues in further nested components; in the popover; in a dialog.
Here's the core parts of the implementation.

//... Component Prep
const formRef = React.useRef<HTMLFormElement>(null);
//... Within my component
<form ref={formRef}>
    <PopoverContent className="w-[200px] p-0" asChild containerRef={formRef}>
    </PopoverContent>
</form>
//.. popover.tsx
const PopoverContent = React.forwardRef<
  React.ElementRef<typeof PopoverPrimitive.Content>,
  React.ComponentPropsWithoutRef<typeof PopoverPrimitive.Content> & {
    containerRef?: React.RefObject<HTMLElement>
  }
>(({ className, align = "center", sideOffset = 4, containerRef, ...props }, ref) => (
  <PopoverPrimitive.Portal container={containerRef?.current}>

@jmccarthy86 I tried the approach with modal={true} and the above, but unfortunately still had issues. More generally the structure I had was (with the changes above).

<Dialog>
// ... Dialog Trigger
<DialogContent>
<form ref={formRef}>
<Popover>
//.. Popover Trigger
<PopoverContent asChild containerRef={formRef}>
   <Command>
   </Command>
</PopoverContent>

</Popover>
</form>
</DialogContent>
</Dialog>

Unfortunately hovering over the commands do not work and I can only use arrow keys to select anything. Do you have any insight to what I can do here?

Actually I changed "cmdk" package to 0.2.0, and hover works, but I still cannot scroll in command menu

@jmccarthy86
Copy link

Difficult to comment on that as the scenario I am working with did not change the cmdk package to 0.2.0. And, your issue could be due to an unrelated issue with your implementation.

I can say

  • I did not change the cmdk package.
  • I changed the class as mentioned in command.tsx
  • I updated popover.tsx to accept a ref passed to the portal primitive
  • I passed a ref of an applicable parent HTMLElement to the portal primitive to set the container to something other than body which is the default.

These had a me a working dialog component with a popover command combobox setup without any further adjustments. To note, I did not use the model=true in this implementation.

If you have a repo you can share I'm happy to take a look.

@Fanoflix
Copy link

Fanoflix commented Jul 12, 2024

Go inside your Shadcn UI popover.tsx file, inside the PopoverContent, remove <PopoverPrimitive.Portal> and its closing tag. I saved this modifiiled popover.tsx into popoverDialog.tsx file so I can use this new PopoverContent in my Dialog Modals.

Just a note - Instead of creating 2 files, one with the portal and one without, just keep one file with the portal removed. If you need to use portal, just go to the component where you are using the Popover and manually wrap the <PopoverContent> with the exported <PopoverPortal>.

This helps if you update Shadcn Popover later on, you'd only need to make a change in this one file (instead of maintaining two). The change will of course be removing the portal after the update.

@jmccarthy86
Copy link

If this is related to the method I highlighted. Whilst this is also a good option. The solution I shared is in essence the same it just added the option to change the portal the popover uses. There's no creation or two files or removing of any portals.

@Fanoflix
Copy link

My bad, I thought it spoke in essence of keeping the 2 versions separate. Nonetheless, the method I shared could be helpful to some people who have created 2 versions :)

@jmccarthy86
Copy link

@Fanoflix 100% sir, both valid methods

@AumeshMisra
Copy link

@jmccarthy86 thanks so much, the command bar changes were what did it for me!

@fiel777
Copy link

fiel777 commented Jul 16, 2024

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

This works thanks

@fysherman
Copy link

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

work like a charm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests