shadcn-ui / ui

Beautifully designed components that you can copy and paste into your apps. Accessible. Customizable. Open Source.
https://ui.shadcn.com
MIT License
63.9k stars 3.61k forks source link

[Bug] ComboBox breaks inside Dialogue #235

Closed seenyat closed 16 hours ago

seenyat commented 1 year ago

When using ComboBox inside Dialog component with the modal: true (default value), clicking on element inside PopoverTrigger causes the whole dialog to rerender, closing PopoverContent in the process. With modal:false it works as expected. Because Popover renders outside the dialog tree, it's considered an outside interaction.

I wonder if this is the expected behaviour. It would be nice to make ComboBox work inside modal:true Dialog as well.

Example code:

    <Dialog>
      <DialogTrigger>
        <div>test</div>
      </DialogTrigger>
      <DialogContent className="">
        <ComboBox placeholder="" onChange={() => null} />
      </DialogContent>
    </Dialog>
  );
vimtor commented 1 year ago

@seenyat have you found any solution?

seenyat commented 1 year ago

@vimtor I fixed it by wrapping my PopoverContent (which is being used by ComboBox) in its own FocusScope and DismissableLayer (suggested in this issue https://github.com/radix-ui/primitives/issues/922#issuecomment-948366132). That fixed my problem with unexpected rerender.


 const PopoverContent = React.forwardRef<
  React.ElementRef<typeof PopoverPrimitive.Content>,
  React.ComponentPropsWithoutRef<typeof PopoverPrimitive.Content>
>(({ className, align = "center", sideOffset = 4, ...props }, ref) => (
  <PopoverPrimitive.Portal>
    <FocusScope asChild loop trapped>
      <DismissableLayer>
        <PopoverPrimitive.Content
          ref={ref}
          align={align}
          sideOffset={sideOffset}
          className={cn(
            "...",
            className
          )}
          {...props}
        />
      </DismissableLayer>
    </FocusScope>
  </PopoverPrimitive.Portal>
));
hafijAsadMVN commented 10 months ago

@vimtor I fixed it by wrapping my PopoverContent (which is being used by ComboBox) in its own FocusScope and DismissableLayer (suggested in this issue radix-ui/primitives#922 (comment)). That fixed my problem with unexpected rerender.

const PopoverContent = React.forwardRef<
 React.ElementRef<typeof PopoverPrimitive.Content>,
 React.ComponentPropsWithoutRef<typeof PopoverPrimitive.Content>
>(({ className, align = "center", sideOffset = 4, ...props }, ref) => (
 <PopoverPrimitive.Portal>
   <FocusScope asChild loop trapped>
     <DismissableLayer>
       <PopoverPrimitive.Content
         ref={ref}
         align={align}
         sideOffset={sideOffset}
         className={cn(
           "...",
           className
         )}
         {...props}
       />
     </DismissableLayer>
   </FocusScope>
 </PopoverPrimitive.Portal>
));

I've tried this but unfortunately not working! Do you have any other suggestions for me to resolve this issue? Thank you

vimtor commented 10 months ago

Hi @hafijAsadMVN, to be honest, I don't know what did the trick for me, as I tried different things I saw in various GitHub issues. Let me copy the dialog.tsx components here to see if it works for you.

"use client";

import * as React from "react";
import * as DialogPrimitive from "@radix-ui/react-dialog";
import { X } from "lucide-react";

import { cn } from "@/lib/utils";

const Dialog = ({ open, ...props }: DialogPrimitive.DialogProps) => {
  return <DialogPrimitive.Root open={open} {...props} />;
};
Dialog.displayName = DialogPrimitive.Dialog.displayName;

const DialogTrigger = DialogPrimitive.Trigger;

const DialogPortal = ({ className, children, ...props }: DialogPrimitive.DialogPortalProps) => (
  <DialogPrimitive.Portal className={cn(className)} {...props}>
    <div className="fixed inset-0 z-50 flex items-start justify-center sm:items-center">{children}</div>
  </DialogPrimitive.Portal>
);
DialogPortal.displayName = DialogPrimitive.Portal.displayName;

const DialogOverlay = React.forwardRef<
  React.ElementRef<typeof DialogPrimitive.Overlay>,
  React.ComponentPropsWithoutRef<typeof DialogPrimitive.Overlay>
>(({ className, ...props }, ref) => (
  <DialogPrimitive.Overlay
    ref={ref}
    className={cn(
      "fixed inset-0 z-50 overflow-y-scroll max-h-screen scrollbar-hide sm:py-8 grid place-items-center bg-black/20 backdrop-blur-sm transition-all duration-100 data-[state=closed]:animate-out data-[state=closed]:fade-out data-[state=open]:fade-in",
      className
    )}
    {...props}
  />
));
DialogOverlay.displayName = DialogPrimitive.Overlay.displayName;

const DialogContent = React.forwardRef<
  React.ElementRef<typeof DialogPrimitive.Content>,
  React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content>
>(({ className, children, ...props }, ref) => (
  <DialogPortal>
    <DialogOverlay>
      <DialogPrimitive.Content
        ref={ref}
        onPointerDownOutside={(event) => event.preventDefault()}
        onInteractOutside={(event) => event.preventDefault()}
        className={cn(
          "relative z-50 gap-4 rounded-b-lg border bg-background p-6 shadow-lg animate-in data-[state=open]:fade-in-90 data-[state=open]:slide-in-from-bottom-10 sm:max-w-lg sm:rounded-lg sm:zoom-in-90 data-[state=open]:sm:slide-in-from-bottom-0",
          className
        )}
        {...props}
      >
        {children}
        <DialogPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground">
          <X className="h-5 w-5" />
          <span className="sr-only">Close</span>
        </DialogPrimitive.Close>
      </DialogPrimitive.Content>
    </DialogOverlay>
  </DialogPortal>
));
DialogContent.displayName = DialogPrimitive.Content.displayName;

const DialogHeader = ({ className, ...props }: React.HTMLAttributes<HTMLDivElement>) => (
  <div className={cn("flex flex-col space-y-1 text-left", className)} {...props} />
);
DialogHeader.displayName = "DialogHeader";

const DialogFooter = ({ className, ...props }: React.HTMLAttributes<HTMLDivElement>) => (
  <div className={cn("flex flex-col-reverse sm:flex-row sm:justify-end sm:space-x-2", className)} {...props} />
);
DialogFooter.displayName = "DialogFooter";

const DialogTitle = React.forwardRef<
  React.ElementRef<typeof DialogPrimitive.Title>,
  React.ComponentPropsWithoutRef<typeof DialogPrimitive.Title>
>(({ className, ...props }, ref) => (
  <DialogPrimitive.Title
    ref={ref}
    className={cn("text-lg font-semibold leading-none tracking-tight", className)}
    {...props}
  />
));
DialogTitle.displayName = DialogPrimitive.Title.displayName;

const DialogDescription = React.forwardRef<
  React.ElementRef<typeof DialogPrimitive.Description>,
  React.ComponentPropsWithoutRef<typeof DialogPrimitive.Description>
>(({ className, ...props }, ref) => (
  <DialogPrimitive.Description ref={ref} className={cn("text-sm text-muted-foreground", className)} {...props} />
));
DialogDescription.displayName = DialogPrimitive.Description.displayName;

export { Dialog, DialogTrigger, DialogContent, DialogHeader, DialogFooter, DialogTitle, DialogDescription };
ManuDoni commented 8 months ago

@vimtor I fixed it by wrapping my PopoverContent (which is being used by ComboBox) in its own FocusScope and DismissableLayer (suggested in this issue radix-ui/primitives#922 (comment)). That fixed my problem with unexpected rerender.

const PopoverContent = React.forwardRef<
 React.ElementRef<typeof PopoverPrimitive.Content>,
 React.ComponentPropsWithoutRef<typeof PopoverPrimitive.Content>
>(({ className, align = "center", sideOffset = 4, ...props }, ref) => (
 <PopoverPrimitive.Portal>
   <FocusScope asChild loop trapped>
     <DismissableLayer>
       <PopoverPrimitive.Content
         ref={ref}
         align={align}
         sideOffset={sideOffset}
         className={cn(
           "...",
           className
         )}
         {...props}
       />
     </DismissableLayer>
   </FocusScope>
 </PopoverPrimitive.Portal>
));

I've tried this but unfortunately not working! Do you have any other suggestions for me to resolve this issue? Thank you

I'm facing the same issue, but that solution didn't work neither for me. Did you solve it? For me, the issue is this: I have a shadcn <Select > component inside a dialogue but the hover on the items doesn't trigger the focus because it is trapped (the click works)

Update: found a solution here: https://github.com/shadcn-ui/ui/issues/1647#issuecomment-1768391351

shadcn commented 16 hours ago

This issue has been automatically closed because it received no activity for a while. If you think it was closed by accident, please leave a comment. Thank you.