From cd8866ecb0f904ddc8d29cfcdf94ee401668c120 Mon Sep 17 00:00:00 2001 From: LukeParkerDev <10430890+Hona@users.noreply.github.com> Date: Sun, 7 Jun 2026 17:57:27 +1000 Subject: [PATCH] fix(desktop): harden picker navigation --- .../components/dialog-select-directory-v2.tsx | 27 ++++++++++++------- .../app/src/components/directory-tree.test.ts | 8 ++++++ packages/app/src/components/directory-tree.ts | 9 +++++-- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/packages/app/src/components/dialog-select-directory-v2.tsx b/packages/app/src/components/dialog-select-directory-v2.tsx index e1391e56913..30d6012b0b0 100644 --- a/packages/app/src/components/dialog-select-directory-v2.tsx +++ b/packages/app/src/components/dialog-select-directory-v2.tsx @@ -10,6 +10,7 @@ import { useLanguage } from "@/context/language" import { ServerConnection } from "@/context/server" import { absoluteTreePath, + activeTreeNavigation, advanceTreePreload, nextSuggestionIndex, nextTreeScrollTop, @@ -42,6 +43,7 @@ export function DialogSelectDirectoryV2(props: DialogSelectDirectoryV2Props) { const [activeSuggestion, setActiveSuggestion] = createSignal(-1) const [loading, setLoading] = createSignal(false) const [error, setError] = createSignal(false) + const [rootValid, setRootValid] = createSignal(false) const listings = new Map | undefined>>() const advanced = new Set() let tree: FileTree | undefined @@ -77,7 +79,7 @@ export function DialogSelectDirectoryV2(props: DialogSelectDirectoryV2Props) { return Array.from(new Map(results.map((result) => [result.absolute, result])).values()).slice(0, 8) }) - async function load(path: string, preload = true) { + async function load(path: string, generation: number, preload = true) { const key = path.replace(/\/+$/, "") setError(false) const absolute = absoluteTreePath(root(), key) @@ -89,17 +91,19 @@ export function DialogSelectDirectoryV2(props: DialogSelectDirectoryV2Props) { .catch(() => undefined) listings.set(key, request) const nodes = await request + if (!activeTreeNavigation(generation, navigation)) return false if (!nodes) { listings.delete(key) - setError(true) - return + if (!key) setError(true) + return false } tree?.batch( policy.entries(key, nodes).map((item) => ({ type: "add", path: item })), ) if (preload && advanceTreePreload(advanced, key)) { - void Promise.all(preloadTreeDirectories(key, nodes).map((directory) => load(directory, false))) + void Promise.all(preloadTreeDirectories(key, nodes).map((directory) => load(directory, generation, false))) } + return true } async function navigate(path: string) { @@ -107,6 +111,7 @@ export function DialogSelectDirectoryV2(props: DialogSelectDirectoryV2Props) { if (!value) return const token = ++navigation setLoading(true) + setRootValid(false) setSelected("") setSuggestionsOpen(false) setActiveSuggestion(-1) @@ -115,8 +120,10 @@ export function DialogSelectDirectoryV2(props: DialogSelectDirectoryV2Props) { listings.clear() advanced.clear() tree?.resetPaths([]) - await load("") - if (token === navigation) setLoading(false) + const valid = await load("", token) + if (!activeTreeNavigation(token, navigation)) return + setRootValid(valid) + setLoading(false) } function complete() { @@ -177,7 +184,7 @@ export function DialogSelectDirectoryV2(props: DialogSelectDirectoryV2Props) { } function resolve() { - const path = policy.result(root(), selected()) + const path = policy.result(root(), selected(), rootValid()) if (!path) return props.onSelect(props.multiple ? [path] : path) dialog.close() @@ -214,7 +221,7 @@ export function DialogSelectDirectoryV2(props: DialogSelectDirectoryV2Props) { } `, onExpansionChange(change) { - if (change.expanded) void load(change.path) + if (change.expanded) void load(change.path, navigation) }, onSelectionChange(paths) { const path = paths.at(-1) @@ -300,11 +307,11 @@ export function DialogSelectDirectoryV2(props: DialogSelectDirectoryV2Props) {
{language.t("common.loading")}
Unable to read this folder
 -
{policy.result(root(), selected())}
+
{policy.result(root(), selected(), rootValid())}
dialog.close()}>{language.t("common.cancel")} - + {policy.action} diff --git a/packages/app/src/components/directory-tree.test.ts b/packages/app/src/components/directory-tree.test.ts index fc91359f6b3..c75819022c1 100644 --- a/packages/app/src/components/directory-tree.test.ts +++ b/packages/app/src/components/directory-tree.test.ts @@ -1,6 +1,7 @@ import { expect, test } from "bun:test" import { absoluteTreePath, + activeTreeNavigation, advanceTreePreload, nextSuggestionIndex, nextTreeScrollTop, @@ -51,11 +52,18 @@ test("centralizes file and directory selection policy", () => { expect(file.selection("/repo/src", "index.ts")).toBe("src/index.ts") expect(file.selection("/repo", "src/")).toBeUndefined() expect(file.result("/repo", "src/index.ts")).toBe("src/index.ts") + expect(file.selection("/tmp", "example.txt")).toBe("/tmp/example.txt") const directory = pickerMode("directory") expect(directory.includeFiles).toBeFalse() expect(directory.selection("/repo", "src/")).toBe("/repo/src") expect(directory.result("/repo", "")).toBe("/repo") + expect(directory.result("/repo", "", false)).toBeUndefined() +}) + +test("accepts mutations only from the active navigation", () => { + expect(activeTreeNavigation(3, 3)).toBeTrue() + expect(activeTreeNavigation(2, 3)).toBeFalse() }) test("scopes file autocomplete to the current browser root", () => { diff --git a/packages/app/src/components/directory-tree.ts b/packages/app/src/components/directory-tree.ts index 149b20d183c..140b65e7885 100644 --- a/packages/app/src/components/directory-tree.ts +++ b/packages/app/src/components/directory-tree.ts @@ -46,7 +46,8 @@ export function pickerMode(mode: "directory" | "file", base?: string) { nodes.filter((node) => node.type === "directory"), ) }, - result(root: string, selected: string) { + result(root: string, selected: string, valid = true) { + if (!valid) return return selected || root || undefined }, selection(root: string, path: string) { @@ -79,6 +80,10 @@ export function advanceTreePreload(advanced: Set, path: string) { return true } +export function activeTreeNavigation(request: number, current: number) { + return request === current +} + export function nextTreeScrollTop(current: number, delta: number, scrollHeight: number, clientHeight: number) { return Math.min(Math.max(0, scrollHeight - clientHeight), Math.max(0, current + delta)) } @@ -105,7 +110,7 @@ export function selectedTreePath(root: string, path: string, mode: "directory" | const prefix = absoluteTreePath(base, "") if (absolute === prefix) return "" if (absolute.startsWith(prefix + "/")) return absolute.slice(prefix.length + 1) - return path + return absolute } return directory ? absoluteTreePath(root, path) : undefined }