Repository navigation
refactor(composer): share the composer textarea between AI chat and Group Chat - #1952
reachrazamair wants to merge 4 commits into
Conversation
…roup Chat Step 6 of the Group Chat composer parity plan. New src/renderer/components/ChatComposer/ComposerTextarea.tsx is the textarea with its live @mention chips: MentionChipOverlay and its select / scroll / blur / change routing, the shared typography, the 176px cap, and the two geometries in use (rows 2 for AI chat, rows 1 for a room), each with the overlay padding that matches its padding classes. It knows nothing about sessions, tabs or rooms and reads no store. The surface passes the value, placeholder, spell check, knownMentionNames and its handlers; prefix and fontFamily carry the AI composer's $ and command-mode fixed-pitch font. InputTextarea is now the AI adapter over it. It keeps what only AI chat has: the roster read from sessionStore, still gated on the draft holding an @ and excluding this agent, the mode-aware placeholder, and the $ prefix and fixed-pitch font. It takes agentId, agentName and toolType instead of the Session, so this agent's own streaming flushes, which replace the Session object, no longer re-render it. New InputTextarea.renders.test.tsx pins the budget: one render per keystroke, none on another agent's flush while the draft has no @ (one with an @, for the roster), none on this agent's own flush. The first three passed against the old InputTextarea too; the last is the improvement. ComposerTextarea adds one component call under the adapter, never a render the adapter did not already do. GroupChatInput renders ComposerTextarea with rows 1 and its existing agents-only roster. Removed from it: the inline textarea, its useMentionChipOverlay call, and OVERLAY_PADDING. Removed from InputTextarea: the inline textarea and overlay, OVERLAY_PADDING, and the Session prop. No capability flag is added: nothing here reads the picker's categories, so fileMentions waits for the change that does. Autosize stays with each surface (AI chat sizes on its own keystroke path). Group Chat behaviour changes: none visible. The textarea now sets caretColor to the text color (the caret already took that color by default) and its max height reads 176px instead of 11rem (the same size). Tests: InputTextarea.test.tsx passes agentId, agentName and toolType instead of a Session (the props it now takes); no assertion changed. InputArea and GroupChatInput tests are unchanged.
…oom's box Step 6 left the room's old `relative flex items-start` wrapper around ComposerTextarea, which now renders that same row as its own root. The component's root became a flex item of the wrapper row without flex-1, so the room's textarea shrank to its intrinsic width: about 200px inside a 916px composer, with the placeholder wrapping (verification V1). AI chat mounts the component directly in its bordered column box and was not affected. The wrapper is removed, so the room's composer markup is step 5's again: the bordered column box, then the row, then the chip overlay and the textarea. A jsdom dump of the room's composer box on step 5 and here differs only on the textarea itself: class order, an explicit caret-color (the text color, which the caret already took), a 176px max height in place of 11rem, and no inline font-family: inherit (Tailwind's preflight already sets it on every textarea). The AI composer's box is unchanged from step 5 apart from class order. The component's doc comment now says to mount it as a direct child of the column box. A guard test on each surface asserts the textarea's root is a direct child of the bordered column box. jsdom does no layout, so it checks the structure the full width depends on, not the width itself. Group Chat behaviour changes (step 6 against step 5): - The textarea's max height is 176px instead of 11rem. The same at the default 16px root font size; at a smaller root size the CSS cap now matches the 176px the room's autosize already clamped to, as AI chat always has.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review: 2
Please check the settings in the CodeRabbit UI or the
 |
 [Medium impact] The PR changes text-entry behavior in the AI chat and Group Chat composers. The PR appears safe to merge, with a non-blocking correction needed in the streaming render test and its claims. Findings
