From c51ec0a21b1997fb267289cecf493d941112d7a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Wed, 2 Sep 2026 00:03:37 +0000 Subject: [PATCH] fix(ui): keep file select-all inside text inputs (#3259) --- apps/web/components/task/file-browser.tsx | 3 +- .../e2e/tests/file-tree-multi-select.spec.ts | 35 ++++++ .../e2e/tests/task/file-tree-create.spec.ts | 43 +++++++ .../e2e/tests/task/mobile-file-viewer.spec.ts | 43 +++++++ docs/plans/file-tree-select-all-input/plan.md | 105 ++++++++++++++++++ .../task-01-scope-file-tree-select-all.md | 99 +++++++++++++++++ docs/specs/ui/README.md | 20 ++-- .../requirements/file-tree-keyboard-scope.md | 54 +++++++++ .../system-design/file-tree-keyboard-scope.md | 82 ++++++++++++++ 9 files changed, 470 insertions(+), 14 deletions(-) create mode 100644 docs/plans/file-tree-select-all-input/plan.md create mode 100644 docs/plans/file-tree-select-all-input/task-01-scope-file-tree-select-all.md create mode 100644 docs/specs/ui/requirements/file-tree-keyboard-scope.md create mode 100644 docs/specs/ui/system-design/file-tree-keyboard-scope.md diff --git a/apps/web/components/task/file-browser.tsx b/apps/web/components/task/file-browser.tsx index e5eee7e518..acd115f1c5 100644 --- a/apps/web/components/task/file-browser.tsx +++ b/apps/web/components/task/file-browser.tsx @@ -22,6 +22,7 @@ import { useTranslation } from "react-i18next"; import type { TFunction } from "i18next"; import { useMultiSelect } from "@/hooks/use-multi-select"; import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; +import { isEditableKeydownTarget } from "@/lib/keyboard/utils"; import { useContextFilesStore } from "@/lib/state/context-files-store"; import { FileBrowserHeader } from "./file-browser-header"; import { insertNodeInTree, removeNodeFromTree, FileBrowserContentArea } from "./file-browser-parts"; @@ -346,7 +347,7 @@ function useKeyboardShortcuts( return; if (e.key === "Escape") { clearSelection(); - } else if ((e.ctrlKey || e.metaKey) && e.key === "a") { + } else if ((e.ctrlKey || e.metaKey) && e.key === "a" && !isEditableKeydownTarget(e)) { e.preventDefault(); selectAll(); } diff --git a/apps/web/e2e/tests/file-tree-multi-select.spec.ts b/apps/web/e2e/tests/file-tree-multi-select.spec.ts index dd82a0a1c6..d16c316ba7 100644 --- a/apps/web/e2e/tests/file-tree-multi-select.spec.ts +++ b/apps/web/e2e/tests/file-tree-multi-select.spec.ts @@ -408,6 +408,41 @@ test.describe("File Tree Multi-Select", () => { // --- Keyboard --- + test("select-all selects every visible row from non-editable tree focus", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + // @covers AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.3 + const git = new GitHelper( + path.join(backend.tmpDir, "repos", "e2e-repo"), + makeGitEnv(backend.tmpDir), + ); + git.createFile("tree-select-all-a.ts", "a"); + git.createFile("tree-select-all-b.ts", "b"); + git.stageAll(); + git.commit("add select-all files"); + + const session = await setupFileTreeTest( + testPage, + apiClient, + seedData, + "ft-tree-select-all", + "FT Tree Select All", + ); + const firstFile = session.fileTreeNode("tree-select-all-a.ts"); + await expect(firstFile).toBeVisible({ timeout: 15_000 }); + const visibleRows = session.files.locator("[data-testid='file-tree-node']:visible"); + const visibleRowCount = await visibleRows.count(); + + const fileBrowser = firstFile.locator("xpath=ancestor::div[@tabindex='-1'][1]"); + await fileBrowser.focus(); + await fileBrowser.press("ControlOrMeta+a"); + + await expect(session.fileTreeSelectedNodes()).toHaveCount(visibleRowCount); + }); + test("escape clears selection", async ({ testPage, apiClient, seedData, backend }) => { const git = new GitHelper( path.join(backend.tmpDir, "repos", "e2e-repo"), diff --git a/apps/web/e2e/tests/task/file-tree-create.spec.ts b/apps/web/e2e/tests/task/file-tree-create.spec.ts index 99a8692c6f..094eaa5b7c 100644 --- a/apps/web/e2e/tests/task/file-tree-create.spec.ts +++ b/apps/web/e2e/tests/task/file-tree-create.spec.ts @@ -85,6 +85,49 @@ test.describe("File tree create file", () => { .toBe(true); }); + test("select-all stays in the new-file name input", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + // @covers AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.1 + // @covers AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.2 + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + git.createFile("select-all-alpha.ts", "alpha"); + git.createFile("select-all-beta.ts", "beta"); + git.stageAll(); + git.commit("seed select-all files"); + + const session = await setupTask( + testPage, + apiClient, + seedData, + "ft-create-select-all", + "FT Create Select All", + ); + await expect(session.fileTreeNode("select-all-alpha.ts")).toBeVisible({ timeout: 15_000 }); + + const input = await startCreateAtRoot(testPage); + const draftName = "draft-name.ts"; + await input.fill(draftName); + await input.press("ControlOrMeta+a"); + + await expect + .poll(async () => ({ + selection: await input.evaluate((element) => ({ + start: element.selectionStart, + end: element.selectionEnd, + })), + selectedRows: await session.fileTreeSelectedNodes().count(), + })) + .toEqual({ + selection: { start: 0, end: draftName.length }, + selectedRows: 0, + }); + }); + test("New file inside expanded folder creates the file in that folder", async ({ testPage, apiClient, diff --git a/apps/web/e2e/tests/task/mobile-file-viewer.spec.ts b/apps/web/e2e/tests/task/mobile-file-viewer.spec.ts index c5d89d909a..d9fc4fb112 100644 --- a/apps/web/e2e/tests/task/mobile-file-viewer.spec.ts +++ b/apps/web/e2e/tests/task/mobile-file-viewer.spec.ts @@ -66,6 +66,49 @@ async function setupMobileFileViewerTest({ return { session, filePath }; } +test.describe("Mobile file tree keyboard shortcuts", () => { + test("select-all stays in the new-file name input", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + // @covers AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.1 + // @covers AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.2 + const { session, filePath } = await setupMobileFileViewerTest({ + testPage, + apiClient, + seedData, + backend, + taskTitle: "Mobile File Create Select All", + }); + + await testPage.getByRole("button", { name: "Files" }).tap(); + await expect(session.fileTreeNode(filePath)).toBeVisible({ timeout: 15_000 }); + + await testPage.getByRole("button", { name: "New file" }).tap(); + const input = testPage.getByPlaceholder("filename..."); + await expect(input).toBeFocused({ timeout: 5_000 }); + + const draftName = "mobile-draft.ts"; + await input.fill(draftName); + await input.press("ControlOrMeta+a"); + + await expect + .poll(async () => ({ + selection: await input.evaluate((element) => ({ + start: element.selectionStart, + end: element.selectionEnd, + })), + selectedRows: await session.fileTreeSelectedNodes().count(), + })) + .toEqual({ + selection: { start: 0, end: draftName.length }, + selectedRows: 0, + }); + }); +}); + test.describe("Mobile file viewer panel", () => { test.describe.configure({ retries: 1 }); diff --git a/docs/plans/file-tree-select-all-input/plan.md b/docs/plans/file-tree-select-all-input/plan.md new file mode 100644 index 0000000000..15f0933e95 --- /dev/null +++ b/docs/plans/file-tree-select-all-input/plan.md @@ -0,0 +1,105 @@ +--- +created: 2026-09-01 +status: complete +requirements: + - REQ-UI-FILE-TREE-KEYBOARD-SCOPE-001 +system_design: + - ../../specs/ui/system-design/file-tree-keyboard-scope.md +legacy_specs: [] +--- + +# Implementation Plan: File Tree Select-All Input Scope + +## Overview + +Keep Command+A and Control+A inside focused Files-panel text controls without +removing the file tree's existing visible-row select-all shortcut. One +sequential frontend slice adds a focused event-target guard and proves desktop +and hardware-keyboard mobile behavior. + +## Confirmed root cause + +`useKeyboardShortcuts` in `apps/web/components/task/file-browser.tsx` attaches a +keydown listener to the full file-browser container. When a nested filename +input emits lowercase Command+A or Control+A, the event bubbles to that +listener. The listener checks only that focus is somewhere inside the +container, then calls `preventDefault()` and `useMultiSelect.selectAll()`. + +A temporary production-build Playwright repro observed the filename selection +remaining at `13..13`, all three visible tree rows becoming selected, and the +lowercase `a` event reaching the document with `defaultPrevented: true`. + +## Scope + +### In scope + +- Defer the file-tree select-all shortcut when its originating target is an + editable Files-panel control. +- Preserve native select-all behavior in new-file, rename, and search inputs. +- Preserve visible-row select-all when the non-editable tree surface owns + focus. +- Prove shared desktop and hardware-keyboard mobile behavior. + +### Out of scope + +- File creation, rename, search, multi-selection, drag/drop, or Escape behavior + beyond select-all event ownership. +- New shortcut preferences or changes to key combinations. +- Files-panel markup, layout, touch actions, scrolling, or copy. +- Backend, API, persistence, or localization changes. + +## Technical approach + +- Import `isEditableKeydownTarget` from + `apps/web/lib/keyboard/utils.ts` into + `apps/web/components/task/file-browser.tsx`. +- In `useKeyboardShortcuts`, gate only the Command+A / Control+A branch on the + event target being non-editable. Keep focus containment, Escape behavior, + `preventDefault`, and `useMultiSelect.selectAll` otherwise unchanged. +- Reuse the established editable-target boundary rather than adding a local + element-type helper or changing nested input handlers. + +## Tests + +- `AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.1` and `.2`: extend + `apps/web/e2e/tests/task/file-tree-create.spec.ts` with the exact reported + flow. Type a draft filename, press lowercase `ControlOrMeta+a`, await the next + paint, then assert the full input selection range and zero selected tree + rows. +- `AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.3`: extend + `apps/web/e2e/tests/file-tree-multi-select.spec.ts` to focus the non-editable + file-browser container, press lowercase `ControlOrMeta+a`, and assert every + visible row is selected. + +## E2E tests + +- Desktop Chromium: the new-file regression fails before the correction for + both observable symptoms; the non-editable tree case protects existing + multi-selection behavior. +- Mobile Chromium: extend + `apps/web/e2e/tests/task/mobile-file-viewer.spec.ts` to enter the existing + Files surface, focus the new-file input, and prove a hardware-keyboard + Control+A remains inside the input. Touch and visual behavior are unchanged, + so no new mobile composition or screenshot contract is needed. + +## Work orders + +- [x] [Task 01: Scope file-tree select-all to focus](task-01-scope-file-tree-select-all.md) + +## Verification results + +Task 01 completed. The targeted desktop input, desktop tree, and mobile input +Playwright checks each passed against the rebuilt production bundle. Frontend +typecheck also passed. The work-order results retain the exact commands and RED +evidence. + +## Risks + +- An overbroad early return could disable Escape clearing or the tree's own + select-all behavior. Gate only the select-all branch and cover both sides. +- Playwright key spelling is case-sensitive for `event.key`. Use lowercase + `ControlOrMeta+a` so the regression exercises the production branch. + +## Open questions + +None. diff --git a/docs/plans/file-tree-select-all-input/task-01-scope-file-tree-select-all.md b/docs/plans/file-tree-select-all-input/task-01-scope-file-tree-select-all.md new file mode 100644 index 0000000000..60b122b5c1 --- /dev/null +++ b/docs/plans/file-tree-select-all-input/task-01-scope-file-tree-select-all.md @@ -0,0 +1,99 @@ +--- +id: "01-scope-file-tree-select-all" +title: "Scope file-tree select-all to focus" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-UI-FILE-TREE-KEYBOARD-SCOPE-001 +acceptance_criteria: + - AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.1 + - AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.2 + - AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.3 +system_design: + - ../../specs/ui/system-design/file-tree-keyboard-scope.md +--- + +# Task 01: Scope File-Tree Select-All to Focus + +## Summary + +Make the file-browser container defer Command+A and Control+A to focused text +controls while retaining visible-row selection for the non-editable tree. Add +desktop and mobile Playwright regressions before changing production code. + +## In scope + +- Add the editable-target guard to the file browser's select-all shortcut. +- Cover new-file input ownership on desktop and hardware-keyboard mobile. +- Cover retained tree-level select-all with non-editable focus. + +## Out of scope + +- Other file-browser keyboard behavior or shortcut configuration. +- Input markup, Files-panel layout, touch interactions, or backend behavior. +- New localization keys or public documentation. + +## Acceptance + +- A focused new-file control selects its complete value without selecting any + file-tree row on desktop and mobile hardware-keyboard paths. +- Other editable Files controls inherit the same guard through the shared + target classifier without per-input handlers. +- A focused non-editable tree container still selects every visible row. + +## Verification + +```bash +cd apps/web && pnpm e2e:run tests/task/file-tree-create.spec.ts -- --grep "select-all" +cd apps/web && pnpm e2e:run --no-build tests/file-tree-multi-select.spec.ts -- --grep "select-all" +cd apps/web && pnpm e2e:run --no-build --project mobile-chrome tests/task/mobile-file-viewer.spec.ts -- --grep "select-all" +cd apps/web && pnpm run typecheck +``` + +## Files likely touched + +- `apps/web/components/task/file-browser.tsx` +- `apps/web/e2e/tests/task/file-tree-create.spec.ts` +- `apps/web/e2e/tests/file-tree-multi-select.spec.ts` +- `apps/web/e2e/tests/task/mobile-file-viewer.spec.ts` + +## Dependencies + +None. + +## Risks + +- Guarding the whole keydown listener would change Escape behavior; restrict + the guard to the select-all branch. +- Uppercase Playwright shortcut spelling would miss the lowercase production + comparison and yield a false green test. + +## Parallelism + +`sequential` + +## Inputs + +- [File Tree Keyboard Scope Requirements](../../specs/ui/requirements/file-tree-keyboard-scope.md) +- [File Tree Keyboard Scope System Design](../../specs/ui/system-design/file-tree-keyboard-scope.md) +- Confirmed source path in `FileBrowser.useKeyboardShortcuts` and established + editable-target behavior in `useAppShortcuts`. + +## Results + +Done. + +- Reused `isEditableKeydownTarget` to defer only the file tree's select-all + branch when an editable descendant originated the event. Escape and + non-editable tree shortcuts remain unchanged. +- Added desktop and mobile new-file regressions plus a desktop preservation + test for visible-row select-all. +- RED evidence: before the fix, the desktop draft stayed selected at `13..13` + while three rows became selected; the mobile draft stayed at `15..15`. +- Verification passed: + - `cd apps/web && pnpm e2e:run tests/task/file-tree-create.spec.ts -- --grep "select-all"` + - `cd apps/web && pnpm e2e:run --no-build tests/file-tree-multi-select.spec.ts -- --grep "select-all"` + - `cd apps/web && pnpm e2e:run --no-build --project mobile-chrome tests/task/mobile-file-viewer.spec.ts -- --grep "select-all"` + - `cd apps/web && pnpm run typecheck` diff --git a/docs/specs/ui/README.md b/docs/specs/ui/README.md index 9fa1a1fea0..75c8054fc0 100644 --- a/docs/specs/ui/README.md +++ b/docs/specs/ui/README.md @@ -12,12 +12,9 @@ owners: ## Ownership UI owns reusable presentation and responsive interaction. - -## Exclusions - [Tasks](../tasks/README.md), [Agents](../agents/README.md), -[Plugins](../plugins/README.md), and [Integrations](../integrations/README.md) -own feature behavior/state. +[Plugins](../plugins/README.md), and [Integrations](../integrations/README.md) own +feature behavior and state. ## Specification map @@ -58,6 +55,7 @@ own feature behavior/state. - [Executor settings card spacing](requirements/executor-settings-card-spacing.md) - [External VCS File Links](requirements/external-vcs-file-links.md) - [File Tree Chat Context](requirements/file-tree-chat-context.md) +- [File Tree Keyboard Scope](requirements/file-tree-keyboard-scope.md) - [Reload Kandev after frozen-tab restore](requirements/fix-duplicated-tab-stale-data.md) - [GitHub PR Review Actions](requirements/github-pr-review-actions.md) - [GitHub Saved-Query Default Views](requirements/github-saved-query-defaults.md) @@ -157,6 +155,7 @@ own feature behavior/state. - [Task PR Automation Controls System Design Part 3](system-design/ci-pr-automation-03.md) - [Merge Queue Recovery Controls](system-design/ci-pr-merge-queue-recovery-controls.md) - [Entity Reference Composer](system-design/entity-reference-composer.md) +- [File Tree Keyboard Scope](system-design/file-tree-keyboard-scope.md) - [Kandev MCP Tool Results](system-design/kandev-mcp-tool-results.md) - [Mobile Task Chrome](system-design/mobile-task-chrome.md) - [Persistent status motion](system-design/persistent-status-motion.md) @@ -180,13 +179,8 @@ own feature behavior/state. - [Task Transcript History Visibility](system-design/task-prompt-transcript-visibility.md) - [Transcript Auto-scroll Stability](system-design/transcript-auto-scroll.md) -## Migration record - -Legacy source detail is still moving to the canonical requirement and -system-design documents above. - ## Related systems -- [Tasks](../tasks/README.md): task state. -- [Plugins](../plugins/README.md): contributions. -- [Platform](../platform/README.md): runtime state. +- [Tasks](../tasks/README.md) +- [Plugins](../plugins/README.md) +- [Platform](../platform/README.md) diff --git a/docs/specs/ui/requirements/file-tree-keyboard-scope.md b/docs/specs/ui/requirements/file-tree-keyboard-scope.md new file mode 100644 index 0000000000..e297976043 --- /dev/null +++ b/docs/specs/ui/requirements/file-tree-keyboard-scope.md @@ -0,0 +1,54 @@ +--- +status: draft +system: ui +created: 2026-09-01 +owners: + - kandev +--- + +# File Tree Keyboard Scope Requirements + +## Overview + +The Files panel combines tree-level multi-selection shortcuts with nested text +controls for creating, renaming, and searching files. The UI system owns which +focused surface receives a keyboard shortcut; task and filesystem systems keep +ownership of file data and mutations. + +## Terminology + +- **Platform select-all shortcut:** Command+A on macOS and Control+A on other + supported desktop platforms. +- **Editable Files control:** A text input, textarea, or contenteditable surface + rendered inside the Files panel. + +## Requirements + +### REQ-UI-FILE-TREE-KEYBOARD-SCOPE-001: Focus-owned select-all behavior + +**Intent:** Keep text-editing shortcuts inside the focused editor while +preserving file-tree multi-selection when the tree itself owns keyboard focus. + +**User story:** As a Kandev user, I want the platform select-all shortcut to +select the filename I am editing, so that replacing it does not also select +files in the tree. + +#### Acceptance criteria + +- **AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.1:** When the new-file name control has + focus, invoking the platform select-all shortcut shall select the control's + complete text value and shall not change file-tree row selection. +- **AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.2:** When any other editable Files + control has focus, including inline rename and file search, invoking the + platform select-all shortcut shall remain owned by that control and shall not + invoke file-tree multi-selection. +- **AC-UI-FILE-TREE-KEYBOARD-SCOPE-001.3:** When keyboard focus belongs to the + non-editable file-tree surface, invoking the platform select-all shortcut + shall continue to select every currently visible file-tree row. + +## Out of scope + +- Changing which file-tree rows are visible or eligible for multi-selection. +- Adding, removing, or making the file-tree shortcut configurable. +- Changing file creation, rename, search, or Escape-key behavior. +- Changing Files-panel layout, touch controls, navigation, or filesystem APIs. diff --git a/docs/specs/ui/system-design/file-tree-keyboard-scope.md b/docs/specs/ui/system-design/file-tree-keyboard-scope.md new file mode 100644 index 0000000000..3209ba8538 --- /dev/null +++ b/docs/specs/ui/system-design/file-tree-keyboard-scope.md @@ -0,0 +1,82 @@ +--- +status: draft +system: ui +requirements: + - REQ-UI-FILE-TREE-KEYBOARD-SCOPE-001 +--- + +# File Tree Keyboard Scope System Design + +## Purpose and boundaries + +This design owns keyboard-event routing inside the shared `FileBrowser`. It +keeps native text editing authoritative while an editable descendant has focus +and preserves the existing visible-row selection model everywhere else. It +does not change file data, filesystem operations, task state, or shortcut +configuration. + +## Requirement mapping + +| Requirement | Design section | +| --- | --- | +| `REQ-UI-FILE-TREE-KEYBOARD-SCOPE-001` | [Shortcut ownership](#shortcut-ownership), [Responsive surfaces](#responsive-surfaces) | + +## Components and responsibilities + +`FileBrowser` in `apps/web/components/task/file-browser.tsx` owns the container +keydown listener through `useKeyboardShortcuts`. `useMultiSelect` continues to +own selected paths and the visible-row `selectAll` operation. Nested controls +such as `InlineFileInput`, `TreeNodeName`, and the file-search input continue to +own their text values and local editing behavior. + +The existing `isEditableKeydownTarget` helper in +`apps/web/lib/keyboard/utils.ts` remains the shared classification boundary for +input, textarea, and contenteditable key targets. The file browser reuses that +boundary instead of defining another editable-element detector. + +## Shortcut ownership + +The container listener first keeps its existing check that keyboard focus is +inside the mounted file browser. Escape handling remains unchanged. + +For Command+A or Control+A, the listener examines the originating keydown +target before preventing the browser default: + +1. If the target is editable, the file browser returns without calling + `preventDefault` or `useMultiSelect.selectAll`. Native browser selection and + the focused control retain the event. +2. If the target is not editable, the file browser prevents native document + selection and calls `useMultiSelect.selectAll` for the current visible paths. + +No new React state, event bus, store field, or browser persistence is added. + +## Responsive surfaces + +Desktop and mobile task layouts mount the same `FileBrowser`, so the ownership +check also applies when a hardware keyboard is used with the phone Files +surface. The nearest mobile baseline remains +`apps/web/e2e/tests/task/mobile-file-viewer.spec.ts`, which enters Files through +the existing bottom navigation. This change adds no mobile composition, +scrolling, safe-area, touch-target, or gesture behavior. + +## Failure and compatibility + +An unrecognized non-editable descendant follows the existing tree shortcut +path. An editable descendant follows the same established guard used by global +application and plugin shortcut dispatchers. File-tree Escape behavior, row +selection state, drag/drop, create, rename, and search flows remain unchanged. + +## Verification + +Desktop Playwright coverage shall reproduce the new-file flow with lowercase +`ControlOrMeta+a`, because Playwright's uppercase key spelling emits +`event.key === "A"` and bypasses the current lowercase shortcut branch. The +test shall assert both the input selection range and zero selected tree rows +after the next browser paint. A second desktop case shall prove that the same +shortcut still selects all visible rows when the non-editable tree container +has focus. + +The existing mobile file-viewer spec shall exercise the filename input with a +hardware-keyboard Control+A event and prove the same focus ownership. No visual +snapshot is required because markup, geometry, and rendered styling do not +change.