Repository navigation
feat(conversation): add ConversationThread - #126
pratikbuilds wants to merge 2 commits into
Conversation
|
Triage: Unsure Waiting on:
Stacked on #125: only this pull request's own changes were checked. Notes:
Waiting on a maintainer to approve: assign. Changed since the last verdict (Pass at |
dc9a67a to
fb3faf7
Compare
e312fe9 to
4bc9068
Compare
TheGreatAxios
left a comment
There was a problem hiding this comment.
Automated multi-agent review: independent review + second-agent validation; only confirmed items listed. Clean composition PR — nothing blocking.
- minor
src/ui/conversation-thread.tsx:44-49(vssrc/ui/conversation.tsx:601-621) — thesentIddoc says a foreign id "moves nothing", which holds undersettlebut notstick:sent(id)glides to the end before any registry lookup, so any foreign-id change glides the thread to its end. Qualify the doc ("undersettle…") or guard the stick branch on registry membership. - file-for-later (plan follow-up)
src/ui/conversation-thread.tsx:96-113— no windowing slot:ConversationEarliermust sit first insideConversationContent, but the thread owns the log's children and renderschildrenoutside the log as the input slot; the divider count is hardwired toreplies.length. An optionalearlier?: ReactNodeslot plus seeding the count fromparent.replies.countwould close it. - nits —
labels.logis documented as the name "when noaria-labelledbyis given" but anaria-labelalso overrides it (:18-20, 99-105);data-thread-parent/data-thread-dividerare set but undocumented as extension hooks; theChatWithThreadsstory keeps a stale replies count on the main conversation after a send (updatemessage.replies.countin the send handler or note it in the story).
4bc9068 to
dba5b9e
Compare
fb3faf7 to
9660d62
Compare
dba5b9e to
b402e73
Compare
40def38 to
bc0fa3e
Compare
b402e73 to
5f8dcec
Compare
|
Thanks for the review. Here's what happened to each point. Fixed1. The
2. Nits
Not changed, and why
Commit: |
bc0fa3e to
d86517e
Compare
5f8dcec to
c8ba54f
Compare
TheGreatAxios
left a comment
There was a problem hiding this comment.
Review · Approve
Adds ConversationThread. Scroll policy and sent id pass through, the log is named, and history and the map stay with the host.
Findings
No findings.
ConversationThread renders a reply conversation headed by its parent message, with a divider counting the replies, built from the same parts as the main conversation through a render prop. Each thread has its own Conversation, so a reply settles at the top of its own log, and in a keep-mounted panel tab a thread keeps its place and the host's draft while the reader switches tabs. The thread's PromptInput sits below the log, and aria-label or aria-labelledby names the log.
The sentId doc said an id from another conversation moves nothing. Say instead that an id which is not one of the thread's items is ignored, matching the engine guard the Conversation change adds. Also note that an aria-label, like aria-labelledby, overrides labels.log, and document data-thread-parent and data-thread-divider as styling hooks. In the ChatWithThreads story the thread's replies lived in the tab, so the count under the parent never moved after a send. Keep them in the chat and derive the count from them, as the full-chat story does. Since the replies now outlive the tab, reply ids come from the thread's earlier sends, and stopping or closing the tab mid-answer marks the answer sent rather than leaving it streaming.
d86517e to
c9738df
Compare
c8ba54f to
145cfc4
Compare
ConversationThread renders a reply conversation headed by its parent message, with a divider counting the replies, built from the same parts as the main conversation through a render prop. Each thread has its own Conversation, so a reply settles at the top of its own log, and in a keep-mounted panel tab a thread keeps its place and the host's draft while the reader switches tabs. The thread's PromptInput sits below the log, and aria-label or aria-labelledby names the log.
Stacked on #125: review only this commit. Part of #94.
Notes for review
Verification
bun run typecheckandbun run linton its own (the stack is checked commit by commit).bun run check,bun run build,bun run dep-guard,bun run contrast-testandbun run test:e2e(Playwright specs and the Storybook axe sweep in light and dark) pass.Closes #108