fix(members): roster row idiom, recency sort, and a single-action header - #6616
Conversation
Design Review (Fable 5) — ✅ PASSDesign-level review of Diff and PR description reviewed: backend Design-Verdict: PASS Additive API field, shared resize/sort/frameless primitives reused, and each removal argued from an invariant — sound, proportionate follow-up to #6210. [DESIGN-REVIEWED] 2d7f629 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsFINDING -- src/kiro_crew/zip_vet.py:119 -- False positive or not applicable? A repository writer can comment: |
UX Review (Fable 5) — ✅ PASSUX-level review of UX-Verdict: PASS Removing the invariant pin chip, adopting the chat-sidebar row idiom, and recency sort all reduce noise; screenshots confirm the header now teaches the same gesture as chat. Suggestions
[UX-REVIEWED] 2d7f629 |
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of All claims verified. I have everything needed to write the review. First-Principles-Verdict: CONCERNS The resizable roster's only stated support is "mirroring the chat sidebar" — symmetry, not a named harm — riding along in a PR framed as fixes. What this change shipsIntent: apply a PM review's post-merge corrections to the just-shipped Crew Members page — a FIX (per title and "Post-merge UX fixes"), with two capabilities added alongside.
WatchItem 7 is the one addition with no named harm: the description's whole case is "mirroring the chat sidebar," which is symmetry with a neighbouring feature, not a person who couldn't use the 264px roster. It does reuse the shared primitive ( SubtractionsDefer the roster resize — drop [FIRST-PRINCIPLES-REVIEWED] 2d7f629 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
|
GPT disposition — the blocker from the 2f159b7 round is fixed in 53bcf27: width persistence moved OUT of the state updater into the pointerup handler behind try/catch, tracked via a ref (the updater was also impure — double-invoked under StrictMode). A quota-exhausted or privacy-mode storage now costs only the remembered preference, never the page: the in-memory width stands, and the initial read falls back to the default width under the same guard. |
53bcf27 to
3446a48
Compare
|
/ai-review override gpt 3446a48: PanelRightSolid is the repo's deliberate lucide replacement (see website/src/components/icons/panels.tsx header docs) already used by ChatPage:7092 for the identical panel toggle; matching it is the point of this change. |
Human judgment recorded@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
Three PM findings on the shipped Crew Members page, plus a layout alignment with the chat sidebar: - Roster rows now use ChatSidebar's rounded-row idiom (rounded-md, accent-subtle selection, hover:bg-bg-hover) inside a p-2 lane with the scrollbar hidden, so the two conversation lists read as one family. - Members sort by most recent DM activity (new last_active_ts on the roster response, from the transcript's mtime in one batched thread hop); never-talked members trail alphabetically. - The pin chip is gone: the pin is an invariant of every member thread, so announcing it taught a term for a state that cannot be otherwise. The header keeps exactly one action — an icon button toggling the detail drawer — and Edit lives inside the drawer as the secondary action it is. - The roster width is user-adjustable on md+ (drag handle, persisted), mirroring the chat sidebar.
3446a48 to
2d7f629
Compare
|
Dispositions for the 3446a48 round — all addressed in 2d7f629:
|
Follow-up PM round on the Crew Members page (the prior batch landed in #6616): - Roster rows drop the Idle/Working label for the thread's last-message preview — the same sub-line a session row carries. Presence already rides the avatar dot, so the textual label said nothing the dot did not. The preview reuses the sessions list's last_message_preview + redaction path, so a member row can never leak what a session row would not. - A SearchInput (the sessions sidebar idiom) filters the roster by name; new search_members key across 12 locales, en-XA regenerated. - The thread header drops its status sub-line (name only, like the chat page header), and the drawer's local close button survives only below md — on desktop the header's panel toggle is the one close gesture, matching the chat page's side panel. Backend: the roster response gains last_message, read in the same one-thread-hop batch as last_active_ts. Tests: backend pins last_message ('' when no transcript); frontend pins the preview sub-line, the absence of status labels, and the name filter. Capture assertions updated and all frames re-verified.
Follow-up PM round on the Crew Members page (the prior batch landed in #6616): - Roster rows drop the Idle/Working label for the thread's last-message preview — the same sub-line a session row carries. Presence already rides the avatar dot, so the textual label said nothing the dot did not. The preview reuses the sessions list's last_message_preview + redaction path, so a member row can never leak what a session row would not. - A SearchInput (the sessions sidebar idiom) filters the roster by name; new search_members key across 12 locales, en-XA regenerated. - The thread header drops its status sub-line (name only, like the chat page header), and the drawer's local close button survives only below md — on desktop the header's panel toggle is the one close gesture, matching the chat page's side panel. Backend: the roster response gains last_message, read in the same one-thread-hop batch as last_active_ts. Tests: backend pins last_message ('' when no transcript); frontend pins the preview sub-line, the absence of status labels, and the name filter. Capture assertions updated and all frames re-verified.
Follow-up PM round on the Crew Members page (the prior batch landed in #6616): - Roster rows drop the Idle/Working label for the thread's last-message preview — the same sub-line a session row carries. Presence already rides the avatar dot, so the textual label said nothing the dot did not. The preview reuses the sessions list's last_message_preview + redaction path, so a member row can never leak what a session row would not. - A SearchInput (the sessions sidebar idiom) filters the roster by name; new search_members key across 12 locales, en-XA regenerated. - The thread header drops its status sub-line (name only, like the chat page header), and the drawer's local close button survives only below md — on desktop the header's panel toggle is the one close gesture, matching the chat page's side panel. Backend: the roster response gains last_message, read in the same one-thread-hop batch as last_active_ts. Tests: backend pins last_message ('' when no transcript); frontend pins the preview sub-line, the absence of status labels, and the name filter. Capture assertions updated and all frames re-verified.
Follow-up PM round on the Crew Members page (the prior batch landed in #6616): - Roster rows drop the Idle/Working label for the thread's last-message preview — the same sub-line a session row carries. Presence already rides the avatar dot, so the textual label said nothing the dot did not. The preview reuses the sessions list's last_message_preview + redaction path, so a member row can never leak what a session row would not. - A SearchInput (the sessions sidebar idiom) filters the roster by name; new search_members key across 12 locales, en-XA regenerated. - The thread header drops its status sub-line (name only, like the chat page header), and the drawer's local close button survives only below md — on desktop the header's panel toggle is the one close gesture, matching the chat page's side panel. Backend: the roster response gains last_message, read in the same one-thread-hop batch as last_active_ts. Tests: backend pins last_message ('' when no transcript); frontend pins the preview sub-line, the absence of status labels, and the name filter. Capture assertions updated and all frames re-verified.
Summary
Post-merge UX fixes to the Crew Members page (#6210), from PM review of the shipped build:
last_active_ts(the DM transcript's mtime, read in one batched thread hop); the list sorts recent-first, never-talked members trail alphabetically.Screenshots
More frames
Update (PM round 2)
framelessmode for embedded hosts: the pane's own title bar and card chrome (border, rounded corners) are dropped when the host page already renders the identity header. Split-view panes keep the chrome unchanged.Testing
test_roster_reports_last_activity_from_the_dm_transcript; black/isort/flake8/mypy cleanNotes
pinned_toi18n key removed across 12 locales + en-XA regenerated.