fix(frontend): task board scroll, session links, summary display by dimakis · Pull Request #417 · dimakis/mitzo

dimakis · 2026-06-27T23:33:13Z

Summary

Scrolling fix: Wrapped task list in the existing .task-board-scroll container (CSS was defined but never used in JSX). Task board now scrolls properly on mobile with momentum scrolling and tab bar padding.
Session links: Session hash in context line is now a clickable link that navigates to /chat/<sessionId>. Shown for all statuses, not just active.
Summary display: Renders task.summary below the context line when present, giving visibility into task outcomes.

Test plan

Open Task Board on mobile with 6+ tasks, verify scrolling works and bottom tasks are reachable
Tap a session hash link on an active task, verify it navigates to the correct chat session
Verify done/blocked/failed tasks also show session links when they have a sessionId
Verify task summary text appears below context line for tasks that have a summary set

🤖 Generated with Claude Code

…mary display Task board list was unscrollable on mobile because the scroll container CSS existed but wasn't used in JSX. Also adds clickable session hash links that navigate to the session chat, and renders task.summary when present for better status visibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

dimakis

Centaur Review

Found 4 issue(s) (1 critical) (2 warning).

`frontend/src/components/TaskNode.tsx`

The useNavigate() addition will break all existing TaskNode tests (missing Router context) — fix by wrapping test renders in MemoryRouter. The <a> without href should be a <Link> for accessibility and consistency.

🔴 regressions (L136): Adding useNavigate() at component level means every render of TaskNode now requires a Router context. The existing TaskNode.test.tsx tests render without MemoryRouter, so all 9 test cases will throw useNavigate() may be used only in the context of a <Router> component. The TaskBoard.test.tsx file already uses MemoryRouter and won't be affected. [fixable]
🟡 style (L64): The <a> element without an href attribute is not keyboard-focusable and not semantically a link. Since this navigates to an internal route, use react-router-dom's <Link to={/chat/${task.sessionId}}> instead — it's already a dependency, provides proper accessibility, and is consistent with how the rest of the app handles navigation (the only other <a> in the codebase uses href). [fixable]
🔵 style (L90): hasContextLine() duplicates the knowledge of which statuses produce a context line — it must stay in sync with ContextLine's switch cases. Consider removing it and instead rendering <ContextLine> unconditionally, since it already returns null for unhandled statuses. The wrapping <div className='task-node-context'> would need a small guard (e.g., render the div inside ContextLine), but it eliminates the duplication. [fixable]

`frontend/src/components/tests/TaskNode.test.tsx`

The useNavigate() addition will break all existing TaskNode tests (missing Router context) — fix by wrapping test renders in MemoryRouter. The <a> without href should be a <Link> for accessibility and consistency.

🟡 missing_tests: No tests cover the new features: session link rendering (when sessionId/sessionHash are present), click-to-navigate behavior, or task.summary display. Given the component gained a new sub-component (ContextLine) with conditional rendering across 5 status branches, at least a test confirming the session link appears for an active task with a sessionId, and a test confirming summary text renders, would be valuable. [fixable]

