ui icon indicating copy to clipboard operation
ui copied to clipboard

shadcn-ui Dialog component triggered twice with ⌘ + k

Open udohjeremiah opened this issue 1 year ago • 4 comments

I am currently using shadcn-ui as my ui-kit with Next.js. What I am trying to achieve is to open a search dialog by using the keyboard shortcut:⌘ + k. However, when I do this, the dialog is triggered twice. But then when I enter the keyboard shortcut: ⌘ + k it closes. Also when I open the dialog by clicking on it, I can't close it by using the keyboard shortcut ⌘ + k. Please, how do I fix it? My code is shown below:

"use client";

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

import { useState, useEffect } from "react";

import { Button } from "@/components/ui/button";
import {
  Dialog,
  DialogContent,
  DialogDescription,
  DialogFooter,
  DialogHeader,
  DialogTitle,
  DialogTrigger,
} from "@/components/ui/dialog";
import { Input } from "@/components/ui/input";
import { Label } from "@/components/ui/label";
import { MagnifyingGlassIcon } from "@radix-ui/react-icons";

export default function Search() {
  const [openSearchDialog, setSearchDialog] = useState(false);

  useEffect(() => {
    const handleKeyDown = (event) => {
      if ((event.metaKey || event.ctrlKey) && event.key === "k") {
        event.preventDefault();
        setSearchDialog((open) => !open);
      }
    };

    window.addEventListener("keydown", handleKeyDown);

    return () => {
      window.removeEventListener("keydown", handleKeyDown);
    };
  }, []);

  return (
    <Dialog open={openSearchDialog} onOpenChange={setSearchDialog}>
      <DialogTrigger asChild>
        <div>
          <Button variant="outline" size="icon" className="lg:hidden">
            <MagnifyingGlassIcon />
          </Button>
          <Button
            className={cn(
              "hidden w-64 items-center gap-2 border bg-primary-foreground text-sm text-muted-foreground",
              "hover:border-card hover:bg-primary-foreground",
              "lg:flex",
              "xl:w-96",
            )}
          >
            <MagnifyingGlassIcon />
            <span>Search website...</span>
            <kbd className="pointer-events-none ml-auto flex h-5 flex-none select-none items-center gap-1 rounded border bg-muted px-1.5 font-mono text-[10px] font-semibold opacity-100">
              <span className="text-xs">⌘</span>K
            </kbd>
          </Button>
        </div>
      </DialogTrigger>
      <DialogContent className="sm:max-w-[425px]">
        <DialogHeader>
          <DialogTitle>Edit profile</DialogTitle>
          <DialogDescription>
            Make changes to your profile here. Click save when you&apos;re done.
          </DialogDescription>
        </DialogHeader>
        <div className="grid gap-4 py-4"></div>
        <DialogFooter>
          <Button type="submit">Save changes</Button>
        </DialogFooter>
      </DialogContent>
    </Dialog>
  );
}

udohjeremiah avatar Jan 17 '24 18:01 udohjeremiah

Hi @udohjeremiah,

There's no problem in the code you've provided. The problem is in your project and how you render the <Search /> component in the parent <Header /> component:

<header className="sticky top-0 z-50 border-b backdrop-blur-sm">
  {/* Mobile nav */}
  <div
    className={cn(
      "flex items-center justify-between gap-4 px-4 py-3",
      "sm:px-8",
      "lg:hidden",
    )}
  >
    <Link aria-label="Home" href="/">
      <Avatar>
        <AvatarImage src="https://source.unsplash.com/random" />
        <AvatarFallback>UJ</AvatarFallback>
      </Avatar>
    </Link>
    <div className="flex gap-2">
      <Search />
      <Navigation />
    </div>
  </div>

  {/* Desktop nav */}
  <div
    className={cn(
      "hidden items-center justify-between gap-4 overflow-y-auto bg-muted px-4",
      "lg:flex",
    )}
  >
    <Profile />
    <Search />
    <RandomQuotes />
  </div>
</header>

The <Search /> component is rendered twice but one of them is hidden by using hidden TailwindCSS class. While it is invisible for the user it is still in the document so you get 2 dialog windows opened at the same time.

iTsygancov avatar Jan 24 '24 12:01 iTsygancov

Wow that's surprising. So in this case, how do I solve it?

udohjeremiah avatar Jan 26 '24 14:01 udohjeremiah

Instead of hiding content with CSS you can use the use-media-query hook.

Here's an implementation of this hook in shadcn-ui library:

import * as React from "react"

export function useMediaQuery(query: string) {
  const [value, setValue] = React.useState(false)

  React.useEffect(() => {
    function onChange(event: MediaQueryListEvent) {
      setValue(event.matches)
    }

    const result = matchMedia(query)
    result.addEventListener("change", onChange)
    setValue(result.matches)

    return () => result.removeEventListener("change", onChange)
  }, [query])

  return value
}

And here's an example of usage:

const isDesktop = useMediaQuery("(min-width: 768px)")

So now in the return() you can use conditional rendering:

<header className="sticky top-0 z-50 border-b backdrop-blur-sm">
  {isDesktop ? (
    <div
      className="flex items-center justify-between gap-4 overflow-y-auto bg-muted px-4"
    >
      <Profile />
      <Search />
      <RandomQuotes />
    </div>
  ) : (
    <div
      className="flex items-center justify-between gap-4 px-4 py-3 sm:px-8"
    >
      <Link aria-label="Home" href="/">
        <Avatar>
          <AvatarImage src="https://source.unsplash.com/random" />
          <AvatarFallback>UJ</AvatarFallback>
        </Avatar>
      </Link>
      <div className="flex gap-2">
        <Search />
        <Navigation />
      </div>
    </div>
  )}
</header>

iTsygancov avatar Jan 26 '24 14:01 iTsygancov

Wow, that's great. Thanks very much @iTsygancov , I really appreciate.

udohjeremiah avatar Jan 27 '24 09:01 udohjeremiah