Skip to content

Commit 07a55f2

Browse files
authored
🤖 fix: preserve creation prompt caret on workspace updates (#1284)
Fixes an annoying UX bug on the ProjectPage (workspace creation screen): when the workspace tree updates (e.g. a new workspace/subworkspace is created elsewhere), we were re-running ChatInput’s `onReady` wiring and re-focusing the textarea, which forces the caret to the end. Changes: - ChatInput: stop re-running `onReady` effect on unrelated prop changes. - ProjectPage: only auto-focus the creation prompt once per mount (defensive against future regressions). - Added a regression test covering the ProjectPage autofocus behavior. Validation: - `make static-check` --- <details> <summary>📋 Implementation Plan</summary> # Fix: prompt textarea caret jumps to end when other workspaces appear ## What’s happening (and why it feels like the cursor “jumps to the bottom”) While you’re typing in the **workspace creation** prompt (the screen in your screenshot), the app re-renders when *any* workspace/subworkspace is created elsewhere. That re-render unintentionally re-runs the “ready” callback for the chat input, which **re-focuses the textarea and forcibly moves the caret to the end**. Concrete chain (with code pointers): - The screen is `ProjectPage` → `ChatInput` (`variant="creation"`) → `VimTextArea`. - `src/browser/components/ProjectPage.tsx` - `src/browser/components/ChatInput/index.tsx` - `ChatInput` exposes an API to parents via an `onReady` **effect**. - Today that effect depends on **`props` (the entire props object)**, so it re-runs whenever *any* prop identity changes. - `src/browser/components/ChatInput/index.tsx` (`useEffect` “Provide API to parent via callback”) - `App.tsx` passes an inline `onWorkspaceCreated={(metadata) => { … }}` function into `ProjectPage`, so **every App re-render creates a new function identity**. - When a workspace/subworkspace is created elsewhere, `workspaceMetadata` updates → `App` re-renders → new `onWorkspaceCreated` prop identity. - `src/browser/App.tsx` (rendering `<ProjectPage … onWorkspaceCreated={(metadata) => { … }} />`) - Because `ChatInput`’s `onReady` effect re-runs, `ProjectPage.handleChatReady` runs again and calls `api.focus()`. - `src/browser/components/ProjectPage.tsx` (`handleChatReady`) - `ChatInput.focusMessageInput()` always does: - `element.focus()` - then `selectionStart/selectionEnd = element.value.length` - so your caret moves to the end and the textarea scrolls to follow it. - `src/browser/components/ChatInput/index.tsx` (`focusMessageInput`) ## Recommended fix (Approach A — minimal, targeted) **Net product LoC estimate:** ~+10 / -2 1) **Stop re-running `onReady` on unrelated re-renders** - In `src/browser/components/ChatInput/index.tsx`, update the `useEffect` that calls `props.onReady(…)`. - **Remove `props`** from the dependency array; depend only on the specific values used (`props.onReady`, and the API callbacks). - This makes `onReady` behave like “component mounted / API changed” instead of “any parent re-render”. 2) **Make `ProjectPage`’s initial autofocus idempotent** - In `src/browser/components/ProjectPage.tsx`, guard `handleChatReady` so it only calls `api.focus()` once per mount. - e.g. `const didAutoFocusRef = useRef(false);` and only focus when false. - (Alternative guard) only focus if `document.activeElement` is not already the textarea / not inside the chat input. - This is defensive: even if `onReady` is called again for some legitimate reason in the future, we won’t steal the caret. ## Optional hardening (Approach B — improve focus behavior) **Net product LoC estimate:** ~+10–20 3) **Don’t force caret to end if the textarea is already focused** - In `focusMessageInput` (`src/browser/components/ChatInput/index.tsx`), detect when `document.activeElement === inputRef.current`. - If it’s already focused, skip the `selectionStart/selectionEnd = value.length` step. Why this is useful: - It prevents any future “spurious focus” call (from keybinds, popovers, or re-renders) from hijacking the user’s selection. <details> <summary>Alternative (more robust, more code): preserve selection across temporary focus loss</summary> If you want the caret to return to the *exact* prior position even after focus moves away (e.g. opening a model picker), store `selectionStart/End` in a ref via `onSelect`/`onKeyUp`, then restore it on re-focus. This is more invasive and should be done only if Approach A+B still leaves edge cases. </details> ## Tests / regression coverage **Net product LoC estimate:** 0 **Net test LoC estimate:** ~+40–80 Add a regression test that fails with today’s behavior: - Render `ChatInput` in `variant="creation"` (or render `ProjectPage` if that’s easier for setup). - Type a multi-line value, then place the caret in the middle (`setSelectionRange`). - Trigger a parent re-render that changes a prop identity (e.g., provide a new `onWorkspaceCreated` function via `rerender`). - Assert that `selectionStart/End` did not change. Good locations: - New: `src/browser/components/ChatInput/ChatInputCaret.test.tsx` - Or extend existing creation-related tests under `src/browser/components/ChatInput/`. ## Manual QA checklist - On the ProjectPage (creation screen), type a multi-line prompt. - Move caret near the top. - Cause another workspace/subworkspace to be created (e.g. from another workspace). - Verify caret stays where you left it (no jump to end). - Verify initial autofocus on first entering ProjectPage still works. - Verify “intentional” focus paths still work (e.g. switching workspaces in sidebar should still focus input). ## Notes / non-goals - This plan intentionally avoids larger architectural changes (like lifting `ChatInput` to a stable position in `App.tsx`). Those would also solve remount/selection issues but are higher-risk and higher-LoC. </details> --- _Generated with `mux` • Model: `openai:gpt-5.2` • Thinking: `xhigh`_ Signed-off-by: Thomas Kosiewski <[email protected]>
1 parent 086aea2 commit 07a55f2

File tree

3 files changed

+106
-11
lines changed

3 files changed

+106
-11
lines changed

src/browser/components/ChatInput/index.tsx

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -506,26 +506,20 @@ const ChatInputInner: React.FC<ChatInputProps> = (props) => {
506506
[setImageAttachments]
507507
);
508508

509+
const onReady = props.onReady;
510+
509511
// Provide API to parent via callback
510512
useEffect(() => {
511-
if (props.onReady) {
512-
props.onReady({
513+
if (onReady) {
514+
onReady({
513515
focus: focusMessageInput,
514516
restoreText,
515517
appendText,
516518
prependText,
517519
restoreImages,
518520
});
519521
}
520-
}, [
521-
props.onReady,
522-
focusMessageInput,
523-
restoreText,
524-
appendText,
525-
prependText,
526-
restoreImages,
527-
props,
528-
]);
522+
}, [onReady, focusMessageInput, restoreText, appendText, prependText, restoreImages]);
529523

530524
useEffect(() => {
531525
const handleGlobalKeyDown = (event: KeyboardEvent) => {
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import React, { useEffect } from "react";
2+
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
3+
import { GlobalWindow } from "happy-dom";
4+
import { cleanup, render, waitFor } from "@testing-library/react";
5+
6+
let focusMock: ReturnType<typeof mock> | null = null;
7+
let readyCalls = 0;
8+
9+
void mock.module("@/browser/contexts/API", () => ({
10+
useAPI: () => ({
11+
api: null,
12+
status: "connecting" as const,
13+
error: null,
14+
authenticate: () => undefined,
15+
retry: () => undefined,
16+
}),
17+
}));
18+
19+
// Mock ChatInput to simulate the old (buggy) behavior where onReady can fire again
20+
// on unrelated re-renders (e.g. workspace list updates).
21+
void mock.module("./ChatInput/index", () => ({
22+
ChatInput: (props: {
23+
onReady?: (api: {
24+
focus: () => void;
25+
restoreText: (text: string) => void;
26+
appendText: (text: string) => void;
27+
prependText: (text: string) => void;
28+
restoreImages: (images: unknown[]) => void;
29+
}) => void;
30+
}) => {
31+
useEffect(() => {
32+
readyCalls += 1;
33+
34+
props.onReady?.({
35+
focus: () => {
36+
if (!focusMock) {
37+
throw new Error("focusMock not initialized");
38+
}
39+
focusMock();
40+
},
41+
restoreText: () => undefined,
42+
appendText: () => undefined,
43+
prependText: () => undefined,
44+
restoreImages: () => undefined,
45+
});
46+
}, [props]);
47+
48+
return <div data-testid="ChatInputMock" />;
49+
},
50+
}));
51+
52+
import { ProjectPage } from "./ProjectPage";
53+
54+
describe("ProjectPage", () => {
55+
beforeEach(() => {
56+
const dom = new GlobalWindow();
57+
globalThis.window = dom as unknown as Window & typeof globalThis;
58+
globalThis.document = globalThis.window.document;
59+
60+
readyCalls = 0;
61+
focusMock = mock(() => undefined);
62+
});
63+
64+
afterEach(() => {
65+
cleanup();
66+
focusMock = null;
67+
globalThis.window = undefined as unknown as Window & typeof globalThis;
68+
globalThis.document = undefined as unknown as Document;
69+
});
70+
71+
test("auto-focuses the creation input only once even if ChatInput re-initializes", async () => {
72+
const baseProps = {
73+
projectPath: "/projects/demo",
74+
projectName: "demo",
75+
onProviderConfig: () => Promise.resolve(undefined),
76+
onWorkspaceCreated: () => undefined,
77+
};
78+
79+
const { rerender } = render(<ProjectPage {...baseProps} />);
80+
81+
await waitFor(() => expect(readyCalls).toBe(1));
82+
await waitFor(() => expect(focusMock).toHaveBeenCalledTimes(1));
83+
84+
// Simulate an unrelated App re-render that changes an inline callback identity.
85+
rerender(<ProjectPage {...baseProps} onWorkspaceCreated={() => undefined} />);
86+
87+
await waitFor(() => expect(readyCalls).toBe(2));
88+
89+
// Focus should not be re-triggered (would move caret to end).
90+
expect(focusMock).toHaveBeenCalledTimes(1);
91+
});
92+
});

src/browser/components/ProjectPage.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,17 @@ export const ProjectPage: React.FC<ProjectPageProps> = ({
5353
};
5454
}, [api, projectPath]);
5555

56+
const didAutoFocusRef = useRef(false);
5657
const handleChatReady = useCallback((api: ChatInputAPI) => {
5758
chatInputRef.current = api;
59+
60+
// Auto-focus the prompt once when entering the creation screen.
61+
// Defensive: avoid re-focusing on unrelated re-renders (e.g. workspace list updates),
62+
// which can move the user's caret.
63+
if (didAutoFocusRef.current) {
64+
return;
65+
}
66+
didAutoFocusRef.current = true;
5867
api.focus();
5968
}, []);
6069

0 commit comments

Comments
 (0)