fix(app): stabilize session row left-side visuals (status slot + Pin/Filter icons)#149
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 55 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the sidebar icons to a 20x20 grid and refactors the SessionRow component by removing a conditional wrapper around the status indicators. Feedback focuses on maintaining visual consistency for the new SVG paths, specifically suggesting the addition of stroke-width and adjustments to stroke-linejoin to match the previous design's visual weight.
Astro-Han
left a comment
There was a problem hiding this comment.
Review Summary
Overall the changes align with the issue requirements, but there are some geometry and visual consistency concerns worth addressing.
P0: None
P1: 1 issue
- PinIcon geometry mismatch: The pin head is a pentagon (not hexagon as described), and needle origin is slightly misaligned with the bottom edge.
P2: 2 issues
- FilterIcon rendering size: size=14 is 70% of 20x20 grid, making it smaller than adjacent icons (16px/80%)
- Icon shape redesign scope: PinIcon changed from traditional pushpin to geometric shape without explicit mention in issue
P3: 1 note
- Empty Switch fallback: When no status matches, Switch renders empty - this is intentional for baseline stability
See inline comments for details.
Closes #143.
Summary
fix) removes the outer<Show>wrapper around the sidebar session-row status slot insidebar-items.tsx, so thesize-6container always reserves 24px of width. The inner<Switch>(running / permission / error / unseen) is unchanged. Session title baseline no longer shifts when status indicators toggle.refactor) redrawsFilterIconandPinIconinpawwork-sidebar.tsxon the 20x20 grid.FilterIconusesstroke-linecap="square"to match upstream straight-line glyphs (close,arrow-right,menu).PinIconuses a hexagonal head polygon + diagonal needle, default stroke width, default size 16. Needle origin(8, 11)sits on the head's bottom-left edgey = x + 3for clean attachment.Follow-up (out of scope)
During live review we noticed that the Pin button (rendered in
SessionItemleadingSlot) and the status slot (insideSessionRow) are two separate reserved slots. The original intent implied by the issue reporter was for running state to appear in the pin button position — i.e. one unified slot with priorityrunning > permission > error > unseen > pin > empty. This PR ships the #143 scope as written (two stable slots, matching the issue body's "Not in scope: Pin slot" note), and defers the merge refactor to #150. Merge is medium-risk: changes pin button a11y contract (pin must remain keyboard-reachable when pinned + running), requires passing leadingSlot intoSessionRow, and shifts the top-level title baseline left by 28px.Test plan
bun run typecheckpassesbun dev:desktop: