refactor(ui): one composer core — overlay/ChatSurface/ChatView consume a shared composer, features ported not dropped (#12188 Phase 3 sub-item B) - #13243
Conversation
… one paste, context-or-local draft (#12188 Phase 3) The headless logic every chat composer surface shares: useComposerKeydown (locked guard → IME-commit Enter passthrough (#9148) → slash-menu key interception → Enter-send/Shift+Enter → Escape hook), useComposerPaste (classifyComposerPaste routing: files attach, oversized text becomes a text-attachment chip, small text passes through), and useChatComposerOrLocal (the shared ChatComposerContext draft slot when a provider is mounted, live local state when none is — stories/fixtures). 14 jsdom contract tests pin the ordering matrix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ChatView sheds its glue (#12188 Phase 3) ChatComposer drops the onKeyDown/onPaste passthrough props and builds both from the shared core internally (onSend + optional pasteAttachments intake). This puts the IME-commit-Enter guard (#9148) on the ChatView/game-modal/ inbox-reply surfaces for the first time — previously a CJK candidate commit sent the message there — and deletes ChatView's copy-pasted keydown + paste classification glue (handleKeyDown, handleComposerPaste, handleReplyKeyDown). New component-level tests pin Enter-send, Shift+Enter, the IME guard, and both paste routes on the real composer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…xt draft, core keydown/paste, private persistence deleted (#12188 Phase 3) The overlay's draft + pending attachments are now the shared ChatComposerContext slot (useChatComposerOrLocal): AppContext's one persistence instance and useChatCallbacks.handleSelectConversation's flush/restore handoff — which the overlay's conversation swipe already routes through — replace the overlay's private useState draft, its second useChatComposerDraftPersistence instance, and its duplicate switch-time handoff layout-effect (two debounced writers on the same localStorage keys, now one). The inline 80-line keydown (IME guard #9148 + slash interception + Enter-send + Escape-collapse) and the inline paste classification are the shared composer-core hooks; the slash menu plugs in via the core's ComposerSlashKeydown binding. Dictation appends through a live draft ref (the context setter has no functional-update form). Overlay draft tests are rewritten as composition tests: an AppComposerHarness reproduces the exact app seam (real context state + the AppContext persistence hook + the handleSelectConversation handoff) around the real overlay, proving restore-on-mount, A→B handoff flush/ repaint, send-clears-draft, and the new one-draft-across-surfaces contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…E-safe Enter, push-to-talk mic hold (#12188 Phase 3) The glass mini-chat's private useState draft becomes the shared ChatComposerContext slot (context-or-local), its hand-rolled Enter handler becomes the core keydown — closing the IME hole where a CJK candidate commit sent the message — and its mic gains the shared usePushToTalk hold machine: hold dictates (new optional onDictateStart/onDictateEnd props, wired by ShellFoundationMount to controller.startRecording("dictate") / stopRecording with a dictation sink that appends into the shared draft), tap still toggles recording, and the trailing click after a hold release is suppressed. GlassIconButton passes onPointerLeave through for the hold machine's slide-off cancel. Six new jsdom tests pin the contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oser-core slash keydown coverage (#12188 Phase 3) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r states (#12188 Phase 3) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
… composer run (#12188 Phase 3) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Code-review note, not an approval: GitHub will not let this account approve its own PR. I reviewed the issue scope, package UI rules, evidence notes, and the main integration paths: I do not see a code-level blocker. The shape is sound: the IME/slash/Enter contract is centralized, paste routing is centralized, the overlay no longer keeps a second draft/persistence path, and the PTT hold machine has one lifecycle with click suppression. The risky regressions I checked — IME Enter, slash menu interception, image/text paste routing, dictation into draft vs submit-on-release, ChatSurface shared draft, and overlay send clearing persisted drafts — are covered by the new or updated tests in the patch. This is still a UI-shell change, so merge should wait for CI, especially |
|
I’m not merging this yet. The implementation/evidence is substantial, but this is app-affecting UI work across shared composer surfaces, and the repo rule is explicit: changes in The PR body says Please add one of:
Also note the PR is described as stacked on #13165. Before merge, please confirm the final diff is against current |
|
Reviewed (adversarial, #12188 Phase 3): extracts a headless composer core (useComposerKeydown/Paste/isImeComposingEnter) consumed by overlay/ChatView/ChatSurface. Traced every keydown branch of the overlay port against the deleted inline handler — behavior-faithful — and it fixes a real bug (ChatView/ChatSurface/inbox-reply lacked an IME-composing guard, so a CJK candidate-commit Enter sent the message; now guarded everywhere). Strong tests (composer-core + chat-composer + ChatSurface + rewritten overlay). Stacked base #13165 already merged so the diff is composer-only; 0 failing CI. Queuing auto-merge on green. |
…ome typography, tray transparency (from #13069 overhaul) (#13466) * feat(ui): home + chat look layer — clouds/shader background polish, home typography, tray transparency (from #13069 overhaul) Upstream the AESTHETIC layer of the feat/ui-overhaul branch that never landed. The overhaul's mechanics + token plumbing merged overnight (57 PRs), but the actual LOOK did not. This ports the missing visual carriers so stock develop looks like the polished build. WHY THE LOOK NEVER LANDED (root cause found + fixed): The overhaul put its ember token system (warm accent #ff6a1f, warm near-black, --surface-1/2/3, --scrim) into theme.css AND base.css, but the running app only imports base.css + tailwind-theme.css via styles.css — theme.css is loaded ONLY by the e2e fixture bundler. So on stock develop, every `bg-scrim` / `bg-card` (=surface-1) usage already shipped (NotificationCenter, drawer, etc.) resolved to an UNDEFINED token → transparent at runtime. This lands the token layer in base.css (the file the app actually imports) so those usages resolve for real, in both light and dark. AREAS (per Shadow's named items + the rest of the look diff): - backgrounds/ShaderBackground.tsx: "midnight ember" field — a deep warm field with a low banked-ember glow breathing up from the composer, replacing the flat rim-pulse wall. Adds DEFAULT_BACKGROUND_GLOW (accent) to ui-preferences. - backgrounds/ImageBackground.tsx: legibility scrim (bg-bg/50) under any wallpaper so content wins over a bright photo, in both themes. - shell/DefaultHomeWidgets.tsx: the home time+weather TYPOGRAPHY — a 6xl left-aligned editorial clock with a quiet date/greeting stack (accent greeting) and a right-aligned weather cluster. Tokenized micro-text (text-xs-tight), no arbitrary text-[11px]. - shell/HomeScreen.tsx: full-height home column — editorial header anchored top, widgets in a flex-1 breathing region, AOSP tiles settled at the bottom. - shell/ContinuousChatOverlay.tsx: chat tray TRANSPARENCY fix (#1 "too transparent" complaint) — the open panel is now a SOLID warm-dark surface (--card) with a warm hairline edge, and the backdrop is a solid --scrim, so the ember field / home widgets no longer bleed through the open thread. Plus the full white/N-alpha → token migration (text-muted-strong, bg-surface, border-border, bg-scrim). STYLING ONLY — no gesture/scroll logic (kept clean for #13439's swipe rail to merge in any order). Uses --card (defined both themes) not --surface-1 (dark-only) so the panel is opaque in light mode too. - widgets/WidgetHost.tsx: home widgets read as full-width rows (hero dashboard), not cramped two-up truncated cards. - shell/HomeLauncherSurface.tsx: touch-pan-y so the home↔launcher flick isn't handed to the browser's scroll/back gesture. - styles/base.css: the runtime keystone — warm accent #ff6a1f (was #ff8a24), warm brown-black #140c07 (no pure #000), warm off-white, the soft radius ramp (6/8/11/14/18/22/28 vs the collapsed 3px), ember --surface-1/2/3, warm muted text, and --scrim (both themes). - styles/theme.css + tailwind-theme.css: the e2e/brand override layer kept in sync with base.css (adds --color-scrim mapping). - shell/ChatSurface.tsx: removed a backdrop-blur-sm that landed today via #13243 and was already failing the no-backdrop-blur battery gate on develop (flat system: solid bg-card + border, no blur/shadow). Unblocks the UI test lane. EXCLUDED as personal (Shadow-specific, not upstreamed): - bg-sunset.jpg wallpaper + the DEFAULT_BACKGROUND_CONFIG mode:"image" default. The "clouds background" Shadow named IS this personal photo wallpaper — the RENDERING support (ImageBackground + its scrim) ships, but the default stays the shader. BuildBadge / sol-build-info: untouched. RELATIONSHIP TO IN-FLIGHT PRs: - #13438 (ember-dark default bg): owns DEFAULT_BACKGROUND_COLOR + the App.tsx safe-area floor. This PR does NOT touch DEFAULT_BACKGROUND_COLOR, the config default, or the presets — only adds DEFAULT_BACKGROUND_GLOW (shader dep) and the base.css token values #13438 assumes. Merges cleanly in either order. - #13439 (swipe rail): owns the ContinuousChatOverlay gesture logic. This PR's overlay changes are STYLING-ONLY (no railX / usePullGesture / settle logic), so it merges clean regardless of order. Tests: targeted suites for every touched area pass (backgrounds, HomeScreen, DefaultHomeWidgets, HomeLauncherSurface, WidgetHost, ContinuousChatOverlay, ChatSurface, NotificationCenter token consumers) + the 4 anti-slop gates (backdrop-blur/focus-ring/widget-chrome/will-change) green + tsgo --noEmit clean over a real install + the home-screen e2e (Playwright) passes and its snapshot is regenerated. AppBackground.test updated for the shader's gradient-vs-flat paint + the new image scrim. One pre-existing develop failure (WidgetHost.home-launch "notifications widget when there is data") reproduces on stock develop and is unrelated to this change. Co-authored-by: wakesync <shadow@shad0w.xyz> * chore(ui): strip generated home artifact whitespace --------- Co-authored-by: wakesync <shadow@shad0w.xyz> Co-authored-by: Shaw <shawgotbags@gmail.com>
1 participant
Completes the composer unification — the piece #13165 deferred. The overlay, ChatView's
ChatComposer, andChatSurfacenow all consume one shared composer core; every feature was ported, not dropped, and each surface keeps its own chrome.The shared core (
packages/ui/src/chat/composer-core.ts+ friends)useComposerKeydownuseComposerPasteuseChatComposerOrLocal→ the sharedChatComposerContextslot (live local-state fallback for stories/fixtures)usePushToTalk(the #12345 core)ComposerSlashKeydownbindingWhat each surface sheds / gains
useStatedraft +pendingImages, its seconduseChatComposerDraftPersistenceinstance, its duplicate switch-time handoff layout-effect, the inline 80-line keydown, and the inline paste classifier. AppContext's one persistence instance +useChatCallbacks.handleSelectConversation's flush/restore (which the overlay's conversation swipe already routes through) now repaint the overlay directly, because it reads the context. This removes a real hazard: two debounced writers on the same localStorage draft keys.onKeyDown/onPastepassthrough props deleted (zero importers outside ChatView/tests/stories, verified); ChatComposer builds both from the core (onSend+ optionalpasteAttachmentsintake). Bug fixed: ChatView's own keydown had no IME guard — a CJK candidate-commit Enter sent the message on the desktop-window, game-modal, and inbox-reply composers. ChatView deleteshandleKeyDown,handleComposerPaste,handleReplyKeyDown.onDictateStart/onDictateEnd, wired byShellFoundationMounttocontroller.startRecording("dictate")/stopRecordingplus a dictation sink appending into the shared slot), tap still toggles recording, trailing click after a hold release suppressed.GlassIconButtonpassesonPointerLeavethrough for slide-off cancel.Chrome kept per-surface (deliberately — the duplication that died is the logic)
GlassIconButtonmic/vision/send.Zero
className/visual changes in the diff — this is logic-only, so the shipping look of all three composers is pixel-identical.No-regression proof for the regression-forbidden features (per surface)
composer-core.test.tsx(isComposing + keyCode 229, with and without slash menu) ·chat-composer.test.tsx"never sends on the Enter that commits an IME composition" (ChatView surface) ·ChatSurface.test.tsxsame ·ContinuousChatOverlay.test.tsxpre-existing IME test still green (overlay surface).usePushToTalk.test.tsx(machine) ·chat-composer.test.tsxhold-release-mid-draft test (ChatView surface) · newChatSurface.test.tsxhold-dictates/tap-toggles/click-suppression · overlay PTT dictation test still green (dictation lands in draft, no send).ContinuousChatOverlay.slash.test.tsx— all 12 tests green against the core-built keydown (Enter/Tab/arrows/Escape now flow throughuseComposerKeydown) · core-level interception matrix incomposer-core.test.tsx.AppComposerHarnessreproduces the exact app seam (real context state + the AppContext persistence hook + thehandleSelectConversationhandoff) around the real overlay: restore-on-mount, A(typed)→B(no draft) flush+clear with the 600 ms wrong-key-write probe, A→B(saved) restore, send-clears-draft, and the new one-draft-across-surfaces contract.Verification
bun run --cwd packages/ui test— 5938 passed / 7 skipped; 18 failures in 6 files (App.navigate-view-wiring,App.screen-background-fuzz,WalletKeysSection,LauncherSurface,no-backdrop-blur-gate,startup-phase-poll) — all pre-existing and untouched by this diff: root cause is theuseAuthStatustest-mock missinguseIsAuthenticatedafter feat(ui): post-login permission-priming modal (soft-ask) for voice/location/notifications #12331 (PermissionPrimingOverlay) merged into the base, plus known flakes; none reference composer code.bun run --cwd packages/ui test:chat-sheet-e2e— PASSED (58 screenshots +.webm: detents, typing→send, image attach, PTT dictation-into-draft, multiline, onboarding lock).bun run --cwd packages/ui test:chatux-gesture-e2e— PASSED (video).bun run --cwd packages/ui test:conversation-swipe-e2e— PASSED (video; exercises the rerouted draft handoff under real swipes). First attempt timed out on a 30 swaitForFunctionwhile the full 5 963-test suite ran concurrently — wall-clock flake, clean pass on rerun.bun run --cwd packages/app test -- chat-gesture-coverage— 5/5 (matrix rows 6/17 updated: ChatSurface PTT + composer-core slash coverage).bun run --cwd packages/ui typecheck— only the pre-existingiwermodule-resolution error in the untouched spatial e2e fixture (shared-worktree node_modules artifact).bun run --cwd packages/ui lint:check— touched files clean; remaining diagnostics pre-existing in untouched files.bun run audit:error-policy-ratchet— “no new fallback-slop in touched files”.bun run --cwd packages/app audit:app— could not boot in this environment: the audit's turbo pre-build fails at@elizaos/loggerwithTS2688: Cannot find type definition file for 'node'(shared-worktreenode_modulesresolution gap, same class as theiwererror; unrelated to this diff). Compensated by: the diff contains zero visual changes (no className/markup edits), plus the e2e runners' 60+ screenshots + 4 videos of the shipping composer states, reviewed by hand (orange accent only, no blue, no card chrome — see evidence).Evidence (
.github/issue-evidence/12188-composer-unification/)chat-sheet-composer-suite.webm— full composer walkthrough video (desktop + mobile drag, typing→send, attach, dictation, onboarding).chatux-gestures.webm,conversation-swipe-draft-handoff.webm— gesture + draft-handoff videos.overlay-composer-typing-send.png,overlay-composer-collapsed-desktop.png,overlay-composer-full-mobile.png,push-to-talk-dictation-in-draft.png,composer-image-attached.png— reviewed by hand.Line delta
Whole diff: 16 files, +1 025 / −316 (tests + evidence dominate the adds). Production code only: +436 / −260; the new shared module
composer-core.tsis +175 of that, and the three consuming surfaces shed their duplicated logic —ContinuousChatOverlay.tsx−59 net (−180 deleted),ChatView.tsx−36 net. Three private keydown implementations, two paste classifiers, two draft-persistence writers, and two private draft copies are now zero.Completes #12188 Phase 3.
🤖 Generated with Claude Code