Fix file picker UX issues

- Change from modal dialog to inline popover (no focus stealing) - Keep textarea focused while file picker is open - Fix loading flickering by caching git files - Debounce file search to prevent rapid refetching - Escape closes picker without removing @ text - Enter selects file from picker - Arrow keys navigate picker when open, history when closed - Position picker above textarea using absolute positioning - Mouse hover updates selection index - Remove blur/focus from picker input
2025-10-23 22:58:09 +01:00
parent 6e4fa9479e
commit 9a47cfd8d9
2 changed files with 178 additions and 161 deletions
--- a/src/components/file-picker.tsx
+++ b/src/components/file-picker.tsx
@@ -1,5 +1,4 @@
-import { Component, createSignal, createEffect, For, Show, onMount } from "solid-js"
-import { Dialog } from "@kobalte/core/dialog"
+import { Component, createSignal, createEffect, For, Show, onCleanup } from "solid-js"

 interface FileItem {
  path: string
@@ -10,39 +9,50 @@ interface FileItem {

 interface FilePickerProps {
  open: boolean
-  onClose: () => void
  onSelect: (path: string) => void
-  instanceId: string
+  onNavigate: (direction: "up" | "down") => void
+  onClose: () => void
  instanceClient: any
-  searchQuery?: string
+  searchQuery: string
+  textareaRef?: HTMLTextAreaElement
 }

 const FilePicker: Component<FilePickerProps> = (props) => {
  const [files, setFiles] = createSignal<FileItem[]>([])
  const [selectedIndex, setSelectedIndex] = createSignal(0)
-  const [query, setQuery] = createSignal(props.searchQuery || "")
  const [loading, setLoading] = createSignal(false)
+  const [cachedGitFiles, setCachedGitFiles] = createSignal<FileItem[]>([])

-  let inputRef: HTMLInputElement | undefined
+  let containerRef: HTMLDivElement | undefined

-  async function fetchFiles(searchQuery: string) {
-    if (!props.instanceClient) return
+  async function fetchGitFiles() {
+    if (!props.instanceClient || cachedGitFiles().length > 0) return

-    setLoading(true)
    try {
-      const gitFilesPromise = props.instanceClient.file.status()
-      const searchFilesPromise = searchQuery
-        ? props.instanceClient.find.files({ query: { query: searchQuery } })
-        : Promise.resolve({ data: [] })
-
-      const [gitResponse, searchResponse] = await Promise.all([gitFilesPromise, searchFilesPromise])
-
+      const gitResponse = await props.instanceClient.file.status()
      const gitFiles: FileItem[] = (gitResponse.data || []).map((file: any) => ({
        path: file.path,
        added: file.added,
        removed: file.removed,
        isGitFile: true,
      }))
+      setCachedGitFiles(gitFiles)
+    } catch (error) {
+      console.error("Failed to fetch git files:", error)
+    }
+  }
+
+  async function fetchFiles(searchQuery: string) {
+    if (!props.instanceClient) return
+
+    setLoading(true)
+    try {
+      const searchFilesPromise = searchQuery
+        ? props.instanceClient.find.files({ query: { query: searchQuery } })
+        : Promise.resolve({ data: [] })
+
+      const searchResponse = await searchFilesPromise
+      const gitFiles = cachedGitFiles()

      const searchFiles: FileItem[] = (searchResponse.data || [])
        .filter((path: string) => !gitFiles.some((gf) => gf.path === path))
@@ -52,7 +62,7 @@ const FilePicker: Component<FilePickerProps> = (props) => {
        }))

      const allFiles = searchQuery
-        ? [...gitFiles.filter((f) => f.path.includes(searchQuery)), ...searchFiles]
+        ? [...gitFiles.filter((f) => f.path.toLowerCase().includes(searchQuery.toLowerCase())), ...searchFiles]
        : gitFiles

      setFiles(allFiles)
@@ -67,53 +77,24 @@ const FilePicker: Component<FilePickerProps> = (props) => {

  createEffect(() => {
    if (props.open) {
-      fetchFiles(query())
+      fetchGitFiles()
+      fetchFiles(props.searchQuery)
    }
  })

  createEffect(() => {
-    if (props.searchQuery !== undefined) {
-      setQuery(props.searchQuery)
+    if (props.open) {
+      fetchFiles(props.searchQuery)
    }
  })

-  onMount(() => {
-    if (props.open && inputRef) {
-      setTimeout(() => inputRef?.focus(), 50)
-    }
+  createEffect(() => {
+    setSelectedIndex(0)
  })

-  function handleKeyDown(e: KeyboardEvent) {
-    const fileList = files()
-    if (fileList.length === 0) return
-
-    switch (e.key) {
-      case "ArrowDown":
-        e.preventDefault()
-        setSelectedIndex((prev) => Math.min(prev + 1, fileList.length - 1))
-        scrollToSelected()
-        break
-      case "ArrowUp":
-        e.preventDefault()
-        setSelectedIndex((prev) => Math.max(prev - 1, 0))
-        scrollToSelected()
-        break
-      case "Enter":
-        e.preventDefault()
-        if (fileList[selectedIndex()]) {
-          handleSelect(fileList[selectedIndex()].path)
-        }
-        break
-      case "Escape":
-        e.preventDefault()
-        props.onClose()
-        break
-    }
-  }
-
  function scrollToSelected() {
    setTimeout(() => {
-      const selectedElement = document.querySelector('[data-file-selected="true"]')
+      const selectedElement = containerRef?.querySelector('[data-file-selected="true"]')
      if (selectedElement) {
        selectedElement.scrollIntoView({ block: "nearest", behavior: "smooth" })
      }
@@ -122,86 +103,112 @@ const FilePicker: Component<FilePickerProps> = (props) => {

  function handleSelect(path: string) {
    props.onSelect(path)
-    props.onClose()
  }

-  function handleQueryChange(value: string) {
-    setQuery(value)
-    fetchFiles(value)
+  function handleNavigateUp() {
+    setSelectedIndex((prev) => {
+      const next = Math.max(prev - 1, 0)
+      scrollToSelected()
+      return next
+    })
  }

+  function handleNavigateDown() {
+    setSelectedIndex((prev) => {
+      const next = Math.min(prev + 1, files().length - 1)
+      scrollToSelected()
+      return next
+    })
+  }
+
+  createEffect(() => {
+    if (!props.open) return
+    const listener = (e: KeyboardEvent) => {
+      if (!props.open) return
+      const fileList = files()
+      if (fileList.length === 0) return
+
+      if (e.key === "ArrowDown") {
+        e.preventDefault()
+        handleNavigateDown()
+        props.onNavigate("down")
+      } else if (e.key === "ArrowUp") {
+        e.preventDefault()
+        handleNavigateUp()
+        props.onNavigate("up")
+      } else if (e.key === "Enter") {
+        e.preventDefault()
+        if (fileList[selectedIndex()]) {
+          handleSelect(fileList[selectedIndex()].path)
+        }
+      } else if (e.key === "Escape") {
+        e.preventDefault()
+        props.onClose()
+      }
+    }
+
+    document.addEventListener("keydown", listener)
+    onCleanup(() => document.removeEventListener("keydown", listener))
+  })
+
  return (
-    <Dialog open={props.open} onOpenChange={(open) => !open && props.onClose()}>
-      <Dialog.Portal>
-        <Dialog.Overlay class="fixed inset-0 z-50 bg-black/50" />
-        <div class="fixed inset-0 z-50 flex items-start justify-center pt-[20vh]">
-          <Dialog.Content
-            class="w-full max-w-2xl rounded-lg border border-gray-300 bg-white shadow-lg dark:border-gray-700 dark:bg-gray-900"
-            onKeyDown={handleKeyDown}
+    <Show when={props.open}>
+      <div
+        ref={containerRef}
+        class="absolute bottom-full left-0 mb-2 w-full max-w-2xl rounded-lg border border-gray-300 bg-white shadow-lg dark:border-gray-700 dark:bg-gray-900"
+        style={{ "z-index": 100 }}
+      >
+        <div class="max-h-96 overflow-y-auto">
+          <Show
+            when={!loading()}
+            fallback={
+              <div class="p-4 text-center text-sm text-gray-500">
+                <div class="inline-block h-4 w-4 animate-spin rounded-full border-2 border-gray-300 border-t-blue-600"></div>
+                <span class="ml-2">Loading files...</span>
+              </div>
+            }
          >
-            <div class="border-b border-gray-200 p-4 dark:border-gray-700">
-              <input
-                ref={inputRef}
-                type="text"
-                placeholder="Search files..."
-                value={query()}
-                onInput={(e) => handleQueryChange(e.currentTarget.value)}
-                class="w-full border-0 bg-transparent text-base outline-none placeholder:text-gray-400 dark:placeholder:text-gray-500"
-              />
-            </div>
-
-            <div class="max-h-96 overflow-y-auto">
-              <Show
-                when={!loading()}
-                fallback={
-                  <div class="p-8 text-center text-sm text-gray-500">
-                    <div class="inline-block h-4 w-4 animate-spin rounded-full border-2 border-gray-300 border-t-blue-600"></div>
-                    <span class="ml-2">Loading files...</span>
-                  </div>
-                }
-              >
-                <Show
-                  when={files().length > 0}
-                  fallback={<div class="p-8 text-center text-sm text-gray-500">No matching files</div>}
-                >
-                  <For each={files()}>
-                    {(file, index) => (
-                      <div
-                        data-file-selected={index() === selectedIndex()}
-                        class={`cursor-pointer border-b border-gray-100 px-4 py-2 hover:bg-gray-50 dark:border-gray-800 dark:hover:bg-gray-800 ${
-                          index() === selectedIndex() ? "bg-blue-50 dark:bg-blue-900/20" : ""
-                        }`}
-                        onClick={() => handleSelect(file.path)}
-                      >
-                        <div class="flex items-center justify-between">
-                          <span class="font-mono text-sm text-gray-900 dark:text-gray-100">{file.path}</span>
-                          <Show when={file.isGitFile && (file.added || file.removed)}>
-                            <div class="flex gap-2 text-xs">
-                              <Show when={file.added}>
-                                <span class="text-green-600 dark:text-green-400">+{file.added}</span>
-                              </Show>
-                              <Show when={file.removed}>
-                                <span class="text-red-600 dark:text-red-400">-{file.removed}</span>
-                              </Show>
-                            </div>
+            <Show
+              when={files().length > 0}
+              fallback={<div class="p-4 text-center text-sm text-gray-500">No matching files</div>}
+            >
+              <For each={files()}>
+                {(file, index) => (
+                  <div
+                    data-file-selected={index() === selectedIndex()}
+                    class={`cursor-pointer border-b border-gray-100 px-4 py-2 hover:bg-gray-50 dark:border-gray-800 dark:hover:bg-gray-800 ${
+                      index() === selectedIndex() ? "bg-blue-50 dark:bg-blue-900/20" : ""
+                    }`}
+                    onClick={() => handleSelect(file.path)}
+                    onMouseEnter={() => setSelectedIndex(index())}
+                  >
+                    <div class="flex items-center justify-between">
+                      <span class="font-mono text-sm text-gray-900 dark:text-gray-100">{file.path}</span>
+                      <Show when={file.isGitFile && (file.added || file.removed)}>
+                        <div class="flex gap-2 text-xs">
+                          <Show when={file.added}>
+                            <span class="text-green-600 dark:text-green-400">+{file.added}</span>
+                          </Show>
+                          <Show when={file.removed}>
+                            <span class="text-red-600 dark:text-red-400">-{file.removed}</span>
                          </Show>
                        </div>
-                      </div>
-                    )}
-                  </For>
-                </Show>
-              </Show>
-            </div>
-
-            <div class="border-t border-gray-200 p-2 text-xs text-gray-500 dark:border-gray-700">
-              <div class="flex items-center justify-between px-2">
-                <span>↑↓ Navigate • Enter Select • Esc Close</span>
-              </div>
-            </div>
-          </Dialog.Content>
+                      </Show>
+                    </div>
+                  </div>
+                )}
+              </For>
+            </Show>
+          </Show>
        </div>
-      </Dialog.Portal>
-    </Dialog>
+
+        <div class="border-t border-gray-200 p-2 text-xs text-gray-500 dark:border-gray-700">
+          <div class="flex items-center justify-between px-2">
+            <span>↑↓ Navigate • Enter Select • Esc Close</span>
+          </div>
+        </div>
+      </div>
+    </Show>
  )
 }

--- a/src/components/prompt-input.tsx
+++ b/src/components/prompt-input.tsx
@@ -45,11 +45,7 @@ export default function PromptInput(props: PromptInputProps) {
  })

  function handleKeyDown(e: KeyboardEvent) {
-    if (showFilePicker()) {
-      return
-    }
-
-    if (e.key === "Enter" && !e.shiftKey) {
+    if (e.key === "Enter" && !e.shiftKey && !showFilePicker()) {
      e.preventDefault()
      handleSend()
      return
@@ -61,7 +57,7 @@ export default function PromptInput(props: PromptInputProps) {
    const atStart = textarea.selectionStart === 0 && textarea.selectionEnd === 0
    const currentHistory = history()

-    if (e.key === "ArrowUp" && atStart && currentHistory.length > 0) {
+    if (e.key === "ArrowUp" && !showFilePicker() && atStart && currentHistory.length > 0) {
      e.preventDefault()
      const newIndex = historyIndex() === -1 ? 0 : Math.min(historyIndex() + 1, currentHistory.length - 1)
      setHistoryIndex(newIndex)
@@ -73,7 +69,7 @@ export default function PromptInput(props: PromptInputProps) {
      return
    }

-    if (e.key === "ArrowDown" && historyIndex() >= 0) {
+    if (e.key === "ArrowDown" && !showFilePicker() && historyIndex() >= 0) {
      e.preventDefault()
      const newIndex = historyIndex() - 1
      if (newIndex >= 0) {
@@ -130,47 +126,64 @@ export default function PromptInput(props: PromptInputProps) {
    target.style.height = Math.min(target.scrollHeight, 200) + "px"

    const cursorPos = target.selectionStart
-    const lastAtIndex = value.lastIndexOf("@", cursorPos)
+    const textBeforeCursor = value.substring(0, cursorPos)
+    const lastAtIndex = textBeforeCursor.lastIndexOf("@")

-    if (lastAtIndex !== -1 && lastAtIndex < cursorPos) {
+    if (lastAtIndex !== -1) {
      const textAfterAt = value.substring(lastAtIndex + 1, cursorPos)
      const hasSpace = textAfterAt.includes(" ") || textAfterAt.includes("\n")

-      if (!hasSpace) {
+      if (!hasSpace && cursorPos === lastAtIndex + textAfterAt.length + 1) {
        setAtPosition(lastAtIndex)
        setFileSearchQuery(textAfterAt)
        setShowFilePicker(true)
-      } else {
-        setShowFilePicker(false)
+        return
      }
-    } else {
-      setShowFilePicker(false)
    }
+
+    setShowFilePicker(false)
+    setAtPosition(null)
  }

  function handleFileSelect(path: string) {
-    const instance = getActiveInstance()
-    if (!instance) return
-
    const filename = path.split("/").pop() || path
    const attachment = createFileAttachment(path, filename)
    addAttachment(props.instanceId, props.sessionId, attachment)

    const currentPrompt = prompt()
    const pos = atPosition()
+    const cursorPos = textareaRef?.selectionStart || 0
+
    if (pos !== null) {
      const before = currentPrompt.substring(0, pos)
-      const after = currentPrompt.substring(textareaRef?.selectionStart || pos)
-      setPrompt(before + after)
+      const after = currentPrompt.substring(cursorPos)
+      const newPrompt = before + " " + after
+      setPrompt(newPrompt)
+
+      setTimeout(() => {
+        if (textareaRef) {
+          const newCursorPos = pos + 1
+          textareaRef.setSelectionRange(newCursorPos, newCursorPos)
+        }
+      }, 0)
    }

    setShowFilePicker(false)
    setAtPosition(null)
    setFileSearchQuery("")

-    setTimeout(() => textareaRef?.focus(), 50)
+    textareaRef?.focus()
  }

+  function handleFilePickerClose() {
+    setShowFilePicker(false)
+    setAtPosition(null)
+    setFileSearchQuery("")
+    textareaRef?.focus()
+  }
+
+  function handleFilePickerNavigate(_direction: "up" | "down") {}
+
  function handleRemoveAttachment(attachmentId: string) {
    removeAttachment(props.instanceId, props.sessionId, attachmentId)
  }
@@ -223,11 +236,23 @@ export default function PromptInput(props: PromptInputProps) {
      </Show>
      <div
        ref={containerRef}
-        class={`prompt-input-wrapper ${isDragging() ? "border-2 border-blue-500 bg-blue-50 dark:bg-blue-900/10" : ""}`}
+        class={`prompt-input-wrapper relative ${isDragging() ? "border-2 border-blue-500 bg-blue-50 dark:bg-blue-900/10" : ""}`}
        onDragOver={handleDragOver}
        onDragLeave={handleDragLeave}
        onDrop={handleDrop}
      >
+        <Show when={showFilePicker() && instance()}>
+          <FilePicker
+            open={showFilePicker()}
+            onClose={handleFilePickerClose}
+            onSelect={handleFileSelect}
+            onNavigate={handleFilePickerNavigate}
+            instanceClient={instance()!.client}
+            searchQuery={fileSearchQuery()}
+            textareaRef={textareaRef}
+          />
+        </Show>
+
        <textarea
          ref={textareaRef}
          class="prompt-input"
@@ -266,21 +291,6 @@ export default function PromptInput(props: PromptInputProps) {
          />
        </div>
      </div>
-
-      <Show when={showFilePicker() && instance()}>
-        <FilePicker
-          open={showFilePicker()}
-          onClose={() => {
-            setShowFilePicker(false)
-            setAtPosition(null)
-            setFileSearchQuery("")
-          }}
-          onSelect={handleFileSelect}
-          instanceId={props.instanceId}
-          instanceClient={instance()!.client}
-          searchQuery={fileSearchQuery()}
-        />
-      </Show>
    </div>
  )
 }