dimakis · 2026-06-27T23:35:20Z

  onReject,
 }: TaskNodeProps) {
  const [expanded, setExpanded] = useState(true);
+  const navigate = useNavigate();


🔴 regressions: Adding useNavigate() at component level means every render of TaskNode now requires a Router context. The existing TaskNode.test.tsx tests render without MemoryRouter, so all 9 test cases will throw useNavigate() may be used only in the context of a <Router> component. The TaskBoard.test.tsx file already uses MemoryRouter and won't be affected. [fixable]

dimakis · 2026-06-27T23:35:20Z

+  meta?: TaskDisplayMeta;
+  onNavigateSession?: (sessionId: string) => void;
+}) {
+  const dot = ' \u00B7 ';


🟡 style: The <a> element without an href attribute is not keyboard-focusable and not semantically a link. Since this navigates to an internal route, use react-router-dom's <Link to={/chat/${task.sessionId}}> instead — it's already a dependency, provides proper accessibility, and is consistent with how the rest of the app handles navigation (the only other <a> in the codebase uses href). [fixable]

dimakis · 2026-06-27T23:35:20Z

    case 'pending_review':
-      return 'awaiting approval';
+      return <>awaiting approval{sessionLink && <>{dot}{sessionLink}</>}</>;
    case 'blocked':


🔵 style: hasContextLine() duplicates the knowledge of which statuses produce a context line — it must stay in sync with ContextLine's switch cases. Consider removing it and instead rendering <ContextLine> unconditionally, since it already returns null for unhandled statuses. The wrapping <div className='task-node-context'> would need a small guard (e.g., render the div inside ContextLine), but it eliminates the duplication. [fixable]

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

dimakis

Centaur Review

Found 4 issue(s) (1 critical) (1 warning).

`frontend/src/components/TaskNode.tsx`

The useNavigate() addition will break all existing TaskNode tests due to missing Router context — critical fix needed before merge.

🔴 regressions (L183): useNavigate() is called unconditionally in TaskNode, but frontend/src/components/tests/TaskNode.test.tsx renders TaskNode without a wrapper. All 7 existing TaskNode tests will throw "useNavigate() may be used only in the context of a component." The TaskBoard test already wraps in MemoryRouter (line 93), but the TaskNode test does not. [fixable]
🟡 missing_tests (L67): No tests cover the new ContextLine component behavior: session link rendering, onNavigateSession callback invocation, or the summary display. Given TDD is required by project convention, tests for the session link click handler (navigate to /chat/:sessionId) and summary rendering should be added. [fixable]
🔵 style (L67): The element on line 67-76 has no href attribute. For an in-app navigation action, use a (styled as a link) or react-router's component. A bare without href is not keyboard-focusable by default and won't be announced as interactive by screen readers. [fixable]
🔵 style (L130): hasContextLine() duplicates the knowledge of which statuses produce context output — it must stay in sync with ContextLine's switch cases. Consider removing hasContextLine and instead letting ContextLine render (it already returns null for non-matching statuses), then conditionally wrapping based on whether the result is non-null, or just always rendering the wrapper div. [fixable]

dimakis · 2026-06-27T23:44:26Z

  onReject,
 }: TaskNodeProps) {
  const [expanded, setExpanded] = useState(true);
+  const navigate = useNavigate();


🔴 regressions: useNavigate() is called unconditionally in TaskNode, but frontend/src/components/tests/TaskNode.test.tsx renders TaskNode without a wrapper. All 7 existing TaskNode tests will throw "useNavigate() may be used only in the context of a component." The TaskBoard test already wraps in MemoryRouter (line 93), but the TaskNode test does not. [fixable]

dimakis · 2026-06-27T23:44:26Z

+  const dot = ' \u00B7 ';
+
+  const sessionLink =
+    task.sessionId && meta?.sessionHash ? (


🟡 missing_tests: No tests cover the new ContextLine component behavior: session link rendering, onNavigateSession callback invocation, or the summary display. Given TDD is required by project convention, tests for the session link click handler (navigate to /chat/:sessionId) and summary rendering should be added. [fixable]

dimakis · 2026-06-27T23:44:26Z

+  const dot = ' \u00B7 ';
+
+  const sessionLink =
+    task.sessionId && meta?.sessionHash ? (


🔵 style: The element on line 67-76 has no href attribute. For an in-app navigation action, use a (styled as a link) or react-router's component. A bare without href is not keyboard-focusable by default and won't be announced as interactive by screen readers. [fixable]

dimakis · 2026-06-27T23:44:26Z

+        </>
+      );
    case 'failed': {
      const label = meta?.blockerSummary ?? 'failed';


🔵 style: hasContextLine() duplicates the knowledge of which statuses produce context output — it must stay in sync with ContextLine's switch cases. Consider removing hasContextLine and instead letting ContextLine render (it already returns null for non-matching statuses), then conditionally wrapping based on whether the result is non-null, or just always rendering the wrapper div. [fixable]

- Replace bare <a> with react-router <Link> for session navigation - Remove useNavigate() from TaskNode (no longer needs Router at component level) - Remove hasContextLine() duplication, render ContextLine unconditionally - Wrap all tests in MemoryRouter, add tests for session link and summary Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

dimakis

Centaur Review

Found 4 issue(s) (1 warning).

`frontend/src/components/TaskNode.tsx`

The ContextLine refactor from string to JSX introduces a truthiness bug: the wrapper div now renders for statuses that should have no context line (pending/skipped), causing a spurious 2px gap. Everything else is solid — session links, summary display, scroll wrapper, and tests are well done.

🟡 bugs (L172): Regression: <ContextLine ... /> is always a truthy React element, so {contextLine && (<div>...)} always renders the wrapper <div class="task-node-context"> — even when ContextLine returns null (for pending and skipped statuses). The old buildContextLine returned string | null, which correctly suppressed the wrapper div. The empty div gets .task-node-context CSS (margin-top: 2px, font-size) causing a spurious 2px gap. Fix: either inline the <ContextLine> inside the conditional div and check status there, or assign the JSX to a variable inside the component body and check truthiness of a separate flag. [fixable]
🔵 style (L55): The session-link pattern {sessionLink && (<>{dot}{sessionLink}</>)} is repeated identically in all five case branches. Extracting it to a small SessionSuffix fragment or a variable rendered after the switch would remove ~25 lines of duplication without adding abstraction overhead. [fixable]

`frontend/src/components/tests/TaskNode.test.tsx`

The ContextLine refactor from string to JSX introduces a truthiness bug: the wrapper div now renders for statuses that should have no context line (pending/skipped), causing a spurious 2px gap. Everything else is solid — session links, summary display, scroll wrapper, and tests are well done.

🔵 missing_tests: No test for statuses where ContextLine returns null (e.g. pending, skipped in non-compact mode). Given the truthiness bug above, a test asserting .task-node-context is absent for pending tasks would catch the regression. [fixable]
🔵 missing_tests: Session link tests only cover the active status. The PR adds session links to pending_review, blocked, done, and failed statuses too — at least one additional status should be tested to verify the shared pattern works. [fixable]

dimakis · 2026-06-28T00:26:51Z

  if (meta && meta.fadeOpacity <= 0) classes.push('task-node--hidden');

-  const contextLine = isCompact ? null : buildContextLine(task, meta);
+  const contextLine = !isCompact ? <ContextLine task={task} meta={meta} /> : null;


🟡 bugs: Regression: <ContextLine ... /> is always a truthy React element, so {contextLine && (<div>...)} always renders the wrapper <div class="task-node-context"> — even when ContextLine returns null (for pending and skipped statuses). The old buildContextLine returned string | null, which correctly suppressed the wrapper div. The empty div gets .task-node-context CSS (margin-top: 2px, font-size) causing a spurious 2px gap. Fix: either inline the <ContextLine> inside the conditional div and check status there, or assign the JSX to a variable inside the component body and check truthiness of a separate flag. [fixable]

dimakis · 2026-06-28T00:26:51Z

 }

-function buildContextLine(task: Task, meta?: TaskDisplayMeta): string | null {
+function ContextLine({ task, meta }: { task: Task; meta?: TaskDisplayMeta }) {


🔵 style: The session-link pattern {sessionLink && (<>{dot}{sessionLink}</>)} is repeated identically in all five case branches. Extracting it to a small SessionSuffix fragment or a variable rendered after the switch would remove ~25 lines of duplication without adding abstraction overhead. [fixable]

…sion suffix duplication - Call ContextLine as function (returns ReactNode) so null is properly falsy and the wrapper div doesn't render for pending/skipped statuses - Extract SessionSuffix component to remove repeated session link pattern across all five case branches Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

dimakis

Centaur Review

Found 2 issue(s).

`frontend/src/components/TaskNode.tsx`

Clean PR — scroll wrapper, session links, and summary display are implemented correctly with good test coverage. Two minor suggestions: rename the function-called component to signal intent, and extend session-link tests to non-active statuses.

🔵 style (L148): ContextLine is PascalCased (React component convention) but invoked as a plain function call ContextLine({ task, meta }) instead of <ContextLine task={task} meta={meta} />. This works because ContextLine has no hooks, but it sidesteps React reconciliation and will break silently if someone later adds a hook. Consider either renaming it to buildContextContent (lowercased, non-component) to signal the intent, or rendering it as JSX and moving the nullability check inside the wrapper. [fixable]

`frontend/src/components/tests/TaskNode.test.tsx`

Clean PR — scroll wrapper, session links, and summary display are implemented correctly with good test coverage. Two minor suggestions: rename the function-called component to signal intent, and extend session-link tests to non-active statuses.

🔵 missing_tests (L162): Session link is only tested for active status. The diff adds {suffix} to pending_review, blocked, done, and failed branches as well. A parameterized test covering at least one other status (e.g., done with a sessionId) would guard against regressions in those paths. [fixable]

dimakis · 2026-06-28T20:42:18Z

  if (meta && meta.fadeOpacity <= 0) classes.push('task-node--hidden');

-  const contextLine = isCompact ? null : buildContextLine(task, meta);
+  const contextContent = !isCompact ? ContextLine({ task, meta }) : null;


🔵 style: ContextLine is PascalCased (React component convention) but invoked as a plain function call ContextLine({ task, meta }) instead of <ContextLine task={task} meta={meta} />. This works because ContextLine has no hooks, but it sidesteps React reconciliation and will break silently if someone later adds a hook. Consider either renaming it to buildContextContent (lowercased, non-component) to signal the intent, or rendering it as JSX and moving the nullability check inside the wrapper. [fixable]

dimakis · 2026-06-28T20:42:19Z

    expect(onDelete).toHaveBeenCalledWith('del-1');
  });
+
+  it('renders session link for active task with sessionId', () => {


🔵 missing_tests: Session link is only tested for active status. The diff adds {suffix} to pending_review, blocked, done, and failed branches as well. A parameterized test covering at least one other status (e.g., done with a sessionId) would guard against regressions in those paths. [fixable]

dimakis commented Jun 27, 2026

View reviewed changes

style(frontend): format TaskNode.tsx with prettier

c6b3483

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

dimakis commented Jun 27, 2026

View reviewed changes

dimakis commented Jun 28, 2026

View reviewed changes

dimakis merged commit cedd6ad into main Jun 28, 2026
1 check passed

dimakis deleted the fix/taskboard-scroll-and-visibility branch June 28, 2026 21:41

Conversation

dimakis commented Jun 27, 2026

Summary

Test plan

Uh oh!

dimakis left a comment

Choose a reason for hiding this comment

Centaur Review

frontend/src/components/TaskNode.tsx

frontend/src/components/__tests__/TaskNode.test.tsx

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimakis left a comment

Choose a reason for hiding this comment

Centaur Review

frontend/src/components/TaskNode.tsx

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimakis left a comment

Choose a reason for hiding this comment

Centaur Review

frontend/src/components/TaskNode.tsx

frontend/src/components/__tests__/TaskNode.test.tsx

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimakis left a comment

Choose a reason for hiding this comment

Centaur Review

frontend/src/components/TaskNode.tsx

frontend/src/components/__tests__/TaskNode.test.tsx

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

`frontend/src/components/TaskNode.tsx`

`frontend/src/components/tests/TaskNode.test.tsx`

`frontend/src/components/TaskNode.tsx`

`frontend/src/components/TaskNode.tsx`

`frontend/src/components/tests/TaskNode.test.tsx`

`frontend/src/components/TaskNode.tsx`

`frontend/src/components/tests/TaskNode.test.tsx`