fix(desktop): contain deeply nested task rows in the task panel - #4215
fix(desktop): contain deeply nested task rows in the task panel#4215liuxiaocs7 wants to merge 4 commits into
Conversation
2d63592 to
8f4c92b
Compare
hqhq1025
left a comment
There was a problem hiding this comment.
Codex-assisted review performed under the maintainer-approved review workflow.
I found one P2 coverage gap. This PR adds two real-component Storybook states for recent terminal tasks and an eight-level task chain; I inspected the complete diff plus TaskLedgerPanel, its model tests, CSS, and the latest-main synthetic merge.
Validation: hosted test is green; exact-head Storybook typecheck, build, 198-story interaction/AX smoke, Biome, and the 264-test UI suite passed. The synthetic merge 8340a25894e60f2e1d74613ca091c57a466239ae also passed Storybook typecheck/build and the 203-story interaction/AX smoke, with ASF headers clean.
I could not validate native Electron-only behavior beyond the real Storybook component path, but the reported overflow is deterministic DOM geometry in both wide and narrow canvases.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
Eighth surface under apache#3944 (one surface per PR): extend the 任务 panel (TaskLedgerPanel, via Product/Session Workbar) with the states the existing Tasks / TasksEmpty / TasksLoadFailed stories never reach. - TasksRecentlyFinished — the 最近结束 section with a failed task (+ reason) and a cancelled task, plus four finished tasks so the three-item cap drops the oldest. - TasksDeepNesting — an eight-level subtask chain; the play asserts the deepest row renders (reachability). The indent clamp (`--task-depth`) is a computed-style contract left to focused tests (review feedback). pending / in_progress / blocked / completed, owners, and shallow nesting were already covered by Tasks; failed, cancelled, the recent-finished cap, and deep nesting were not. stale / responding / unread / project-grouping have no UI in this panel and are left out. Refs apache#3944, apache#3893 Generated-by: Claude Code
Keep nested task groups at the panel width, apply depth indentation only to row content, and assert the deepest task has no horizontal overflow. Generated-by: Codex
da68dc2 to
0f2e1f7
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for adding focused coverage for the deep-nesting state and for moving depth padding off the recursive group wrapper. I found one remaining bounded case at the supported minimum Workbar width below. This is a suggestion from an outside review rather than a Request Changes review; please push back if I have misunderstood the panel-width contract.\n\nAI-assisted review disclosure: Codex ran independent code/UI and test-quality lanes; Astro-Han independently verified the exact-head CSS, production width authority, reachability, CI, and severity and owns this review.
The deep-nesting containment fix collapsed to the compact three-column row via a viewport @media (max-width: 620px). But the right workbar resizes down to SESSION_WORKBAR_MIN_WIDTH (320px) inside an otherwise wide window, where that viewport query never fires — so at clamped depth 6 the 128px indent plus key/subject/status columns overflowed the 320px panel horizontally. Make .maka-task-ledger-panel a named inline-size container and select the compact layout with @container (max-width: 420px), matching the inspector/chat-header container-query precedent. 420px keeps the 480px default wide while covering the 320px floor and the common 400px resize width. Pin TasksDeepNesting at width=320 so its containment assertion exercises the supported minimum panel width.
|
Thanks for turning both review rounds around properly — I want to be clear up front that what follows is not a problem with your work. This PR has been superseded by #4351, which merged on 2026-09-01. That change retired the Task Ledger demand chain and replaced it with a Host-owned, flat SessionTodo document. <li className="maka-task-ledger-row" key={...}>
<StatusIcon size={ICON_SIZE.control} aria-hidden="true" />
<span>{item.content}</span>
</li>Only the class names carried over. Every premise this PR builds on is gone from For the record, both rounds of feedback were resolved correctly before that landed. The Suggest converting this to draft (or closing it) and re-deciding what coverage is still worth having against Reviewed at head AI use: Claude Code traced the supersede against |
|
Closing this PR: it has been obsoleted by #4351 ( The nested rendering this PR fixes no longer exists on main:
Because the whole premise is gone, rebasing onto main produces a modify/delete conflict against a deleted file rather than a resolvable merge, so continuing this PR would mean re-introducing retired code. Closing instead. Thanks @hqhq1025 and @Astro-Han for the reviews — the 320px containment concern is moot under the flat panel. If the leftover dead rules in |
|
Thanks @Astro-Han and @hqhq1025 for the reviewing, close it now! |
Summary
Extends the real
TaskLedgerPanelStorybook surface with task states that the existing Tasks / TasksEmpty / TasksLoadFailed stories did not cover:TasksRecentlyFinishedcovers failed and cancelled outcomes, failure detail, and the three-item recent-terminal cap.TasksDeepNestingcovers an eight-level task chain and asserts that the deepest visible row remains contained with no horizontal overflow.Refs #3944, #3893
Verification
npm --workspace @maka/ui run typechecknpm --workspace @maka/desktop run typecheck:storiesnpm --workspace @maka/ui run test— 275 tests passednpx biome check apps/desktop/stories/session-workbar.stories.tsx apps/desktop/src/renderer/styles/task-ledger.css packages/ui/src/task-ledger-panel.tsxnpm --workspace @maka/desktop run build-storybooknpm --workspace @maka/desktop run smoke:storybook— 207 stories passed, includingproduct-session-workbar--tasks-deep-nestingAI use
Tool(s) and scope: Claude Code authored the original Storybook stories. Codex resolved the merge conflict, implemented the nested-task layout fix, added the overflow regression assertion, and ran verification. Affected commits retain
Generated-bytrailers.Checklist
Does this PR entail a change in behavior?