Lists S2: Projects app Lists tab UI (#2140) - #2379
Conversation
Adds a Lists tab to the Projects workspace with a list rail showing all project lists and an entries panel for the selected list. Entry rows include a done checkbox, category chip, status pill (new=blue, seen=grey, actioned=green, discuss=amber), a quick-add input with Enter-to-submit, and an indicator to view the original text when it has been tidied. Follows the Store/Images design bar pattern with rounded borders and shell surface backgrounds. Adds vitest tests covering quick-add creation, status pill rendering, and done toggle behavior.
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
|
|
|
Warning Review limit reached
Next review available in: 55 seconds You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR adds project-list and entry API types and methods. It adds a Lists tab to the project workspace with list selection, CRUD actions, status updates, completion toggles, original-text display, responsive layouts, and Vitest coverage. ChangesProject Lists
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🟡 Moderate · up to The Lists tab can show or mutate entries from the wrong project list during overlapping requests, and its mobile layout may make the entry panel unusable; these current-head correctness and usability risks should be addressed or explicitly accepted before merging. Sequence Diagram(s)sequenceDiagram
participant User
participant ProjectWorkspace
participant ProjectLists
participant projectsApi
User->>ProjectWorkspace: Select Lists tab
ProjectWorkspace->>ProjectLists: Render project lists
ProjectLists->>projectsApi: Load lists and entries
projectsApi-->>ProjectLists: Return list data
User->>ProjectLists: Add or update an entry
ProjectLists->>projectsApi: Submit entry mutation
projectsApi-->>ProjectLists: Return updated data
ProjectLists->>projectsApi: Refresh entries
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| .then((ls) => { | ||
| if (!cancelled) { | ||
| setLists(ls); | ||
| if (!selectedListId && ls.length > 0) setSelectedListId(ls[0]!.id); |
There was a problem hiding this comment.
WARNING: selectedListId is not reset when project.id changes
When the user navigates to a different project, the selectedListId state persists. If the new project has a list with the same ID as the previously selected list, the entries panel will display entries from the wrong list. If no matching list exists, the component shows an error. This state should be reset when the project changes.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| const createList = async () => { | ||
| const title = prompt("New list name"); | ||
| if (!title) return; |
There was a problem hiding this comment.
WARNING: createList accepts whitespace-only titles
if (!title) return; does not catch inputs like " ". After title.trim(), the resulting empty string is passed to the API, potentially creating a list with an empty title. Use if (!title.trim()) return; instead.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| role="button" | ||
| aria-label={`View original text for ${entry.text}`} | ||
| onKeyDown={(e) => { if (e.key === "Enter" || e.key === " ") { e.preventDefault(); alert(entry.original_text); } }} | ||
| onClick={() => alert(entry.original_text)} |
There was a problem hiding this comment.
SUGGESTION: Replace alert() with a non-blocking UI pattern
alert() blocks the main thread and is an intrusive UX pattern. The title attribute already surfaces original_text on hover; consider a tooltip, popover, or inline expansion instead.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous Review Summaries (2 snapshots, latest commit 2bd3d2d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 2bd3d2d)Status: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 0b0cf82)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (5 files)
Reviewed by step-3.7-flash · Input: 97K · Output: 19.3K · Cached: 349.4K |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@desktop/src/apps/ProjectsApp/ProjectLists.tsx`:
- Around line 31-42: Update refreshEntries to track the selected-list request
and ignore responses or errors that no longer belong to the current
selectedListId, preventing list A results from replacing list B entries. Ensure
both setEntries and setError are guarded by the active request/list identity
while preserving the existing empty-list behavior.
- Line 144: Update the container div in ProjectLists so its flex direction
changes to a vertical column below the 768px breakpoint, while preserving the
existing horizontal row layout on larger screens. Ensure the listsRail and
entries panel stack with usable width on mobile.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 32450318-f3c4-4f51-8d11-fcb46bdb0d7a
📒 Files selected for processing (5)
desktop/src/apps/ProjectsApp/ProjectLists.tsxdesktop/src/apps/ProjectsApp/ProjectWorkspace.tsxdesktop/src/apps/ProjectsApp/ProjectsApp.module.cssdesktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsxdesktop/src/lib/projects.ts
|
|
||
| return ( | ||
| <div className="flex flex-col h-full min-h-0"> | ||
| <div className="flex flex-1 min-h-0 gap-3"> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stack the list panels on mobile screens.
Below 768px, .listsRail is width: 100% and flex: none, but this container remains a horizontal flex row. The rail consumes the available row width, so the entries panel has no usable width.
Proposed fix
- <div className="flex flex-1 min-h-0 gap-3">
+ <div className="flex flex-1 min-h-0 flex-col gap-3 md:flex-row">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="flex flex-1 min-h-0 gap-3"> | |
| <div className="flex flex-1 min-h-0 flex-col gap-3 md:flex-row"> |
🤖 Prompt for 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.
In `@desktop/src/apps/ProjectsApp/ProjectLists.tsx` at line 144, Update the
container div in ProjectLists so its flex direction changes to a vertical column
below the 768px breakpoint, while preserving the existing horizontal row layout
on larger screens. Ensure the listsRail and entries panel stack with usable
width on mobile.
|
nemotron-super review VERDICT: Blocking issues found.
Automated first-pass review by the nemotron-super lane. The lead still reviews before merge. |
refreshLists() fetched the lists and returned them without calling setLists, and the mount effect was the only caller that stored the result. createList and deleteList each awaited refreshLists() and discarded it, so a created list never appeared in the rail and a deleted one never left it. The entry paths were unaffected because refreshEntries already set its own state, which is why the existing tests all passed: they cover entries, not lists. refreshLists now sets state and returns the lists, matching refreshEntries. Two tests added for the paths that had none, both proven to fail against 0b0cf82 first ("Unable to find an element with the text: New list"). The fetch mock is now stateful for lists as it already was for entries, because a fixed lists response cannot tell a refreshed rail from a stale one. Docs-Reviewed: no desktop app was added or removed. ProjectLists.tsx is a component inside the existing Projects app, so README's app list and count are unchanged. The doc-gate 'apps' rule matches desktop/src/apps/*/** and so fires on any file added under any existing app; that rule defect is carded separately and is not fixed here.
|
[LEAD REVIEW] Lead-completed rather than bounced: this lane exits after opening its PR, so a demand posted here would reach nobody. ONE REAL BUG, and the passing tests are why it survived.
Fixed in 2bd3d2d by making refreshLists set state and return, matching refreshEntries. RED FIRST, against this PR's own head 0b0cf82, with the final test file: and with the fix in place: tsc --noEmit rc 0. The fetch mock is now STATEFUL for lists, as it already was for entries. It returned a fixed one-list array for every GET, which means it could not have distinguished a refreshed rail from a stale one no matter what was asserted. A mock that ignores writes cannot test a refresh. TWO THINGS I DID NOT CHANGE, deliberately:
DOC-GATE: the changelog half was a real miss and is now fixed with a fragment. The Gate on green: |
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 `@desktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx`:
- Around line 43-48: Update the creation test around the POST mock and its “New
list” assertion to scope the lookup to the rail container, then assert the
created list item within that container. Use the rail’s existing accessible role
or test identifier rather than searching the whole document, so the assertion
verifies refreshLists() updates the rail state.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0eaf17ef-e38b-44e8-95de-ea2692df4631
📒 Files selected for processing (3)
changelog.d/2379-projects-lists-tab.mddesktop/src/apps/ProjectsApp/ProjectLists.tsxdesktop/src/apps/ProjectsApp/__tests__/ProjectLists.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- desktop/src/apps/ProjectsApp/ProjectLists.tsx
Two review findings, both reachable and both proven red first.
ProjectWorkspace renders <ProjectLists project={project} /> with no
key and is itself rendered unkeyed, so switching project reuses this
component instead of remounting it. selectedListId survived the switch
and the entries effect then fetched
/api/projects/<new>/lists/<old-list-id>/entries, showing the previous
project's entries against the new project.
Resetting in an effect is not enough: the entries effect re-runs in the
same commit because its deps include project.id, so it fires with the
stale id before any effect-based reset lands. The selection is now
dropped during render (React's adjust-state-on-prop-change pattern),
which runs before effects. The lists effect additionally keeps a
selection only if it belongs to the lists just fetched.
createList trimmed the title only after the empty check, so a
whitespace-only answer to the prompt reached the API as an empty title.
Docs-Reviewed: no user-visible behaviour beyond the Lists tab already
described in changelog.d/2379-projects-lists-tab.md, and no desktop app
was added or removed.
Both bots flagged this independently. refreshEntries wrote whatever came back, so two quick list clicks raced and the slower response landed last, showing one list's entries under another's heading. The response is now dropped unless its list is still the selected one, compared against a ref because the closure's own copy is stale by definition. Covered by a deterministic race test: lst-1's fetch is held open, the user selects lst-2, then lst-1's response is released. It fails without the guard with "STALE MILK" on screen. The created-list assertion is now scoped to the rail. It searched the whole document, and the entries header renders the selected list's title too, so it could have passed while the rail stayed stale. Docs-Reviewed: behaviour is already covered by changelog.d/2379-projects-lists-tab.md and no desktop app was added or removed.
|
[LEAD] Bot round adjudicated at head 95465c0. Kilo re-reviewed at 2bd3d2d (5 issues, its own summary marks that one authoritative and keeps the 0b0cf82 one for context); CodeRabbit posted at 2bd3d2d. THREE ACCEPTED, TWO DEFERRED WITH A CARD, ONE BOT OUTPUT REJECTED AS UNUSABLE. ACCEPTED 1 - kilo WARNING, ACCEPTED 2 - kilo WARNING, whitespace-only titles. ACCEPTED 3 - kilo WARNING and CodeRabbit Major, both independently: stale entries response. ACCEPTED 4 - CodeRabbit Minor, on MY test: DEFERRED, carded not skipped - mobile stacking (CodeRabbit Major, kilo SUGGESTION): below 768px DEFERRED - REJECTED AS UNUSABLE - the nemotron-super lane's comment. It says "VERDICT: Blocking issues found", lists seventeen line numbers at a fixed 15-line stride (82, 104, 119, 134 ... 314), and then ends with "No blocking issues found". A verdict contradicted by its own list, with line numbers that are an arithmetic sequence rather than findings, is not a review artifact. Recorded against that lane's known over-reporting; nothing in it was actionable. Red evidence for this round, all against this PR's own earlier heads with the final tests: Green now: 10/10, tsc --noEmit rc 0. |
|
[LEAD] The two deferred UI items are carded as tsk-7wwcdg (mobile stacking + replacing prompt/confirm/alert with the app's own dialogs), with a grep-based acceptance check and red-first demanded. Carded before citing the id, so this is the real one. |
|
@coderabbitai full review Requesting a fresh artifact rather than reusing the round at 2bd3d2d: two commits since then changed PRODUCTION code, not just tests. f8cf50e drops the previous project's selection during render (the effect-based reset fired too late, so the stale entries fetch still went out) and trims the list title before the empty check; 95465c0 ignores an entries response whose list is no longer selected, covered by a deterministic race test that fails without the guard. Current head is 95465c0. |
|
|
|
[LEAD] Merging on the documented rate-limit fallback, stated plainly rather than dressed up as a clean bot round. BOT POSTURE, honestly: the newest CodeRabbit artifact is for 2bd3d2d and kilo's newest is also 2bd3d2d. Current head is 95465c0. I requested a fresh full review, CodeRabbit acknowledged it would evaluate 95465c0, and no artifact arrived within an 11 minute wait. Kilo's check reads SUCCESS with an EMPTY description, which is the run completing, not a review artifact for this head. So neither bot has reviewed the last two commits and I am not going to pretend otherwise. WHAT IS ACTUALLY UNREVIEWED BY A BOT, and why I judge it safe to merge:
Two deferred items are carded as tsk-7wwcdg (mobile stacking, native prompt/confirm/alert). The nemotron-super comment on this PR was rejected as structurally malformed, with evidence recorded on tsk-x6fzgf. |
1 participant
CARD TITLE (intent, not commit subject): Lists S2: Projects app Lists tab UI (#2140)
Autonomous build of board card tsk-zitqoj.
Adds a Lists tab to the Projects workspace with a list rail showing all
project lists and an entries panel for the selected list. Entry rows
include a done checkbox, category chip, status pill (new=blue, seen=grey,
actioned=green, discuss=amber), a quick-add input with Enter-to-submit,
and an indicator to view the original text when it has been tidied.
Follows the Store/Images design bar pattern with rounded borders and
shell surface backgrounds.
Adds vitest tests covering quick-add creation, status pill rendering,
and done toggle behavior.
Files:
desktop/src/apps/ProjectsApp/ProjectLists.tsx | 314 +++++++++++++++++++++
desktop/src/apps/ProjectsApp/ProjectWorkspace.tsx | 8 +-
.../src/apps/ProjectsApp/ProjectsApp.module.css | 34 +++
.../ProjectsApp/tests/ProjectLists.test.tsx | 121 ++++++++
desktop/src/lib/projects.ts | 68 +++++
5 files changed, 542 insertions(+), 3 deletions(-)
Summary by CodeRabbit