From 79d0d6ea55ea020e5ded98a329d48e64981ff00e Mon Sep 17 00:00:00 2001 From: murashit Date: Wed, 24 Jun 2026 10:05:37 +0900 Subject: [PATCH] Promote composer page scrolling defaults --- .../application/composer/boundary-scroll.ts | 27 +++-- .../chat/panel/composer-controller.ts | 3 +- .../panel/surface/message-stream-scroll.ts | 2 +- .../chat/ui/message-stream/flow-scroll.ts | 17 +++ src/settings/tab.tsx | 4 +- .../composer/boundary-scroll.test.ts | 32 ++++-- .../conversation/composer/controller.test.ts | 100 ++++++++++++++++++ .../surface/message-stream-presenter.test.ts | 7 +- .../ui/message-stream/flow-scroll.test.ts | 16 +++ tests/settings/settings-tab.test.ts | 10 +- tests/settings/settings.test.ts | 1 + 11 files changed, 192 insertions(+), 27 deletions(-) diff --git a/src/features/chat/application/composer/boundary-scroll.ts b/src/features/chat/application/composer/boundary-scroll.ts index 249e28c0..70124edb 100644 --- a/src/features/chat/application/composer/boundary-scroll.ts +++ b/src/features/chat/application/composer/boundary-scroll.ts @@ -1,11 +1,20 @@ type ComposerBoundaryScrollDirection = -1 | 1; type ComposerBoundaryScrollAmount = "text-lines" | "page"; +type ComposerBoundaryScrollEdge = "start" | "end"; -export interface ComposerBoundaryScrollAction { +export type ComposerBoundaryScrollAction = ComposerBoundaryScrollByAction | ComposerBoundaryScrollToAction; + +interface ComposerBoundaryScrollByAction { + kind: "scroll-by"; direction: ComposerBoundaryScrollDirection; amount: ComposerBoundaryScrollAmount; } +interface ComposerBoundaryScrollToAction { + kind: "scroll-to"; + edge: ComposerBoundaryScrollEdge; +} + export interface ComposerBoundaryScrollKeyEvent { key: string; ctrlKey: boolean; @@ -34,7 +43,7 @@ export function composerBoundaryScrollDirection( const keyAction = composerBoundaryScrollKeyAction(event); if (!keyAction) return null; - if (keyAction.amount === "page") return keyAction; + if (keyAction.kind === "scroll-to" || keyAction.amount === "page") return keyAction; if (composer.selectionStart !== composer.selectionEnd) return null; return keyAction.direction === -1 @@ -48,16 +57,18 @@ export function composerBoundaryScrollDirection( function composerBoundaryScrollKeyAction(event: ComposerBoundaryScrollKeyEvent): ComposerBoundaryScrollAction | null { if (!event.ctrlKey) { - if (event.key === "ArrowUp") return { direction: -1, amount: "text-lines" }; - if (event.key === "ArrowDown") return { direction: 1, amount: "text-lines" }; - if (event.key === "PageUp") return { direction: -1, amount: "page" }; - if (event.key === "PageDown") return { direction: 1, amount: "page" }; + if (event.key === "ArrowUp") return { kind: "scroll-by", direction: -1, amount: "text-lines" }; + if (event.key === "ArrowDown") return { kind: "scroll-by", direction: 1, amount: "text-lines" }; + if (event.key === "PageUp") return { kind: "scroll-by", direction: -1, amount: "page" }; + if (event.key === "PageDown") return { kind: "scroll-by", direction: 1, amount: "page" }; + if (event.key === "Home") return { kind: "scroll-to", edge: "start" }; + if (event.key === "End") return { kind: "scroll-to", edge: "end" }; return null; } const key = event.key.toLowerCase(); - if (key === "p") return { direction: -1, amount: "text-lines" }; - if (key === "n") return { direction: 1, amount: "text-lines" }; + if (key === "p") return { kind: "scroll-by", direction: -1, amount: "text-lines" }; + if (key === "n") return { kind: "scroll-by", direction: 1, amount: "text-lines" }; return null; } diff --git a/src/features/chat/panel/composer-controller.ts b/src/features/chat/panel/composer-controller.ts index ccd6b729..56f1e447 100644 --- a/src/features/chat/panel/composer-controller.ts +++ b/src/features/chat/panel/composer-controller.ts @@ -153,13 +153,14 @@ export class ChatComposerController { } private handleBoundaryScrollKeydown(event: KeyboardEvent): boolean { - if (!this.composer || !this.options.scrollThreadFromComposerEdges()) return false; + if (!this.composer) return false; const composer = this.composer; const action = composerBoundaryScrollDirection(event, composer, { cursorAtVisualBoundary: (direction) => textareaCursorAtVisualBoundary(direction, composer), }); if (!action) return false; + if (action.kind === "scroll-by" && action.amount === "text-lines" && !this.options.scrollThreadFromComposerEdges()) return false; event.preventDefault(); this.options.threadScrollFromComposer(action); diff --git a/src/features/chat/panel/surface/message-stream-scroll.ts b/src/features/chat/panel/surface/message-stream-scroll.ts index f67af888..b9d37a2c 100644 --- a/src/features/chat/panel/surface/message-stream-scroll.ts +++ b/src/features/chat/panel/surface/message-stream-scroll.ts @@ -31,7 +31,7 @@ export function createChatMessageScrollController(): ChatMessageScrollController }, scrollFromComposer(action): void { - dispatch({ kind: "scroll-by", amount: action.amount, direction: action.direction }); + dispatch(action); }, dispose(): void { diff --git a/src/features/chat/ui/message-stream/flow-scroll.ts b/src/features/chat/ui/message-stream/flow-scroll.ts index 3c056c1d..ae4d8b43 100644 --- a/src/features/chat/ui/message-stream/flow-scroll.ts +++ b/src/features/chat/ui/message-stream/flow-scroll.ts @@ -6,6 +6,7 @@ type MessageScrollDirection = -1 | 1; export type MessageStreamScrollCommand = | { kind: "show-latest" } + | { kind: "scroll-to"; edge: "start" | "end" } | { kind: "scroll-by"; amount: "text-lines" | "page"; direction: MessageScrollDirection }; export interface MessageStreamScrollPort { @@ -207,6 +208,15 @@ function applyMessageFlowScrollCommand(runtime: MessageFlowRuntime, command: Mes scrollMessageFlowToEnd(runtime); scheduleMessageFlowEndRestore(runtime); break; + case "scroll-to": + if (command.edge === "start") { + scrollMessageFlowToStart(runtime); + } else { + runtime.followingEnd = true; + scrollMessageFlowToEnd(runtime); + scheduleMessageFlowEndRestore(runtime); + } + break; case "scroll-by": scrollMessageFlowBy(runtime, messageFlowScrollDelta(runtime, command.amount, command.direction)); break; @@ -226,6 +236,13 @@ function scrollMessageFlowBy(runtime: MessageFlowRuntime, delta: number): void { runtime.followingEnd = isMessageFlowAtEnd(container); } +function scrollMessageFlowToStart(runtime: MessageFlowRuntime): void { + const container = runtime.container; + if (!container) return; + container.scrollTop = 0; + runtime.followingEnd = false; +} + function messageFlowScrollDelta(runtime: MessageFlowRuntime, amount: "text-lines" | "page", direction: MessageScrollDirection): number { const container = runtime.container; if (!container) return 0; diff --git a/src/settings/tab.tsx b/src/settings/tab.tsx index 52b47387..61bc93c5 100644 --- a/src/settings/tab.tsx +++ b/src/settings/tab.tsx @@ -143,8 +143,8 @@ export class CodexPanelSettingTab extends PluginSettingTab { }); }); new Setting(composerItems) - .setName("Scroll thread from composer edges") - .setDesc("Use Up/Ctrl+P and Down/Ctrl+N at composer edges to scroll the thread.") + .setName("Scroll thread from composer line edges") + .setDesc("Use Up/Ctrl+P and Down/Ctrl+N at composer line edges to scroll the thread.") .addToggle((toggle) => { toggle.setValue(this.plugin.settings.scrollThreadFromComposerEdges).onChange(async (value) => { this.plugin.settings.scrollThreadFromComposerEdges = value; diff --git a/tests/features/chat/conversation/composer/boundary-scroll.test.ts b/tests/features/chat/conversation/composer/boundary-scroll.test.ts index cb1cd88d..ac7d1670 100644 --- a/tests/features/chat/conversation/composer/boundary-scroll.test.ts +++ b/tests/features/chat/conversation/composer/boundary-scroll.test.ts @@ -7,19 +7,37 @@ import { describe("composer boundary scroll shortcuts", () => { it("scrolls up from the first composer line", () => { - expect(direction("ArrowUp", "first\nsecond", 3)).toEqual({ direction: -1, amount: "text-lines" }); - expect(direction("p", "first\nsecond", 3, { ctrlKey: true })).toEqual({ direction: -1, amount: "text-lines" }); + expect(direction("ArrowUp", "first\nsecond", 3)).toEqual({ kind: "scroll-by", direction: -1, amount: "text-lines" }); + expect(direction("p", "first\nsecond", 3, { ctrlKey: true })).toEqual({ + kind: "scroll-by", + direction: -1, + amount: "text-lines", + }); }); it("scrolls down from the last composer line", () => { - expect(direction("ArrowDown", "first\nsecond", 9)).toEqual({ direction: 1, amount: "text-lines" }); - expect(direction("n", "first\nsecond", 9, { ctrlKey: true })).toEqual({ direction: 1, amount: "text-lines" }); + expect(direction("ArrowDown", "first\nsecond", 9)).toEqual({ kind: "scroll-by", direction: 1, amount: "text-lines" }); + expect(direction("n", "first\nsecond", 9, { ctrlKey: true })).toEqual({ + kind: "scroll-by", + direction: 1, + amount: "text-lines", + }); }); it("scrolls by page from any composer line for PageUp and PageDown", () => { - expect(direction("PageUp", "first\nsecond", 9)).toEqual({ direction: -1, amount: "page" }); - expect(direction("PageDown", "first\nsecond", 3)).toEqual({ direction: 1, amount: "page" }); - expect(direction("PageDown", "first\nsecond", 3, { selectionEnd: 8 })).toEqual({ direction: 1, amount: "page" }); + expect(direction("PageUp", "first\nsecond", 9)).toEqual({ kind: "scroll-by", direction: -1, amount: "page" }); + expect(direction("PageDown", "first\nsecond", 3)).toEqual({ kind: "scroll-by", direction: 1, amount: "page" }); + expect(direction("PageDown", "first\nsecond", 3, { selectionEnd: 8 })).toEqual({ + kind: "scroll-by", + direction: 1, + amount: "page", + }); + }); + + it("scrolls to stream edges from any composer line for Home and End", () => { + expect(direction("Home", "first\nsecond", 9)).toEqual({ kind: "scroll-to", edge: "start" }); + expect(direction("End", "first\nsecond", 3)).toEqual({ kind: "scroll-to", edge: "end" }); + expect(direction("End", "first\nsecond", 3, { selectionEnd: 8 })).toEqual({ kind: "scroll-to", edge: "end" }); }); it("keeps normal cursor movement away from composer edges", () => { diff --git a/tests/features/chat/conversation/composer/controller.test.ts b/tests/features/chat/conversation/composer/controller.test.ts index e54b26e1..12f0dae2 100644 --- a/tests/features/chat/conversation/composer/controller.test.ts +++ b/tests/features/chat/conversation/composer/controller.test.ts @@ -243,6 +243,106 @@ describe("ChatComposerController", () => { expect(submit).toHaveBeenCalledOnce(); }); + it("scrolls by page from the composer even when line edge scrolling is disabled", () => { + const stateStore = createChatStateStore(); + const parent = document.createElement("div"); + const threadScrollFromComposer = vi.fn(); + const controller = new ChatComposerController({ + noteCandidateProvider: noteProvider(), + sourcePath: () => "", + stateStore, + viewId: "view", + sendShortcut: () => "enter", + scrollThreadFromComposerEdges: () => false, + threadScrollFromComposer, + canInterrupt: (_state) => false, + composerProjection: defaultComposerProjection, + currentModelForSuggestions: () => null, + togglePlan: vi.fn(), + toggleAutoReview: vi.fn(), + toggleFast: vi.fn(), + onDraftChange: vi.fn(), + onHeightChange: vi.fn(), + }); + + renderComposerController(parent, controller, stateStore); + setTextAreaValue(composer(parent), "first\nsecond"); + composer(parent).setSelectionRange(3, 3); + const event = new KeyboardEvent("keydown", { bubbles: true, cancelable: true, key: "PageDown" }); + composer(parent).dispatchEvent(event); + + expect(threadScrollFromComposer).toHaveBeenCalledWith({ kind: "scroll-by", direction: 1, amount: "page" }); + expect(event.defaultPrevented).toBe(true); + }); + + it("scrolls to stream edges from the composer even when line edge scrolling is disabled", () => { + const stateStore = createChatStateStore(); + const parent = document.createElement("div"); + const threadScrollFromComposer = vi.fn(); + const controller = new ChatComposerController({ + noteCandidateProvider: noteProvider(), + sourcePath: () => "", + stateStore, + viewId: "view", + sendShortcut: () => "enter", + scrollThreadFromComposerEdges: () => false, + threadScrollFromComposer, + canInterrupt: (_state) => false, + composerProjection: defaultComposerProjection, + currentModelForSuggestions: () => null, + togglePlan: vi.fn(), + toggleAutoReview: vi.fn(), + toggleFast: vi.fn(), + onDraftChange: vi.fn(), + onHeightChange: vi.fn(), + }); + + renderComposerController(parent, controller, stateStore); + setTextAreaValue(composer(parent), "first\nsecond"); + composer(parent).setSelectionRange(3, 8); + const home = new KeyboardEvent("keydown", { bubbles: true, cancelable: true, key: "Home" }); + composer(parent).dispatchEvent(home); + const end = new KeyboardEvent("keydown", { bubbles: true, cancelable: true, key: "End" }); + composer(parent).dispatchEvent(end); + + expect(threadScrollFromComposer).toHaveBeenNthCalledWith(1, { kind: "scroll-to", edge: "start" }); + expect(threadScrollFromComposer).toHaveBeenNthCalledWith(2, { kind: "scroll-to", edge: "end" }); + expect(home.defaultPrevented).toBe(true); + expect(end.defaultPrevented).toBe(true); + }); + + it("leaves composer line edge scrolling disabled by the setting", () => { + const stateStore = createChatStateStore(); + const parent = document.createElement("div"); + const threadScrollFromComposer = vi.fn(); + const controller = new ChatComposerController({ + noteCandidateProvider: noteProvider(), + sourcePath: () => "", + stateStore, + viewId: "view", + sendShortcut: () => "enter", + scrollThreadFromComposerEdges: () => false, + threadScrollFromComposer, + canInterrupt: (_state) => false, + composerProjection: defaultComposerProjection, + currentModelForSuggestions: () => null, + togglePlan: vi.fn(), + toggleAutoReview: vi.fn(), + toggleFast: vi.fn(), + onDraftChange: vi.fn(), + onHeightChange: vi.fn(), + }); + + renderComposerController(parent, controller, stateStore); + setTextAreaValue(composer(parent), "first\nsecond"); + composer(parent).setSelectionRange("first\nsecond".length, "first\nsecond".length); + const event = new KeyboardEvent("keydown", { bubbles: true, cancelable: true, key: "n", ctrlKey: true }); + composer(parent).dispatchEvent(event); + + expect(threadScrollFromComposer).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(false); + }); + it("clears the Preact-owned textarea ref when the composer unmounts", () => { const stateStore = createChatStateStore(); stateStore.dispatch({ type: "composer/draft-set", draft: "state draft" }); diff --git a/tests/features/chat/panel/surface/message-stream-presenter.test.ts b/tests/features/chat/panel/surface/message-stream-presenter.test.ts index 965e07b9..1fc73e00 100644 --- a/tests/features/chat/panel/surface/message-stream-presenter.test.ts +++ b/tests/features/chat/panel/surface/message-stream-presenter.test.ts @@ -317,8 +317,9 @@ describe("MessageStreamPresenter scroll pinning", () => { expect(() => { scrollController.showLatest(); - scrollController.scrollFromComposer({ direction: 1, amount: "text-lines" }); - scrollController.scrollFromComposer({ direction: -1, amount: "page" }); + scrollController.scrollFromComposer({ kind: "scroll-by", direction: 1, amount: "text-lines" }); + scrollController.scrollFromComposer({ kind: "scroll-by", direction: -1, amount: "page" }); + scrollController.scrollFromComposer({ kind: "scroll-to", edge: "start" }); presenter.dispose(); scrollController.showLatest(); }).not.toThrow(); @@ -349,7 +350,7 @@ describe("MessageStreamPresenter scroll pinning", () => { expect(() => { scrollController.showLatest(); - scrollController.scrollFromComposer({ direction: 1, amount: "page" }); + scrollController.scrollFromComposer({ kind: "scroll-by", direction: 1, amount: "page" }); }).not.toThrow(); }); diff --git a/tests/features/chat/ui/message-stream/flow-scroll.test.ts b/tests/features/chat/ui/message-stream/flow-scroll.test.ts index 4f3427df..8ce8cbc8 100644 --- a/tests/features/chat/ui/message-stream/flow-scroll.test.ts +++ b/tests/features/chat/ui/message-stream/flow-scroll.test.ts @@ -182,6 +182,22 @@ describe("message stream flow scrolling", () => { }); expect(messages.scrollTop).toBe(280); }); + + it("scrolls to stream edges from composer commands", () => { + const { controller, messages } = renderFlowMessageStream(["first", "second"], { first: 300, second: 300 }); + messages.scrollTop = 240; + messages.dispatchEvent(new Event("scroll")); + + void act(() => { + controller.dispatch({ kind: "scroll-to", edge: "start" }); + }); + expect(messages.scrollTop).toBe(0); + + void act(() => { + controller.dispatch({ kind: "scroll-to", edge: "end" }); + }); + expect(messages.scrollTop).toBe(500); + }); }); interface TestMessageStreamScrollController extends MessageStreamScrollControllerBinding { diff --git a/tests/settings/settings-tab.test.ts b/tests/settings/settings-tab.test.ts index e3192a4d..4f114a96 100644 --- a/tests/settings/settings-tab.test.ts +++ b/tests/settings/settings-tab.test.ts @@ -107,7 +107,7 @@ describe("settings tab", () => { "Show chat toolbar", "Composer", "Send shortcut", - "Scroll thread from composer edges", + "Scroll thread from composer line edges", "Codex helpers", "Automatic thread naming", "Selection rewrite", @@ -157,20 +157,20 @@ describe("settings tab", () => { expect(settingDesc(tab, "Show chat toolbar")).toContain("toolbar above the chat panel"); }); - it("saves the composer edge scroll setting", async () => { + it("saves the composer line edge scroll setting", async () => { const saveSettings = vi.fn().mockResolvedValue(undefined); const tab = newSettingsTab({ saveSettings }); tab.display(); - const toggle = inputForSetting(tab, "Scroll thread from composer edges"); - if (!toggle) throw new Error("Missing composer edge scroll toggle"); + const toggle = inputForSetting(tab, "Scroll thread from composer line edges"); + if (!toggle) throw new Error("Missing composer line edge scroll toggle"); toggle.checked = true; toggle.dispatchEvent(new Event("change")); await flushPromises(); expect(saveSettings).toHaveBeenCalledOnce(); - expect(settingDesc(tab, "Scroll thread from composer edges")).toContain("Up/Ctrl+P"); + expect(settingDesc(tab, "Scroll thread from composer line edges")).toContain("Up/Ctrl+P"); }); it("saves archive export settings", async () => { diff --git a/tests/settings/settings.test.ts b/tests/settings/settings.test.ts index 6cf5b16d..24cdc9ac 100644 --- a/tests/settings/settings.test.ts +++ b/tests/settings/settings.test.ts @@ -110,6 +110,7 @@ describe("settings", () => { }); it("normalizes composer edge scrolling", () => { + expect(normalizeSettings({}).scrollThreadFromComposerEdges).toBe(false); expect(normalizeSettings({ scrollThreadFromComposerEdges: true }).scrollThreadFromComposerEdges).toBe(true); expect(normalizeSettings({ scrollThreadFromComposerEdges: "yes" }).scrollThreadFromComposerEdges).toBe( DEFAULT_SETTINGS.scrollThreadFromComposerEdges,