Fix mobile composer focus race on expand#2499
Conversation
- Extract keyboard expand flow into a helper - Use flushSync to commit focus state before focusing the editor - Add a regression test for the expand sequence
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository 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)
Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
vouch:trusted
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Dead ref never assigned a non-null value
- Removed the dead mobileComposerExpandFrameRef, the always-no-op cancelPendingExpandFocus callback, its interface member, and the cleanup effect entry since the ref was never assigned a non-null value after the synchronous focus refactoring.
Or push these changes by commenting:
@cursor push 12084242b0
Preview (12084242b0)
diff --git a/apps/web/src/components/chat/ChatComposer.tsx b/apps/web/src/components/chat/ChatComposer.tsx
--- a/apps/web/src/components/chat/ChatComposer.tsx
+++ b/apps/web/src/components/chat/ChatComposer.tsx
@@ -813,7 +813,6 @@
const composerMenuItemsRef = useRef<ComposerCommandItem[]>([]);
const activeComposerMenuItemRef = useRef<ComposerCommandItem | null>(null);
const composerBlurFrameRef = useRef<number | null>(null);
- const mobileComposerExpandFrameRef = useRef<number | null>(null);
const mobileComposerExpandReleaseFrameRef = useRef<number | null>(null);
const mobileComposerExpandInFlightRef = useRef(false);
const dragDepthRef = useRef(0);
@@ -1636,12 +1635,6 @@
composerBlurFrameRef.current = null;
}
},
- cancelPendingExpandFocus: () => {
- if (mobileComposerExpandFrameRef.current !== null) {
- window.cancelAnimationFrame(mobileComposerExpandFrameRef.current);
- mobileComposerExpandFrameRef.current = null;
- }
- },
cancelPendingRelease: () => {
if (mobileComposerExpandReleaseFrameRef.current !== null) {
window.cancelAnimationFrame(mobileComposerExpandReleaseFrameRef.current);
@@ -1844,9 +1837,6 @@
if (composerBlurFrameRef.current !== null) {
window.cancelAnimationFrame(composerBlurFrameRef.current);
}
- if (mobileComposerExpandFrameRef.current !== null) {
- window.cancelAnimationFrame(mobileComposerExpandFrameRef.current);
- }
if (mobileComposerExpandReleaseFrameRef.current !== null) {
window.cancelAnimationFrame(mobileComposerExpandReleaseFrameRef.current);
}
diff --git a/apps/web/src/components/chat/mobileComposerFocus.test.ts b/apps/web/src/components/chat/mobileComposerFocus.test.ts
--- a/apps/web/src/components/chat/mobileComposerFocus.test.ts
+++ b/apps/web/src/components/chat/mobileComposerFocus.test.ts
@@ -7,7 +7,6 @@
expandMobileComposerForKeyboard({
cancelPendingBlur: vi.fn(() => calls.push("cancel-blur")),
- cancelPendingExpandFocus: vi.fn(() => calls.push("cancel-expand-focus")),
cancelPendingRelease: vi.fn(() => calls.push("cancel-release")),
setExpandInFlight: vi.fn((inFlight) => calls.push(`in-flight:${inFlight}`)),
commitExpandedState: vi.fn(() => calls.push("commit-expanded")),
@@ -17,7 +16,6 @@
expect(calls).toEqual([
"cancel-blur",
- "cancel-expand-focus",
"cancel-release",
"in-flight:true",
"commit-expanded",
diff --git a/apps/web/src/components/chat/mobileComposerFocus.ts b/apps/web/src/components/chat/mobileComposerFocus.ts
--- a/apps/web/src/components/chat/mobileComposerFocus.ts
+++ b/apps/web/src/components/chat/mobileComposerFocus.ts
@@ -1,6 +1,5 @@
export interface MobileComposerExpandOptions {
cancelPendingBlur: () => void;
- cancelPendingExpandFocus: () => void;
cancelPendingRelease: () => void;
setExpandInFlight: (inFlight: boolean) => void;
commitExpandedState: () => void;
@@ -10,7 +9,6 @@
export function expandMobileComposerForKeyboard(options: MobileComposerExpandOptions) {
options.cancelPendingBlur();
- options.cancelPendingExpandFocus();
options.cancelPendingRelease();
options.setExpandInFlight(true);
options.commitExpandedState();You can send follow-ups to the cloud agent here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved Targeted bug fix for mobile composer focus race condition. The refactoring simplifies state management by using CSS-based visibility instead of React state, with changes contained to mobile UI behavior and no sensitive code paths affected. You can customize Macroscope's approvability policy. Learn more. |
- Replace JS-driven collapse state with a primed mobile focus attribute - Simplify keyboard expansion flow and update focus sequencing tests
macroscopeapp
Bot
dismissed
their stale review
Dismissing prior approval to re-evaluate 699caa7
github-actions
Bot
added
size:L
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared a fix for 1 of the 2 issues found in the latest run.
- ✅ Fixed: Portaled floating layers cause mobile composer collapse
- Added data-mobile-floating-open attribute to the composer surface that tracks when CompactComposerControlsMenu or ProviderModelPicker is open, and included group-data-[mobile-floating-open=true]/composer in all mobile expanded/collapsed CSS conditions to prevent collapse while portaled layers are active.
Or push these changes by commenting:
@cursor push ee9cabf430
Preview (ee9cabf430)
diff --git a/apps/web/src/components/chat/ChatComposer.tsx b/apps/web/src/components/chat/ChatComposer.tsx
--- a/apps/web/src/components/chat/ChatComposer.tsx
+++ b/apps/web/src/components/chat/ChatComposer.tsx
@@ -785,13 +785,15 @@
const [isComposerFooterCompact, setIsComposerFooterCompact] = useState(false);
const [isComposerPrimaryActionsCompact, setIsComposerPrimaryActionsCompact] = useState(false);
const [isComposerModelPickerOpen, setIsComposerModelPickerOpen] = useState(false);
+ const [isComposerCompactMenuOpen, setIsComposerCompactMenuOpen] = useState(false);
const isMobileViewport = useMediaQuery("max-sm");
+ const isMobileFloatingLayerOpen = isComposerModelPickerOpen || isComposerCompactMenuOpen;
const mobileCollapsedOnlyClassName =
- "sm:hidden max-sm:group-focus-within/composer:hidden max-sm:group-data-[mobile-focus-primed=true]/composer:hidden";
+ "sm:hidden max-sm:group-focus-within/composer:hidden max-sm:group-data-[mobile-focus-primed=true]/composer:hidden max-sm:group-data-[mobile-floating-open=true]/composer:hidden";
const mobileExpandedOnlyClassName =
- "max-sm:hidden max-sm:group-focus-within/composer:block max-sm:group-data-[mobile-focus-primed=true]/composer:block";
+ "max-sm:hidden max-sm:group-focus-within/composer:block max-sm:group-data-[mobile-focus-primed=true]/composer:block max-sm:group-data-[mobile-floating-open=true]/composer:block";
const mobileExpandedFlexOnlyClassName =
- "max-sm:hidden max-sm:group-focus-within/composer:flex max-sm:group-data-[mobile-focus-primed=true]/composer:flex";
+ "max-sm:hidden max-sm:group-focus-within/composer:flex max-sm:group-data-[mobile-focus-primed=true]/composer:flex max-sm:group-data-[mobile-floating-open=true]/composer:flex";
// ------------------------------------------------------------------
// Refs
@@ -1915,6 +1917,7 @@
<div
ref={composerSurfaceRef}
data-mobile-focus-primed="false"
+ data-mobile-floating-open={isMobileFloatingLayerOpen ? "true" : "false"}
className={cn(
"group/composer rounded-[20px] border bg-card transition-colors duration-200 has-focus-visible:border-ring/45",
isDragOverComposer ? "border-primary/70 bg-accent/30" : "border-border",
@@ -2324,6 +2327,7 @@
runtimeMode={runtimeMode}
showInteractionModeToggle={composerProviderControls.showInteractionModeToggle}
traitsMenuContent={providerTraitsMenuContent}
+ onOpenChange={setIsComposerCompactMenuOpen}
onToggleInteractionMode={toggleInteractionMode}
onTogglePlanSidebar={togglePlanSidebar}
onRuntimeModeChange={handleRuntimeModeChange}
diff --git a/apps/web/src/components/chat/CompactComposerControlsMenu.tsx b/apps/web/src/components/chat/CompactComposerControlsMenu.tsx
--- a/apps/web/src/components/chat/CompactComposerControlsMenu.tsx
+++ b/apps/web/src/components/chat/CompactComposerControlsMenu.tsx
@@ -20,12 +20,13 @@
runtimeMode: RuntimeMode;
showInteractionModeToggle: boolean;
traitsMenuContent?: ReactNode;
+ onOpenChange?: (open: boolean) => void;
onToggleInteractionMode: () => void;
onTogglePlanSidebar: () => void;
onRuntimeModeChange: (mode: RuntimeMode) => void;
}) {
return (
- <Menu>
+ <Menu onOpenChange={(open) => props.onOpenChange?.(open)}>
<MenuTrigger
render={
<ButtonYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 699caa7. Configure here.
| data-mobile-focus-primed="false" | ||
| className={cn( | ||
| "rounded-[20px] border bg-card transition-colors duration-200 has-focus-visible:border-ring/45", | ||
| "group/composer rounded-[20px] border bg-card transition-colors duration-200 has-focus-visible:border-ring/45", |
There was a problem hiding this comment.
Portaled floating layers cause mobile composer collapse
Medium Severity
Removing isInsideComposerFloatingLayer and the onBlurCapture handler without a CSS-based replacement means that when mobile users open portaled floating layers (e.g., CompactComposerControlsMenu via MenuPrimitive.Portal or ProviderModelPicker via PopoverPrimitive.Portal), focus moves outside the group/composer div, focus-within deactivates, and the composer collapses. The footer containing the trigger disappears, breaking the interaction. The old code explicitly guarded against this via COMPOSER_FLOATING_LAYER_SELECTOR.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 699caa7. Configure here.
| options.primeExpandedState(); | ||
| options.focusEditorAtEnd(); | ||
| options.scheduleRelease(); | ||
| } |
There was a problem hiding this comment.
Trivial helper adds unnecessary indirection over inline calls
Low Severity
expandMobileComposerForKeyboard is a 4-line function that sequentially calls 4 callbacks. Since JavaScript is single-threaded, the call order is inherently deterministic without a wrapper. The associated test only verifies that JS executes statements top-to-bottom — a language guarantee. This adds indirection and a separate file without meaningful safety or encapsulation benefit beyond what inline calls already provide.
Reviewed by Cursor Bugbot for commit 699caa7. Configure here.
There was a problem hiding this comment.
Bugbot Autofix determined this is a false positive.
This is a code style preference, not a functional bug — the helper correctly orchestrates the focus sequence and the extracted module provides a clear contract boundary for the mobile focus flow.
You can send follow-ups to the cloud agent here.
1 participant
Summary
flushSyncso the focused state commits before the editor is focused.Testing
bun fmtbun lintbun typecheckbun run testNote
Medium Risk
Moderate risk: rewires mobile expand/collapse behavior from React focus state to CSS/data attributes and changes event timing (
pointerdown+ RAF), which could regress mobile-only composer visibility/focus edge cases.Overview
Fixes a mobile chat composer expand/focus race by removing the
isComposerFocusedstate + blur/expand RAF bookkeeping and instead driving collapsed/expanded UI purely via CSSgroup-focus-withinplus a temporarydata-mobile-focus-primedattribute.Extracts the expansion sequence into
expandMobileComposerForKeyboardand triggers expansion onpointerdown(not just click) for collapsed prompt/custom-answer interactions; adds a small unit test asserting the helper’s call order.Reviewed by Cursor Bugbot for commit 699caa7. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix mobile composer focus race on expand by replacing state with CSS and a primed data attribute
isComposerFocused) and multiple animation frame refs used to manage mobile composer expand/collapse, eliminating the race condition they caused.group focus-withinand adata-mobile-focus-primedattribute on the composer surface, set inChatComposer.tsx.mobileComposerFocus.ts: cancel pending release → prime expanded state → focus editor → schedule release.pointerDownbefore the click fires.onFocusCapture/onBlurCapturehandlers are removed; expand/collapse is no longer driven by focus events on the composer surface.Macroscope summarized 699caa7.