fix: performance issue when there are many items caused by ItemContex…

…tMenu
upstash · Nov 18, 2024 · 46abc38 · 46abc38
1 parent 6bdfaf9
commit 46abc38
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 67 deletions.
diff --git a/src/components/databrowser/components/display/display-list.tsx b/src/components/databrowser/components/display/display-list.tsx
@@ -39,9 +39,11 @@ export const ListDisplay = ({ dataKey, type }: { dataKey: string; type: ListData
         <InfiniteScroll query={query}>
           <div className="pr-3">
             <table className="w-full">
-              <tbody>
-                <ListItems dataKey={dataKey} type={type} query={query} />
-              </tbody>
+              <ItemContextMenu dataKey={dataKey} type={type}>
+                <tbody>
+                  <ListItems dataKey={dataKey} type={type} query={query} />
+                </tbody>
+              </ItemContextMenu>
             </table>
           </div>
         </InfiniteScroll>
@@ -50,7 +52,7 @@ export const ListDisplay = ({ dataKey, type }: { dataKey: string; type: ListData
   )
 }
 
-type ItemData = {
+export type ItemData = {
   key: string
   value?: string
 }
@@ -75,63 +77,58 @@ export const ListItems = ({
   return (
     <>
       {keys.map(({ key, value }, i) => (
-        <ItemContextMenu
+        <tr
           key={`${dataKey}-${key}-${i}`}
-          dataKey={dataKey}
-          type={type}
-          itemKey={key}
-          itemValue={value}
+          data-item-key={key}
+          data-item-value={value}
+          onClick={() => {
+            setSelectedListItem({ key, value })
+          }}
+          className="h-10 border-b border-b-zinc-100 hover:bg-zinc-50"
         >
-          <tr
-            onClick={() => {
-              setSelectedListItem({ key, value })
-            }}
-            className="h-10 border-b border-b-zinc-100 hover:bg-zinc-50"
+          <td
+            className={cn(
+              "cursor-pointer truncate px-3",
+              type === "list" || type === "stream" ? "w-32 min-w-24" : "max-w-0"
+            )}
           >
+            {key}
+          </td>
+          {value !== undefined && (
             <td
-              className={cn(
-                "cursor-pointer truncate px-3",
-                type === "list" || type === "stream" ? "w-32 min-w-24" : "max-w-0"
-              )}
+              className={cn("cursor-pointer truncate px-3", type === "zset" ? "w-24" : "max-w-0")}
             >
-              {key}
+              {value}
             </td>
-            {value !== undefined && (
-              <td
-                className={cn("cursor-pointer truncate px-3", type === "zset" ? "w-24" : "max-w-0")}
-              >
-                {value}
-              </td>
-            )}
-            {type !== "stream" && (
-              <td
-                width={20}
-                className="px-3"
-                onClick={(e) => {
+          )}
+          {type !== "stream" && (
+            <td
+              width={20}
+              className="px-3"
+              onClick={(e) => {
+                e.stopPropagation()
+              }}
+            >
+              <DeleteAlertDialog
+                deletionType="item"
+                onDeleteConfirm={(e) => {
                   e.stopPropagation()
+                  editItem({
+                    type,
+                    dataKey,
+                    itemKey: key,
+                    // For deletion
+                    newKey: undefined,
+                  })
                 }}
               >
-                <DeleteAlertDialog
-                  deletionType="item"
-                  onDeleteConfirm={(e) => {
-                    e.stopPropagation()
-                    editItem({
-                      type,
-                      dataKey,
-                      itemKey: key,
-                      // For deletion
-                      newKey: undefined,
-                    })
-                  }}
-                >
-                  <Button size="icon-sm" variant="secondary" onClick={(e) => e.stopPropagation()}>
-                    <IconTrash className="size-4 text-zinc-500" />
-                  </Button>
-                </DeleteAlertDialog>
-              </td>
-            )}
-          </tr>
-        </ItemContextMenu>
+                <Button size="icon-sm" variant="secondary" onClick={(e) => e.stopPropagation()}>
+                  <IconTrash className="size-4 text-zinc-500" />
+                </Button>
+              </DeleteAlertDialog>
+            </td>
+          )}
+        </tr>
       ))}
     </>
   )

diff --git a/src/components/databrowser/components/item-context-menu.tsx b/src/components/databrowser/components/item-context-menu.tsx
@@ -12,21 +12,19 @@ import { toast } from "@/components/ui/use-toast"
 
 import { useEditListItem } from "../hooks"
 import { DeleteAlertDialog } from "./display/delete-alert-dialog"
+import type { ItemData } from "./display/display-list"
 
 export const ItemContextMenu = ({
   children,
   dataKey,
-  itemKey,
-  itemValue,
   type,
 }: PropsWithChildren<{
   dataKey: string
   type: ListDataType
-  itemKey: string
-  itemValue?: string
 }>) => {
   const { mutate: editItem } = useEditListItem()
   const [isAlertOpen, setAlertOpen] = useState(false)
+  const [data, setData] = useState<ItemData | undefined>()
 
   return (
     <>
@@ -36,33 +34,54 @@ export const ItemContextMenu = ({
         onOpenChange={setAlertOpen}
         onDeleteConfirm={(e) => {
           e.stopPropagation()
-          editItem({
-            type,
-            dataKey,
-            itemKey,
-            // For deletion
-            newKey: undefined,
-          })
+          if (data) {
+            editItem({
+              type,
+              dataKey,
+              itemKey: data?.key,
+              // For deletion
+              newKey: undefined,
+            })
+          }
           setAlertOpen(false)
         }}
       />
       <ContextMenu>
-        <ContextMenuTrigger asChild>{children}</ContextMenuTrigger>
+        <ContextMenuTrigger
+          asChild
+          // NOTE: We did not put the ContextMenu on every key because of performance reasons
+          onContextMenu={(e) => {
+            const el = e.target as HTMLElement
+            const item = el.closest("[data-item-key]")
+
+            if (item && item instanceof HTMLElement && item.dataset.itemKey !== undefined) {
+              setData({
+                key: item.dataset.itemKey,
+                value: item.dataset.itemValue,
+              })
+            } else {
+              throw new Error("Key not found")
+            }
+          }}
+        >
+          {children}
+        </ContextMenuTrigger>
         <ContextMenuContent>
           <ContextMenuItem
             onClick={() => {
-              navigator.clipboard.writeText(itemKey)
+              if (!data) return
+              navigator.clipboard.writeText(data?.key)
               toast({
                 description: "Key copied to clipboard",
               })
             }}
           >
             Copy key
           </ContextMenuItem>
-          {itemValue !== undefined && (
+          {data?.value && (
             <ContextMenuItem
               onClick={() => {
-                navigator.clipboard.writeText(itemValue)
+                navigator.clipboard.writeText(data?.value ?? "")
                 toast({
                   description: "Value copied to clipboard",
                 })