diff --git a/.changeset/composer-scroll-containment.md b/.changeset/composer-scroll-containment.md new file mode 100644 index 00000000..af1d24c7 --- /dev/null +++ b/.changeset/composer-scroll-containment.md @@ -0,0 +1,11 @@ +--- +"@intentface/chat": patch +--- + +A composer embedded in a scrolling page no longer scrolls that page. + +**Programmatic text changes.** `clear()`, `setText()` and the ask-user step changes used to nudge the page by a few pixels each time. Keeping the caret in view after a write now scrolls only the composer's own scroll areas, never past `Composer.Root`. A caret on an empty line, including an empty editor's, is measured by that line's break: on its own it has no box, and its all-zero rect read as "above the view". A trailing empty line in an overflowing composer now scrolls into view too. + +**Ask-user focus moves.** Moving into the options, arrowing between them, and typing to answer move focus without the browser's focus-scroll. An option outside its own list's view is still brought into view within the question. + +**`focus(options)`.** The composer controller's `focus` now takes `HTMLElement.focus()`'s options, so a consumer can focus an on-screen composer with `{ preventScroll: true }`. Called with no arguments, it behaves as before. diff --git a/content/docs/primitives/composer.mdx b/content/docs/primitives/composer.mdx index 4361f7de..3dcce6cb 100644 --- a/content/docs/primitives/composer.mdx +++ b/content/docs/primitives/composer.mdx @@ -240,7 +240,9 @@ export const rootAttrs = [ Returns a `ComposerStore` handle. Pass it to `store`, read it with `useComposerStore(store, selector)`, and drive it imperatively through `store.controller` (`focus`, `blur`, `clear`, `insertText`, `insertChip`, -`getText`, `setText`, `serialize`, `ensureFocus`). +`getText`, `setText`, `serialize`, `ensureFocus`). `focus` takes the same +options as `HTMLElement.focus()`: pass `{ preventScroll: true }` to focus a +composer that's already on screen without scrolling the page around it. ```tsx import { Composer, useComposerStore } from "@intentface/chat/composer"; diff --git a/content/docs/primitives/composer/demos/ask-user-flow.tsx b/content/docs/primitives/composer/demos/ask-user-flow.tsx index 381b32f8..b7de9072 100644 --- a/content/docs/primitives/composer/demos/ask-user-flow.tsx +++ b/content/docs/primitives/composer/demos/ask-user-flow.tsx @@ -82,15 +82,18 @@ export const AskUserFlow = () => { - {done && ( - - )} + {/* A fixed slot: the button appearing must not push the composer up. */} +
+ {done && ( + + )} +
); }; diff --git a/packages/chat/src/ask-user/ask-user.tsx b/packages/chat/src/ask-user/ask-user.tsx index 7e43e027..95cc9259 100644 --- a/packages/chat/src/ask-user/ask-user.tsx +++ b/packages/chat/src/ask-user/ask-user.tsx @@ -21,6 +21,7 @@ import { import { useComposer } from "../composer/store"; import type { PrimitiveProps } from "../internal/primitive-props"; import { useRenderElement } from "../internal/render/useRenderElement"; +import { scrollIntoViewWithin } from "../internal/scroll-into-view-within"; // Shared ids so Options can label itself from the question text and describe // itself from the step indicator without any consumer wiring: Root mints both, @@ -230,9 +231,16 @@ export const AskUserOptions = ({ }, []); // Roving tabindex: DOM focus follows the highlight for keyboard navigation. + // Focus without the browser's scroll, which would also move the page around + // an embedded question; bring the option into view within the question's + // own scroll area instead. const focusValue = useCallback((itemValue: string | null) => { if (itemValue === null) return; - itemElements.current.get(itemValue)?.()?.focus(); + const element = itemElements.current.get(itemValue)?.(); + if (!element) return; + element.focus({ preventScroll: true }); + const boundary = element.closest("[data-ask-user]") ?? element; + scrollIntoViewWithin(element.getBoundingClientRect(), element, boundary); }, []); useImperativeHandle( diff --git a/packages/chat/src/composer/editor-dom.ts b/packages/chat/src/composer/editor-dom.ts index 8a1ea981..acbfda6e 100644 --- a/packages/chat/src/composer/editor-dom.ts +++ b/packages/chat/src/composer/editor-dom.ts @@ -20,6 +20,7 @@ // Round-trips: "" ↔ , "a\n" ↔ a
, "a\n\n" ↔ a

. import type { ChipData } from "../chip-markdown"; +import { scrollIntoViewWithin } from "../internal/scroll-into-view-within"; import { nextChipId, type SegmentDoc } from "./segments"; // --------------------------------------------------------------------------- @@ -504,29 +505,36 @@ export const writeCaretToDom = (root: HTMLElement, start: number, end: number = selection.setBaseAndExtent(anchor.node, anchor.offset, focus.node, focus.offset); }; -// Native typing auto-scrolls; programmatic writes don't. Nudge only the nearest -// scrollable ancestor (never scrollIntoView — it yanks the whole page). +// The caret's box. A collapsed caret on an empty line has none of its own — +// browsers return an all-zero rect, which would read as "above the view" — so +// measure the line instead: the
that follows it (a line break, or the +// padding break that gives an empty document its line). Null if neither has +// a box. +const measureCaret = (range: Range): DOMRect | null => { + const rect = range.getBoundingClientRect(); + if (rect.height > 0) return rect; + const next = range.startContainer.childNodes[range.startOffset]; + if (next?.nodeName !== "BR") return null; + const line = next.ownerDocument?.createRange(); + if (!line) return null; + line.selectNode(next); + const lineRect = line.getBoundingClientRect(); + return lineRect.height > 0 ? lineRect : null; +}; + +// Native typing auto-scrolls; programmatic writes don't. Scroll the caret into +// view within the composer's own scroll areas, never the page around it. The +// caret is the selection's focus end: a restored selection can be taller than +// the view, and its top edge would win. export const scrollCaretIntoView = (root: HTMLElement): void => { const selection = root.ownerDocument.getSelection(); - if (!selection || selection.rangeCount === 0) return; - const rect = selection.getRangeAt(0).getBoundingClientRect(); - - let scroller: HTMLElement | null = root; - while (scroller && scroller !== root.ownerDocument.body) { - const { overflowY } = getComputedStyle(scroller); - if ( - (overflowY === "auto" || overflowY === "scroll") && - scroller.scrollHeight > scroller.clientHeight - ) { - break; - } - scroller = scroller.parentElement; - } - if (!scroller || scroller === root.ownerDocument.body) return; - - const view = scroller.getBoundingClientRect(); - if (rect.top < view.top) scroller.scrollTop -= view.top - rect.top; - else if (rect.bottom > view.bottom) scroller.scrollTop += rect.bottom - view.bottom; + if (!selection?.focusNode) return; + const caret = root.ownerDocument.createRange(); + caret.setStart(selection.focusNode, selection.focusOffset); + caret.collapse(true); + const rect = measureCaret(caret); + if (!rect) return; + scrollIntoViewWithin(rect, root, root.closest("[data-composer-root]") ?? root); }; // --------------------------------------------------------------------------- diff --git a/packages/chat/src/composer/store.ts b/packages/chat/src/composer/store.ts index a7a2800f..431e813c 100644 --- a/packages/chat/src/composer/store.ts +++ b/packages/chat/src/composer/store.ts @@ -61,7 +61,7 @@ export type ComposerEditorState = ComposerEditorHandle & { const createEditorController = ( editorRef: RefObject, ): ComposerEditorState => ({ - focus: () => editorRef.current?.focus(), + focus: (options) => editorRef.current?.focus(options), blur: () => editorRef.current?.blur(), clear: () => editorRef.current?.clear(), insertText: (text) => editorRef.current?.insertText(text), @@ -193,6 +193,11 @@ export type ComposerStore = { // the store factory so activateAskUser reads as just "blur, listen, unlisten". // --------------------------------------------------------------------------- +// Focus moves the ask-user flow makes itself — back to the input, or in from +// an option. The composer is on screen already, and the browser's +// focus-scroll would also move the page around an embedded composer. +const FOCUS_IN_PLACE: FocusOptions = { preventScroll: true }; + // Scope a document-level listener to this composer: keystrokes aimed at // another editable or inside another composer's options are theirs; our own // options/editor and unclaimed targets (body — e.g. after a click on panel @@ -253,7 +258,7 @@ const createAskUserKeydownHandler = return; case "navigate-options": { const newValue = optionsHandle?.navigate(action.direction); - if (newValue === null) controller.focus(); + if (newValue === null) controller.focus(FOCUS_IN_PLACE); return; } case "select-option": { @@ -269,7 +274,7 @@ const createAskUserKeydownHandler = return; case "insert-character": optionsHandle?.clearHighlight(); - controller.focus(); + controller.focus(FOCUS_IN_PLACE); controller.insertText(action.character); return; } @@ -424,7 +429,7 @@ export const createComposerStore = (): ComposerStore => { controller.setText(effect.text); break; case "focus-input": - controller.focus(); + controller.focus(FOCUS_IN_PLACE); break; case "focus-options": // Step advance / navigation: focus lands on the new step's diff --git a/packages/chat/src/composer/types.ts b/packages/chat/src/composer/types.ts index 563ef49c..d12efe1a 100644 --- a/packages/chat/src/composer/types.ts +++ b/packages/chat/src/composer/types.ts @@ -35,7 +35,8 @@ export type TriggerRule = "doc-start" | "word-boundary"; export type { ChipData }; export type ComposerEditorHandle = { - focus: () => void; + /** Focus the editor. Takes `HTMLElement.focus()`'s options: pass `{ preventScroll: true }` to leave the page where it is. */ + focus: (options?: FocusOptions) => void; blur: () => void; clear: () => void; insertText: (text: string) => void; diff --git a/packages/chat/src/composer/use-composer-editor.tsx b/packages/chat/src/composer/use-composer-editor.tsx index e021e084..9d5108ef 100644 --- a/packages/chat/src/composer/use-composer-editor.tsx +++ b/packages/chat/src/composer/use-composer-editor.tsx @@ -333,10 +333,13 @@ const createEditorEngine = (getDependencies: () => EngineDependencies) => { mirrorCommandState(); }; - const focusEditor = () => { + const focusEditor = (options?: FocusOptions) => { if (!root) return; - root.focus(); + root.focus(options); writeCaretToDom(root, caret.start, caret.end); + // Focus scrolls the editor box into view (unless prevented), not the + // caret: keep the caret visible inside the composer's own scroll area. + scrollCaretIntoView(root); }; // --- Event handlers (attached as native listeners — React synthetics @@ -658,7 +661,8 @@ const createEditorEngine = (getDependencies: () => EngineDependencies) => { }, closeCommands, dismissCommands: () => { - root?.focus(); + // The editor is on screen already: moving focus back must not scroll the page. + root?.focus({ preventScroll: true }); closeCommands(); }, }; diff --git a/packages/chat/src/internal/scroll-into-view-within.ts b/packages/chat/src/internal/scroll-into-view-within.ts new file mode 100644 index 00000000..a67692f6 --- /dev/null +++ b/packages/chat/src/internal/scroll-into-view-within.ts @@ -0,0 +1,26 @@ +// Bring a rect into view by scrolling each scrollable ancestor of `from` just +// enough — but never one outside `boundary`. A primitive scrolls only its own +// scroll areas: walking further would reach the page around it (an embedded +// composer's docs page, say) and yank it, which is exactly what scrollIntoView +// does and why it isn't used. `boundary` itself may scroll. +export const scrollIntoViewWithin = (rect: DOMRect, from: HTMLElement, boundary: Element): void => { + let { top, bottom } = rect; + let scroller: HTMLElement | null = from; + while (scroller && boundary.contains(scroller)) { + const { overflowY } = getComputedStyle(scroller); + if ( + (overflowY === "auto" || overflowY === "scroll") && + scroller.scrollHeight > scroller.clientHeight + ) { + const view = scroller.getBoundingClientRect(); + const before = scroller.scrollTop; + if (top < view.top) scroller.scrollTop -= view.top - top; + else if (bottom > view.bottom) scroller.scrollTop += bottom - view.bottom; + // The rect moves with the scrolled content; carry that to the next scroller out. + const moved = scroller.scrollTop - before; + top -= moved; + bottom -= moved; + } + scroller = scroller.parentElement; + } +}; diff --git a/packages/chat/tests/composer-scroll-containment.test.tsx b/packages/chat/tests/composer-scroll-containment.test.tsx new file mode 100644 index 00000000..f918dd5c --- /dev/null +++ b/packages/chat/tests/composer-scroll-containment.test.tsx @@ -0,0 +1,288 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, fireEvent, render, screen } from "@testing-library/react"; +import { AskUser } from "../src/ask-user"; +import { Composer } from "../src/composer"; +import { scrollCaretIntoView } from "../src/composer/editor-dom"; +import { useComposer } from "../src/composer/store"; +import type { AskUserQuestion } from "../src/composer/types"; +import { scrollIntoViewWithin } from "../src/internal/scroll-into-view-within"; + +// A composer embedded in a scrolling page scrolls only its own scroll areas: +// programmatic writes and the ask-user flow's focus moves must leave the page +// where it is (CHAT-42). + +// happy-dom has no layout, so each element gets a stubbed box. +type Box = { top: number; height: number; scrollHeight?: number }; + +const place = (element: HTMLElement, { top, height, scrollHeight = height }: Box) => { + element.getBoundingClientRect = () => + ({ top, bottom: top + height, height, left: 0, right: 0, width: 0 }) as DOMRect; + Object.defineProperty(element, "clientHeight", { value: height, configurable: true }); + Object.defineProperty(element, "scrollHeight", { value: scrollHeight, configurable: true }); + Object.defineProperty(element, "scrollTop", { value: 0, writable: true, configurable: true }); +}; + +// page (scrolls) > composer root > text area (may scroll) > editor +const embeddedComposer = ({ textAreaOverflows }: { textAreaOverflows: boolean }) => { + const page = document.createElement("div"); + const composer = document.createElement("div"); + const textArea = document.createElement("div"); + const editor = document.createElement("div"); + page.style.overflowY = "auto"; + textArea.style.overflowY = "auto"; + composer.setAttribute("data-composer-root", ""); + page.append(composer); + composer.append(textArea); + textArea.append(editor); + document.body.append(page); + + place(page, { top: 100, height: 600, scrollHeight: 2000 }); + place(composer, { top: 400, height: 140 }); + place(textArea, { top: 410, height: 100, scrollHeight: textAreaOverflows ? 400 : 100 }); + place(editor, { top: 410, height: textAreaOverflows ? 400 : 100 }); + return { page, composer, textArea, editor }; +}; + +const rectOf = (rect: Partial) => + ({ top: 0, bottom: 0, height: 0, left: 0, right: 0, width: 0, ...rect }) as DOMRect; + +// A range with a stubbed box that records where it was placed. +const fakeRange = (rect: Partial) => { + const range = { + startContainer: document.body as Node, + startOffset: 0, + selectedNode: null as Node | null, + setStart: (container: Node, offset: number) => { + range.startContainer = container; + range.startOffset = offset; + }, + collapse: () => {}, + selectNode: (node: Node) => { + range.selectedNode = node; + }, + getBoundingClientRect: () => rectOf(rect), + }; + return range; +}; + +// The selection's focus point, and the boxes measured there: the caret's own, +// then — when that is empty — the line's. +const caretAt = ( + rect: Partial, + point: { container: Node; offset: number } = { container: document.body, offset: 0 }, + lineRect: Partial = {}, +) => { + const caret = fakeRange(rect); + const line = fakeRange(lineRect); + const ranges = [caret, line]; + let created = 0; + document.getSelection = () => + ({ rangeCount: 1, focusNode: point.container, focusOffset: point.offset }) as never; + document.createRange = () => ranges[created++] as never; + return { caret, line }; +}; + +describe("scrollCaretIntoView", () => { + const getSelection = document.getSelection.bind(document); + const createRange = document.createRange.bind(document); + afterEach(() => { + document.getSelection = getSelection; + document.createRange = createRange; + document.body.innerHTML = ""; + }); + + test("a caret with no box and no line to measure scrolls nothing", () => { + const { page, textArea, editor } = embeddedComposer({ textAreaOverflows: false }); + caretAt({ top: 0, bottom: 0, height: 0 }, { container: editor, offset: 0 }); + scrollCaretIntoView(editor); + expect(page.scrollTop).toBe(0); + expect(textArea.scrollTop).toBe(0); + }); + + test("the walk stops at the composer: a caret outside the page's view leaves the page alone", () => { + const { page, editor } = embeddedComposer({ textAreaOverflows: false }); + caretAt({ top: 40, bottom: 60, height: 20 }); + scrollCaretIntoView(editor); + expect(page.scrollTop).toBe(0); + }); + + test("a caret on an empty last line is measured by its
and scrolled into view", () => { + const { page, textArea, editor } = embeddedComposer({ textAreaOverflows: true }); + // "text\n" renders as text, a line break, and the padding break that gives + // the empty last line its box. The caret sits before that padding break. + editor.append("text", document.createElement("br"), document.createElement("br")); + const { line } = caretAt( + { top: 0, bottom: 0, height: 0 }, + { container: editor, offset: 2 }, + { top: 530, bottom: 550, height: 20 }, + ); + scrollCaretIntoView(editor); + expect(line.selectedNode).toBe(editor.childNodes[2] as Node); + expect(textArea.scrollTop).toBe(40); + expect(page.scrollTop).toBe(0); + }); + + test("overflowing text scrolls the caret into view inside the composer's own text area", () => { + const { page, textArea, editor } = embeddedComposer({ textAreaOverflows: true }); + caretAt({ top: 530, bottom: 550, height: 20 }); + scrollCaretIntoView(editor); + expect(textArea.scrollTop).toBe(40); + expect(page.scrollTop).toBe(0); + }); + + test("a selection is measured at its focus end, not across its full extent", () => { + const { editor } = embeddedComposer({ textAreaOverflows: true }); + editor.append("first line", "last line"); + const focusNode = editor.childNodes[1] as Node; + const { caret } = caretAt( + { top: 530, bottom: 550, height: 20 }, + { container: focusNode, offset: 4 }, + ); + scrollCaretIntoView(editor); + expect(caret.startContainer).toBe(focusNode); + expect(caret.startOffset).toBe(4); + }); +}); + +describe("scrollIntoViewWithin", () => { + afterEach(() => { + document.body.innerHTML = ""; + }); + + test("the boundary itself may scroll", () => { + const { textArea, editor } = embeddedComposer({ textAreaOverflows: true }); + scrollIntoViewWithin({ top: 380, bottom: 400, height: 20 } as DOMRect, editor, textArea); + expect(textArea.scrollTop).toBe(-30); + }); + + test("nested scroll areas inside the boundary each scroll their share", () => { + const { page, composer, textArea, editor } = embeddedComposer({ textAreaOverflows: true }); + // The text area sits partly below the scrollable composer's view (400–540). + composer.style.overflowY = "auto"; + place(composer, { top: 400, height: 140, scrollHeight: 300 }); + place(textArea, { top: 520, height: 100, scrollHeight: 400 }); + scrollIntoViewWithin({ top: 700, bottom: 720, height: 20 } as DOMRect, editor, composer); + expect(textArea.scrollTop).toBe(100); + expect(composer.scrollTop).toBe(80); + expect(page.scrollTop).toBe(0); + }); + + test("an element with no scrollable ancestor inside the boundary scrolls nothing", () => { + const { page, editor } = embeddedComposer({ textAreaOverflows: false }); + scrollIntoViewWithin({ top: 0, bottom: 20, height: 20 } as DOMRect, editor, editor); + expect(page.scrollTop).toBe(0); + }); +}); + +// --------------------------------------------------------------------------- +// The ask-user flow's own focus moves +// --------------------------------------------------------------------------- + +const QUESTIONS: AskUserQuestion[] = [ + { question: "Which database?", options: [{ label: "PostgreSQL" }, { label: "SQLite" }] }, +]; + +const AskUserOptions = () => { + const askUser = useComposer((composer) => composer.askUser); + const question = askUser.questions?.[askUser.step]; + if (!question) return null; + return ( + + + {question.options.map((option) => ( + askUser.toggleOption(option.label)} + > + {option.label} + + ))} + + + ); +}; + +// activateAskUser moves focus into the options one frame past the commit. +const flushFrames = () => + act( + () => + new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }), + ); + +describe("ask-user focus moves keep the page in place", () => { + const focus = HTMLElement.prototype.focus; + let calls: { element: HTMLElement; options?: FocusOptions }[] = []; + + beforeEach(() => { + calls = []; + HTMLElement.prototype.focus = function (this: HTMLElement, options?: FocusOptions) { + calls.push({ element: this, options }); + focus.call(this, options); + }; + }); + + afterEach(() => { + HTMLElement.prototype.focus = focus; + }); + + const renderQuestion = async (container?: HTMLElement) => { + render( + {}}> + + + , + { container }, + ); + await flushFrames(); + }; + + const focusesOf = (element: HTMLElement) => calls.filter((call) => call.element === element); + + test("focus moves into the first option without scrolling", async () => { + await renderQuestion(); + const first = screen.getByRole("radio", { name: "PostgreSQL" }); + expect(focusesOf(first).at(-1)?.options).toEqual({ preventScroll: true }); + }); + + test("arrowing between options does not scroll", async () => { + await renderQuestion(); + act( + () => void fireEvent.keyDown(document.activeElement ?? document.body, { key: "ArrowDown" }), + ); + const second = screen.getByRole("radio", { name: "SQLite" }); + expect(document.activeElement).toBe(second); + expect(focusesOf(second).at(-1)?.options).toEqual({ preventScroll: true }); + }); + + test("arrowing to an option outside its list's view scrolls the list, not the page", async () => { + const page = document.createElement("div"); + page.style.overflowY = "auto"; + document.body.append(page); + await renderQuestion(page); + // The list shows one option at a time; the second sits just below its view, + // and the list runs past the page's view, so a boundary that leaked out of + // the question would scroll the page too. + const list = screen.getByRole("radiogroup"); + list.style.overflowY = "auto"; + place(page, { top: 0, height: 420, scrollHeight: 2000 }); + place(list, { top: 400, height: 40, scrollHeight: 80 }); + place(screen.getByRole("radio", { name: "SQLite" }), { top: 440, height: 40 }); + + act( + () => void fireEvent.keyDown(document.activeElement ?? document.body, { key: "ArrowDown" }), + ); + expect(list.scrollTop).toBe(40); + expect(page.scrollTop).toBe(0); + }); + + test("typing to answer moves focus back to the input without scrolling", async () => { + await renderQuestion(); + act(() => void fireEvent.keyDown(document.activeElement ?? document.body, { key: "a" })); + const input = screen.getByRole("textbox", { name: "Message" }); + expect(document.activeElement).toBe(input); + expect(focusesOf(input).at(-1)?.options).toEqual({ preventScroll: true }); + }); +});