feat(todos): rich detail overlay with line navigation, commenting, and streamlined flow#40
feat(todos): rich detail overlay with line navigation, commenting, and streamlined flow#40Fatih0234 wants to merge 2 commits into
Conversation
…d streamlined flow This replaces the read-only detail overlay with an interactive one and streamlines the list->detail flow. Overlay improvements: - Line-by-line cursor navigation (↑/↓) with auto-scroll - Page scroll (←/→ or PgUp/PgDn) - Multi-select mode (v) to select body lines (s) - Comment extraction (n) — quotes selected/current line and prefills editor - Direct actions: Enter work, r refine, c close/reopen, d delete, Esc back Flow changes: - Enter on list item goes straight to detail overlay (was: action menu) - a opens action menu for copy/release/delete operations - Unified delete confirmation used by both overlay and action menu Closes mitsuhiko#34
There was a problem hiding this comment.
Pull request overview
This PR upgrades the todos TUI from a read-only detail overlay to an interactive overlay and streamlines list→detail navigation to make todos more usable and action-oriented.
Changes:
- Adds an interactive todo detail overlay with cursor navigation, multi-select, comment extraction, and inline actions (work/refine/close/delete).
- Changes the list flow so
Enteropens the detail overlay directly andaopens the action menu. - Introduces a shared delete confirmation flow usable from both overlay and action menu paths.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (keyData === "a" || keyData === "A") { | ||
| const selected = this.filteredTodos[this.selectedIndex]; | ||
| if (selected && this.onActionMenu) this.onActionMenu(selected); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Binding plain "a" to open the action menu prevents users from typing the letter "a" into the search input (since this handler runs before passing input to this.searchInput.handleInput). Consider using a modified key (e.g., Ctrl/Alt+...)
, or only triggering the action menu when the list (not the input) has focus.
| deleteConfirm = new TodoDeleteConfirmComponent(theme, message, (confirmed) => { | ||
| if (!confirmed) { | ||
| setActiveComponent(selector); | ||
| return; | ||
| } |
There was a problem hiding this comment.
confirmDelete always returns to the selector when the user cancels (!confirmed). When delete is initiated from the action menu, this changes the prior flow (cancel used to return to the action menu) and makes it impossible to continue from the menu after backing out. Consider letting confirmDelete accept a "return" component/callback (e.g., selector vs actionMenu) and using the appropriate one per caller.
| const result = await deleteTodo(todosDir, record.id, ctx); | ||
| if ("error" in result) { | ||
| ctx.ui.notify(result.error, "error"); | ||
| } else { | ||
| const updatedTodos = await listTodos(todosDir); | ||
| selector?.setTodos(updatedTodos); | ||
| ctx.ui.notify(`Deleted todo ${formatTodoId(record.id)}`, "info"); | ||
| } |
There was a problem hiding this comment.
confirmDelete duplicates the delete+refresh+notify logic that already exists in applyTodoAction for "delete". This duplication risks behavioral drift (e.g., notifications, refresh behavior, future error handling). Consider reusing the existing applyTodoAction(record, "delete") path (or extracting a shared helper) and then handling the post-action navigation (setActiveComponent(...)) in one place.
| const result = await deleteTodo(todosDir, record.id, ctx); | |
| if ("error" in result) { | |
| ctx.ui.notify(result.error, "error"); | |
| } else { | |
| const updatedTodos = await listTodos(todosDir); | |
| selector?.setTodos(updatedTodos); | |
| ctx.ui.notify(`Deleted todo ${formatTodoId(record.id)}`, "info"); | |
| } | |
| await applyTodoAction(record, "delete"); |
| function stripAnsi(str: string): string { | ||
| // eslint-disable-next-line no-control-regex | ||
| return str.replace(/\x1B\[[0-9;]*m/g, ""); | ||
| } |
There was a problem hiding this comment.
There is already a similar ANSI-stripping regex defined in extensions/prompt-editor.ts (inline in its render). Consider centralizing this helper (e.g., a shared util) to avoid duplicated regexes diverging over time.
- Bind action menu to Ctrl+A instead of plain 'a' to avoid clashing with search input - Fix confirmDelete to return to action menu when canceled from there - Reuse applyTodoAction for delete instead of duplicating logic - Extract shared stripAnsi utility into extensions/lib/utils.ts
This replaces the read-only detail overlay with an interactive one and streamlines the list→detail flow.
Overlay improvements:
v) to select body lines (s)n) — quotes selected/current line and prefills editorEnterwork,rrefine,cclose/reopen,ddelete,EscbackFlow changes:
Enteron list item goes straight to detail overlay (was: action menu)aopens action menu for copy/release/delete operationsCloses #34