feat(web): drag-reorder subtasks within a parent task#962
Merged
Conversation
📝 WalkthroughWalkthroughAdds per-parent subtask reordering: new UI state and actions, localStorage persistence and pruning, apply-view ordering, a SortableSubtaskList DnD component using ChangesSubtask drag-and-drop reordering
sequenceDiagram
participant Sidebar as TaskSessionSidebar
participant Prefs as useSidebarTaskPrefs
participant Switcher as TaskSwitcher
participant List as SortableSubtaskList
participant Storage as LocalStorage
Sidebar->>Prefs: read handleReorderSubtasks
Sidebar->>Switcher: pass onReorderSubtasks(handleReorderSubtasks)
Switcher->>List: render with onReorderSubtasks
List->>Switcher: onDragEnd -> onReorderSubtasks(parentId, orderedIds)
Switcher->>Prefs: invoke handleReorderSubtasks(parentId, orderedIds)
Prefs->>Storage: setStoredSubtaskOrderByParentId(updatedMap)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @jcfs's task in 3m 51s —— View job Code Review Complete
The feature is well-implemented with solid test coverage (73 tests). Three suggestions posted — all non-blocking:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
FindingsSuggestion (recommended, doesn't block)
Summary
Verdict: Ready to merge — no blockers. The three suggestions are low-urgency clean-up items that can land as a follow-up if preferred. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
| Filename | Overview |
|---|---|
| apps/web/components/task/task-switcher-subtask-dnd.tsx | New file: per-parent sortable subtask list with nested DndContext; correctly skips all DnD scaffolding (and grab cursor) when onReorderSubtasks is omitted. |
| apps/web/lib/sidebar/apply-view.ts | Adds applySubtaskOrder helper and wires it into applyView after separateSubtasks; listed subtasks come first in stored order, unlisted fall back to the active sort — logic is correct and isolated from pinning and root-task ordering. |
| apps/web/lib/state/slices/ui/sidebar-task-prefs-actions.ts | Adds setSubtaskOrder action (writes per-parent map, deletes key on empty array) and extends removeTaskFromSidebarPrefs to call pruneSubtaskOrder on the Immer draft; consistent with existing pin/order patterns. |
| apps/web/lib/local-storage.ts | Adds validated getStoredSubtaskOrderByParentId (guards against corrupt values), setStoredSubtaskOrderByParentId, and the shared pruneSubtaskOrder mutator; cleanupTaskStorage wired correctly. |
| apps/web/lib/sidebar/apply-view.test.ts | New describe block covers per-parent override, partial-override fallback, and cross-parent isolation; existing custom-sort test updated to reflect new subtask fallback behaviour. |
| apps/web/lib/state/slices/ui/ui-slice.test.ts | Good coverage: hydration from localStorage, setSubtaskOrder persistence, empty-array cleanup, and removeTaskFromSidebarPrefs parent-key and sibling-list pruning all tested. |
| apps/web/hooks/domains/sidebar/use-sidebar-task-prefs.ts | Exposes subtaskOrderByParentId and handleReorderSubtasks; handleReorderSubtasks is intentionally a thin wrapper that never touches the global view sort. |
| apps/web/components/task/task-session-sidebar.tsx | Extracts useGroupedSidebarView hook to pass subtaskOrderByParentId to applyView and threads handleReorderSubtasks down to TaskSwitcher cleanly. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User drags subtask] --> B[SortableSubtaskRow\nPointerSensor activates\n≥8px movement]
B --> C[handleDragEnd\narrayMove ids]
C --> D[onReorderSubtasks\nparentTaskId, newIds]
D --> E[setSubtaskOrder\nZustand action]
E --> F[Immer draft update\nsubtaskOrderByParentId]
E --> G[setStoredSubtaskOrderByParentId\nlocalStorage write]
F --> H[useMemo recomputes\napplyView]
H --> I[applySubtaskOrder\nlisted first, unlisted keep sort]
I --> J[Updated subTasksByParentId\nre-rendered list]
K[Task deleted] --> L[cleanupTaskStorage\nlocalStorage path]
K --> M[removeTaskFromSidebarPrefs\nZustand path]
L --> N[pruneSubtaskOrder\nplain object]
M --> O[pruneSubtaskOrder\nImmer draft]
N --> P[localStorage updated]
O --> Q[In-memory + localStorage updated]
Reviews (4): Last reviewed commit: "refactor(web): single-source DRAG_ACTIVA..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
added 2 commits
May 21, 2026 00:17
Adds per-parent subtask ordering in the sidebar. Drag a subtask under its parent to reorder; the chosen order is independent of the global sort and persists per browser via localStorage. - New `sidebarTaskPrefs.subtaskOrderByParentId` slice field and matching `setSubtaskOrder(parentId, ids)` action, persisted to localStorage under `kandev.sidebar.subtaskOrderByParentId`. - `applyView` applies the per-parent override after `separateSubtasks`, so it never touches root order or the active sort spec. - TaskSwitcher exposes `onReorderSubtasks(parentId, ids)`; each parent's subtask list lives in its own nested DndContext / SortableContext using the existing dnd-kit setup. - Task-deletion cleanup strips the deleted id as both parent key and subtask entry from `subtaskOrderByParentId` in store + localStorage.
- Extract `pruneSubtaskOrder(map, taskId)` helper in `local-storage.ts`; use it in both `cleanupTaskStorage` and `removeTaskFromSidebarPrefs` to keep the two cleanup paths in lockstep. - Move `SortableSubtaskRow`, `SortableSubtaskList`, and `useSubtaskDnd` into a new sibling file `task-switcher-subtask-dnd.tsx` so the switcher stays under the 600-line soft limit. - `SortableSubtaskList` now short-circuits to a plain row map when `onReorderSubtasks` is undefined — read-only callers no longer get a misleading grab cursor or unused DnD scaffolding.
jcfs
force-pushed
the
feature/order-subtasks-insid-bg4
branch
from
db8c2df to
ebbba6a
Compare
|
Claude finished @jcfs's task in 2m 47s —— View job Code Review
All three suggestions from the initial review have been addressed in the follow-up commit ( |
FindingsAll three suggestions from the initial review have been addressed:
No new issues found in the follow-up commit. A few notes for completeness:
Summary
Verdict: Ready to merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/components/task/task-switcher-subtask-dnd.tsx`:
- Around line 94-96: The non-DnD render path maps subtasks without keys, causing
unstable reconciliation; update the branch where onReorderSubtasks is falsy so
each mapped child has a stable key (use the subtask unique id field such as
sub.id or sub._id). Either add the key to the element returned by renderRow(sub)
or wrap renderRow(sub) in a keyed fragment (e.g., <React.Fragment
key={sub.id}>), ensuring you reference the subtasks array and renderRow function
so keys are derived from a stable identifier rather than array index.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aecbd9ba-54e2-416c-bb10-a1e33fdc88fb
📒 Files selected for processing (13)
apps/web/components/task/mobile/session-task-switcher-sheet.tsxapps/web/components/task/task-session-sidebar.tsxapps/web/components/task/task-switcher-subtask-dnd.tsxapps/web/components/task/task-switcher.tsxapps/web/hooks/domains/sidebar/use-sidebar-task-prefs.tsapps/web/lib/local-storage.tsapps/web/lib/sidebar/apply-view.test.tsapps/web/lib/sidebar/apply-view.tsapps/web/lib/state/slices/ui/sidebar-task-prefs-actions.tsapps/web/lib/state/slices/ui/types.tsapps/web/lib/state/slices/ui/ui-slice.test.tsapps/web/lib/state/slices/ui/ui-slice.tsapps/web/lib/state/store.ts
When `onReorderSubtasks` is undefined, the early-return branch in `SortableSubtaskList` mapped subtasks without keys, falling back to React's index-based reconciliation. Wrap each child in a keyed `Fragment` so reorders or size changes reconcile correctly.
|
Claude finished @jcfs's task in 2m 26s —— View job Code Review
FindingsAll three suggestions from the initial review and the CodeRabbit key-stability fix have landed. One minor item remains: Suggestion (recommended, doesn't block)
Summary
Verdict: Ready to merge. The one suggestion is a trivial constant dedup that can follow as cleanup. |
Export the constant from `task-switcher-subtask-dnd.tsx` and import it in `task-switcher.tsx` instead of declaring the same value twice.
|
Claude finished @jcfs's task in 1m 46s —— View job Code Review
FindingsThe latest commit ( No new issues found. All four review suggestions across the three previous review rounds have now been resolved:
Summary
Verdict: Ready to merge. |
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
1 participant
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.
Users wanted to control the order of subtasks under a parent in the sidebar without it forcing the whole sidebar into "Custom" sort. Subtasks are now drag-reorderable per parent, with the override scoped to that parent and the global sort untouched.
Important Changes
sidebarTaskPrefs.subtaskOrderByParentId: Record<parentId, ids[]>slice field +setSubtaskOrderaction, persisted to localStorage underkandev.sidebar.subtaskOrderByParentId.applyViewapplies the override afterseparateSubtasks(listed subtasks first in user order, rest keep the active sort's order).TaskSwitcheradds anonReorderSubtasks(parentId, ids)callback; each parent's subtask list wraps in its own nestedDndContext/SortableContextso subtask drags don't trigger root reorder.subtaskOrderByParentIdboth as a parent key and from any sibling's list (slice action +cleanupTaskStorage).Validation
pnpm --filter @kandev/web test run— 73/73 affected tests pass (slice action persistence/cleanup,applyViewper-parent override, isolation across parents, partial-override fallback).pnpm --filter @kandev/web lint— clean.pnpm --filter @kandev/web exec tsc --noEmit— clean (only pre-existing@/generated/*errors that exist onmaintoo).make -C apps/backend test lint— clean (touched no backend code, run as part of/verify).Possible Improvements
Low risk: ordering lives entirely in localStorage and is keyed on task id, so a stale entry survives only until task deletion. The applied override skips parents with no entry, so non-users see no change.
Checklist
apps/web/), I have added or updated Playwright e2e tests inapps/web/e2e/and verified them withmake test-e2e.