feat(ui): make What's Next feed scrollable beyond 5 items#354
Conversation
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 2 issue(s).
frontend/src/components/AttentionFeed.tsx
Clean, well-scoped change that correctly moves the display cap from hook to component. No bugs or regressions — just a minor stale-state edge case and missing component tests for the new toggle.
- 🔵 missing_tests: No component-level tests for the new show-more/show-less toggle behavior. The project has component tests for other components (ToolPill, ToolGroup, CodeBlock, etc.). Consider adding tests that verify: (1) only 5 items render by default, (2) clicking 'Show all' renders all items, (3) the button doesn't appear when there are <= 5 items, (4) the button label shows the correct total count.
[fixable] - 🔵 bugs (L54): If the user clicks 'Show all' and items subsequently shrink below DEFAULT_VISIBLE_COUNT (e.g., a todo is completed),
showAllstays true buthasMorebecomes false, hiding the button. The user can never click 'Show less' — they're stuck with showAll=true, though it's functionally harmless since slice(0, 5) on fewer items returns all of them anyway. Not a real bug, but the state is stale.[fixable]
|
|
||
| const hasUrgent = tier1Count > 0; | ||
| const [manualOpen, setManualOpen] = useState<boolean | null>(null); | ||
| const [showAll, setShowAll] = useState(false); |
There was a problem hiding this comment.
🔵 bugs: If the user clicks 'Show all' and items subsequently shrink below DEFAULT_VISIBLE_COUNT (e.g., a todo is completed), showAll stays true but hasMore becomes false, hiding the button. The user can never click 'Show less' — they're stuck with showAll=true, though it's functionally harmless since slice(0, 5) on fewer items returns all of them anyway. Not a real bug, but the state is stale. [fixable]
Remove the hard MAX_ITEMS=5 cap from useAttentionFeed hook so all attention items are returned. The component now shows 5 by default with a "Show all N" / "Show less" toggle button. The header badge shows the true tier-1 count regardless of collapsed state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ef97b96 to
553a2d3
Compare
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 2 issue(s).
frontend/src/components/AttentionFeed.tsx
Clean, focused change that correctly moves the item cap from the data layer to the UI layer. No bugs; minor gap in component-level test coverage for the new toggle.
- 🔵 missing_tests (L79): The new show-more/show-less toggle in the component has no component-level tests. The hook test verifies the cap was removed, but there's no test for the UI slicing logic (DEFAULT_VISIBLE_COUNT), the toggle state, or that the button text changes correctly. This is consistent with the existing lack of component tests for AttentionFeed, so it's not a regression — but worth noting for a feature that introduces interactive UI state.
[fixable] - 🔵 bugs (L54): When the section is collapsed (isOpen=false) and reopened, showAll retains its previous state. Similarly, if items shrink below 5 (e.g. SSE update removes items), showAll stays true but hasMore becomes false — the button correctly hides, and showAll=true with <=5 items is a no-op (slice returns all), so no actual bug. Just noting the state isn't reset.
| const t2Count = items.filter((i) => i.tier === 2).length; | ||
| if (t2Count > 0) summaryParts.push(`${t2Count} in focus`); | ||
|
|
||
| const visibleItems = showAll ? items : items.slice(0, DEFAULT_VISIBLE_COUNT); |
There was a problem hiding this comment.
🔵 missing_tests: The new show-more/show-less toggle in the component has no component-level tests. The hook test verifies the cap was removed, but there's no test for the UI slicing logic (DEFAULT_VISIBLE_COUNT), the toggle state, or that the button text changes correctly. This is consistent with the existing lack of component tests for AttentionFeed, so it's not a regression — but worth noting for a feature that introduces interactive UI state. [fixable]
|
|
||
| const hasUrgent = tier1Count > 0; | ||
| const [manualOpen, setManualOpen] = useState<boolean | null>(null); | ||
| const [showAll, setShowAll] = useState(false); |
There was a problem hiding this comment.
🔵 bugs: When the section is collapsed (isOpen=false) and reopened, showAll retains its previous state. Similarly, if items shrink below 5 (e.g. SSE update removes items), showAll stays true but hasMore becomes false — the button correctly hides, and showAll=true with <=5 items is a no-op (slice returns all), so no actual bug. Just noting the state isn't reset.
Summary
MAX_ITEMS=5cap fromuseAttentionFeedhook — all attention items are now returnedTest plan
useAttentionFeedtest: verifies all 10 items returned (was asserting 5-cap)🤖 Generated with Claude Code