SummaryExtracts
reachrazamair explicitly identifies the room's Reviews (1) · Last reviewed commit: "fix(group-chat): mount the shared compos..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it("does not render when this agent's own Session object is replaced", () => { | ||
| const composer = renderCounted('ask @Other'); | ||
| composer.ownFlush('ask @Other'); | ||
| expect(composer.renders()).toBe(0); |
There was a problem hiding this comment.
ownFlush only rerenders with unchanged scalar props; it never replaces the agent in sessionStore. This checks memo, not a streaming flush. With the tested draft (ask @Other), InputTextarea reads state.sessions, so a real flush replaces that array and still triggers a render.
Add a store update to cover a real flush, and keep the parent-only render check separate. Update the matching component comments and guide claims to describe the guarantee actually covered.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Progress now covers step 7 (refactor/composer-step7, 252d324, no PR yet), which completes rows 50 and 52, notes PR #1952 for step 6, and notes step 8 is not started. Records today's origin/rc catch-up across steps 3-6 (ee4f997, 5d6674a, a59a296 resolving the CLI-UI-PARITY.md conflict, and 6b6a429) and the step 3 fix for P5 (af82861, merged up in 7a06492, bee2d68 and 6fd3979). Moves P5 to "Findings fixed" and adds the step 7 room behaviour entries for the phone fold and the quit-when-idle pill.
Summary
Step 6 of the Group Chat composer parity plan (
Plans/group-chat-composer-parity.md). Stacked on #1945; review that first.New
src/renderer/components/ChatComposer/ComposerTextarea.tsxis the composer textarea with its live@mentionchips, and both composers render it:MentionChipOverlayand the select / scroll / blur / change routing that keeps chips on their tokens, the shared typography, the 176pxKEYSTROKE_TEXTAREA_MAX_HEIGHTcap, and the two geometries in use.rows={2}is AI chat, with the 4px bottom padding that leaves 8 whole rows under the cap.rows={1}is a room. Each comes with the overlay padding that matches its classes.knownMentionNamesand its handlers. It knows nothing about sessions, tabs or rooms, and it reads no store.$prefix and command mode's fixed-pitch font arrive throughprefixandfontFamily.InputArea/components/InputTextarea.tsxis now the AI adapter over it:sessionStore, still gated on the draft holding an@and excluding this agent. It also keeps the mode-aware placeholder and the$prefix and font.agentId,agentNameandtoolTypeinstead of theSession.GroupChatInputrendersComposerTextareawithrows={1}and its existing agents-only roster, as a direct child of its bordered column box, the same wayInputAreamounts it.Per-keystroke performance
The plan's open question was whether the AI composer's store isolation (CLAUDE-PERFORMANCE.md) survives behind a shared component. It does. The gated roster subscription stays in the adapter, and the shared component reads no store.
New
InputTextarea.renders.test.tsxcounts the adapter's renders (at a pass-through wrapper aroundComposerTextarea):@@(roster)Sessionobject replaced)The first three rows passed against the old
InputTextareatoo, before the refactor. The last row is new: the old adapter took the wholeSession, so its memo broke on every one of this agent's flushes.ComposerTextareaadds one component call under the adapter per render, never a render the adapter did not already do.Room behaviour changes
176px(KEYSTROKE_TEXTAREA_MAX_HEIGHT) instead of11rem. These are the same at the default 16px root font size. At a smaller root size (the root size is a user setting) the cap is now a little taller, matching the 176px the room's autosize already clamped to, as AI chat always has.caret-colorequal to the text color, which the caret already took;font-family: inherit, which Tailwind's preflight already sets on every textarea.The first commit left the room's old
relative flex items-startwrapper aroundComposerTextarea, whose root is that same row. The component's root then shrank to the textarea's intrinsic width (about 200px in a 916px composer, measured against the built CSS in headless Chromium). The second commit removes the wrapper, notes the mounting rule in the component's doc comment, and adds a guard test on each surface.Removed
GroupChatInput: the inline textarea and the row around it (now the component's root), itsuseMentionChipOverlaycall,OVERLAY_PADDING, and theMentionChipOverlayimport.InputTextarea: the inline textarea and overlay,OVERLAY_PADDING, and theSessionprop.Not in this step
fileMentionswaits for the change that does (see "Step 4 deviation" in the plan).useInputAreaAutosize); the room keepsuseAutosizeTextarea.Test plan
npm run lintnpm run lint:eslintnpm run docs:verify(1014 asserted paths, 344 index entries, 0 findings)npx vitest runon the stack's test set (40 files, 941 tests):composerKeyDown.test.ts,GroupChatInput.test.tsx,GroupChatInput.toolbar.test.tsx,useInputKeyDown.test.ts,useInputKeyDown.focus.test.tsx,useMainKeyboardHandler.test.ts,useInputHandlers.test.ts,InputArea.test.tsx,components/InputArea/,components/ChatComposer/,ui/PhoneBottomSheet.test.tsx,FilePreview/FilePreviewHeaderPhone.test.tsx,useVoiceInput.test.tsComposerTextarea.test.tsx: placeholder, value, ref, roster-gated chips, chips off, overlay layering, both geometries, prefix, shared font override, read-only focus, handlers, dragover.GroupChatInput.toolbar.test.tsxandInputArea.test.tsx: the textarea's root is a direct child of the bordered column box. jsdom does no layout, so they check the structure the full width depends on, not the width itself. The room's guard fails without the fix.InputTextarea.test.tsx: passesagentId/agentName/toolTypeinstead of aSession(the props it now takes); no assertion changed.InputAreaandGroupChatInputtests unchanged.