[5/7] Open tasks in a modal over the board - #276
Open
alex-clickhouse wants to merge 3 commits into
Open
Conversation
3 tasks
alex-clickhouse
force-pushed
the
alex-clickhouse/task-detail-modal
branch
from
August 5, 2026 10:31
4569a58 to
6c8449b
Compare
alex-clickhouse
changed the title
There was a problem hiding this comment.
Pull request overview
This PR introduces background-location routing so opening a task from the Tasks board overlays task details in a modal without unmounting the board (preserving scroll/position and keeping /tasks/:taskId shareable), and adds Tasks-scoped keyboard shortcuts plus a ShortcutsModal section. It also removes a dead TaskList component and adds new task detail modal/body components.
Changes:
- Implement background-location routing in
App.tsxand navigate-to-task withstate.backgroundfromTasksPage. - Add
TaskDetailModal(modal overlay) andTaskDetailBody(extracted detail/editor UI) components. - Add Tasks page-scoped keyboard shortcuts and document them in
ShortcutsModal; extend shortcut section typing; delete unusedTaskList.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/utils/keyboard.ts | Extends ShortcutDef.section to include a new tasks section. |
| web/src/pages/TasksPage.tsx | Adds tasks-scoped shortcuts and navigates to task detail with background state for modal routing. |
| web/src/components/Tasks/TaskList.tsx | Deletes a dead/unused legacy tasks list component. |
| web/src/components/Tasks/TaskDetailModal.tsx | Adds the modal-rendered task detail route for board overlay behavior. |
| web/src/components/Tasks/TaskDetailBody.tsx | Adds the extracted task detail editor/body intended to be shared between modal and full page. |
| web/src/components/ShortcutsModal.tsx | Documents Tasks shortcuts in the shortcuts modal. |
| web/src/App.tsx | Implements background-location routing and conditional modal route overlay for /tasks/:taskId. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3 tasks
alex-clickhouse
changed the title
alex-clickhouse
force-pushed
the
alex-clickhouse/task-detail-modal
branch
from
August 5, 2026 12:07
32d204d to
89d6161
Compare
alex-clickhouse
marked this pull request as ready for review
August 5, 2026 12:26
4 tasks
Clicking a board card navigated to the full task page, tearing down the board and losing its scroll position — for what is usually a glance at the description. /tasks/:taskId now renders either way depending on how it was reached. Entered from the board it carries a `background` location in history state, so App renders the board's route tree and overlays the task as a dialog; a cold load, a refresh, or a shared link has no such state and gets the ordinary full page. The URL is the same either way, so links stay shareable and Back still means back. First use of location.state in the app. The editor itself moves into TaskDetailBody, shared by the page and the modal so the two can't drift — only the surrounding chrome differs. One behaviour change fell out of the extraction: content no longer resyncs from the store while edits are unsaved, because a task_updated broadcast arriving mid-typing would otherwise discard what you were writing. Also adds page-scoped shortcuts (b/l to switch view, n for new, / to focus search) and the Tasks section in ShortcutsModal — whose comment says the list is manual and must be kept in sync. Card-level movement is left to dnd-kit's own keyboard handling rather than reimplemented. Deletes components/Tasks/TaskList.tsx: 89 lines, zero importers, dead since the page was rewritten. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
At 620px the markdown body wrapped prose into a column too narrow to read comfortably, let alone edit — the dialog was sized like a form when it holds a document. Adds a `wide` size to Modal (min(1100px, 94vw)) rather than a one-off override, since any future document-shaped dialog wants the same thing, and takes the task modal to 85vh so the editor has vertical room too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ialog Five review findings on the task modal. TaskDetailBody said it was shared by the full page and the modal, and it was not — TaskDetailPage still hand-rolled its own copy of the same editor, so the claim was false and the two had already drifted: the status-history button lands on the body only, leaving the full page quietly missing a shipped feature. The page now renders TaskDetailBody and keeps just the chrome the modal has no use for, a back button and a heading. That is a visible change to the full page — the textarea and preview take the body's metrics, and the meta row, edit/preview toggle and Save move out of the page header into the body's single row. Page shortcuts kept firing while a dialog was open. /tasks stays mounted behind the task modal by design, so its document listener stayed live: `n` opened a second dialog on top of the first, and `/` moved focus out of an aria-modal dialog into a search box behind the backdrop, where typing edits something the user cannot see. Gating on the URL would only cover the task modal, and the same leak reproduces under the shortcuts dialog with no background routing at all, so the gate is "is any dialog open" — the modal stack Modal already keeps, moved to its own module so a page can read it without turning Modal.tsx into a mixed component/util file. The shortcuts list rendered ArrowUp for a binding that is all four arrows, and its file comment still claimed the only handlers were in App.tsx and ChatPage.tsx. The dirty guard gets the unit test it warrants now that two surfaces depend on it, and an honest comment: it is defensive, not a fix for something reachable today, because a task_updated broadcast carries the tasks row and that table has no content column. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
alex-clickhouse
force-pushed
the
alex-clickhouse/task-detail-modal
branch
from
August 6, 2026 17:00
89d6161 to
7384d6d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Clicking a board card navigated to the full task page — tearing down the board and losing its scroll position for what is usually a glance at the description.
Background-location routing
/tasks/:taskIdnow renders either way, depending on how it was reached:backgroundlocation in history state, soApprenders the board's route tree and overlays the task as a dialog.Same URL either way. Links stay shareable, Back still means back, and the board survives underneath. First use of
location.statein the app; the pattern is confined toApp.tsxandTasksPage.openTask.The modal's close does
navigate(-1)rather thannavigate('/tasks'), so returning restores the board with its scroll intact.Shared editor
TaskDetailBodynow holds the metadata row, edit/preview toggle, markdown body, and save — used by both the page and the modal so they can't drift. Only the surrounding chrome differs.One behaviour change fell out of the extraction, and it's a fix: content no longer resyncs from the store while edits are unsaved. Previously a
task_updatedbroadcast landing mid-typing would replace the textarea contents. That was unreachable before #275 (nothing broadcast); with live updates in, it's a real way to lose work.Also
b/lview,nnew,/focus search), bound only while/tasksis mounted — theChatPagepattern. Card movement is left to dnd-kit's own keyboard handling rather than reimplemented on top of it.ShortcutsModal, whose own comment says the list is manual and must be kept in sync with the handlers.components/Tasks/TaskList.tsx— 89 lines, zero importers, dead since the page was rewritten.Testing
npm test— 46 passednpx tsc -b— cleannpm run build— cleannpx eslint .— 148 problems vs 152 onmain; net −4No new specs here: the routing behaviour is a
<Routes location=…>swap that's better checked by clicking than by mocking a router, andTaskDetailBodyis a move rather than new logic. The unsaved-edits guard is the one thing here I'd want a spec for — flagging it rather than quietly skipping it.Still pending across the stack: the live drag pass, which needs #272's API deployed (
nerve restarton the instance).🤖 Generated with Claude Code