Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .fork/customizations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,11 @@
focus. 14 is a deliberate divergence from the drawn 12, and the pills are
12px where the designs draw 10 — the guard pins both so a later
reconciliation against Figma does not quietly "correct" them back.
Overflow on the prompt stays hidden until content clears max-h: the tight
14/16 line box lets Geist ink inflate scrollHeight on a single line, so
upstream's overflow-y: auto painted a scrollbar with nothing to scroll.
data-composer-prompt-scrollable restores auto once scrollHeight clears
the capped max; the wrap latch owns that toggle.
The prompt and its placeholder are set by one rule and must stay that way:
they are two elements stacked exactly on top of each other, and any
disagreement about size or leading puts the caret beside the text it
Expand Down
28 changes: 28 additions & 0 deletions apps/web/src/__fork_guards__/forkComposerShell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import { FORK_MARKER_ATTRIBUTE, FORK_MARKER_VALUE } from "../custom/forkMarker";
import {
resolveComposerDensity,
isComposerPromptScrollable,
isPromptHeightWrapped,
nextWrapLatch,
} from "../custom/composerDensity";
Expand Down Expand Up @@ -182,6 +183,22 @@
expect(theme).toMatch(/\[data-testid="composer-editor"\][\s\S]{0,160}min-height:\s*0/u);
});

it("keeps the prompt scrollbar off until content clears max-height", () => {
// The fork's 14/16 line box lets Geist ink overflow by a pixel or two, which
// made upstream's overflow-y: auto paint a thumb on a single unwrapped line.
// Hidden until data-composer-prompt-scrollable; the latch toggles that when
// scrollHeight clears the capped max.
expect(theme).toMatch(
/\[data-testid="composer-editor"\]\s*\{[\s\S]{0,200}overflow-y:\s*hidden/u,
);
expect(theme).toMatch(
/\[data-testid="composer-editor"\]\[data-composer-prompt-scrollable\]\s*\{[\s\S]{0,80}overflow-y:\s*auto/u,
);
const latchHook = readSibling("../custom/useComposerPromptWrapLatch.ts");
expect(latchHook).toContain("COMPOSER_PROMPT_SCROLLABLE_ATTR");
expect(latchHook).toContain("isComposerPromptScrollable");
});

it("squares the send and stop buttons, flattens release send, and reddens stop", () => {
expect(theme).toMatch(/\[data-fork-composer-action\]\s*\{[\s\S]{0,200}border-radius:\s*6px/u);
// Flat (no stage art) is pure white / black icon in dark mode only; light
Expand Down Expand Up @@ -410,7 +427,7 @@
// the tall shell and strand it there — the prompt is non-empty, so the
// release effect never fires, and the observer only ever latches on.
const latchHook = readSibling("../custom/useComposerPromptWrapLatch.ts");
expect(latchHook).toMatch(/setIsPromptWrapped\(false\);\s*\n\s*\}, \[draftKey\]\)/u);

Check failure on line 430 in apps/web/src/__fork_guards__/forkComposerShell.test.ts

View workflow job for this annotation

GitHub Actions / Test

[unit] src/__fork_guards__/forkComposerShell.test.ts > fork guard: fork-composer-shell > clears the latch when the draft changes, not just when the prompt empties

AssertionError: expected 'import { useEffect, useState } from "…' to match /setIsPromptWrapped\(false\);\s*\n\s*…/u - Expected: /setIsPromptWrapped\(false\);\s*\n\s*\}, \[draftKey\]\)/u + Received: "import { useEffect, useState } from \"react\"; import { COMPOSER_PROMPT_SCROLLABLE_ATTR, isComposerPromptScrollable, isPromptHeightWrapped, nextWrapLatch, } from \"./composerDensity\"; export interface ComposerPromptWrapLatch { /** Latched: the prompt has wrapped and has not been cleared since. */ isPromptWrapped: boolean; /** Ref callback for the element wrapping the prompt editor. */ attachPromptElement: (element: HTMLDivElement | null) => void; } function syncComposerPromptScrollable(editable: HTMLElement): void { const maxHeightPx = Number.parseFloat(window.getComputedStyle(editable).maxHeight); editable.toggleAttribute( COMPOSER_PROMPT_SCROLLABLE_ATTR, isComposerPromptScrollable({ scrollHeightPx: editable.scrollHeight, maxHeightPx, }), ); } /** * Watches the prompt editor and reports whether it has wrapped past one line — * the signal that grows the slim shell into the tall one. * * This is the impure half of the density decision, and it lives here rather * than inline in ChatComposer for two reasons: it is fork-authored behaviour * that upstream has no counterpart for, and keeping it beside the pure * `nextWrapLatch` it drives means the whole rule reads in one place. * * Two things about it are load-bearing: * * The measurement is observed, not derived. Lexical reflows the prompt without * a React render, so a height read during render is stale by construction. * * The result is latched, and the caller must not un-latch it. The two shells * give the prompt different widths — roughly 460px slim against 736px tall — so * a prompt between them un-wraps the instant the flip lands, re-wraps when it * flips back, and oscillates until it pins the main thread. */ export function useComposerPromptWrapLatch( prompt: string, /** * The draft the prompt belongs to. ChatComposer is not keyed by thread, so it * persists across thread and draft switches and so does this latch — and the * latch only ever clears on an *empty* prompt. Without this, leaving a thread * whose prompt had wrapped and arriving at one with a short saved draft would * render the new thread in the tall shell and strand it there until the user * emptied the field. \"Only an empty prompt turns it off\" is a rule about a * single composing session; a different thread is a different session. */ draftKey: string, ): ComposerPromptWrapLatch { const [isPromptWrapped, setIsPromptWrapped] = useState(false); const [promptElement, setPromptElement] = useState<HTMLDivElement | null>(null); useEffect(() => { const editable = promptElement?.querySelector<HTMLElement>('[data-testid=\"composer-editor\"]'); if (!editable) { setIsPromptWrapped(false); return; } const measure = (heightPx: number) => { const lineHeight = Number.parseFloat(window.getComputedStyle(editable).lineHeight); const measuredWrapped = isPromptHeightWrapped(heightPx, lineHeight); setIsPromptWrapped((latched) => nextWrapLatch({ latched, measuredWrapped, isPromptEmpty: editable.textContent?.trim().length === 0, }), ); // Height can clamp at max-h while content keeps growing, so the wrap // measurement alone would miss the transition into a scrollable prompt. syncComposerPromptScrollable(editable); }; const observer = new ResizeObserver((entries) => { const entry = entries[0]; if (!entry) return; measure(entry.contentRect.height); }); // No priming read. `observe()` delivers an initial observation for a // rendered element, so a manual call was always redundant — and it was the // only path using a padding-box metric (`clientHeight`) against a threshold // calibrated for the observer's content-box `contentRect`. The editor picks //
expect(chatComposer).toContain("useComposerPromptWrapLatch(");
expect(chatComposer).toContain("scopedThreadKey(composerDraftTarget)");
});
Expand Down Expand Up @@ -453,4 +470,15 @@
expect(isPromptHeightWrapped(16, Number.NaN)).toBe(false);
expect(isPromptHeightWrapped(64, Number.NaN)).toBe(true);
});

it("does not treat glyph ink overflow as a scrollable prompt", () => {
// A one-line prompt whose scrollHeight is a couple of pixels over its
// client height is noise from the tight line box, not content past max-h.
expect(isComposerPromptScrollable({ scrollHeightPx: 18, maxHeightPx: 200 })).toBe(false);
expect(isComposerPromptScrollable({ scrollHeightPx: 201, maxHeightPx: 200 })).toBe(false);
expect(isComposerPromptScrollable({ scrollHeightPx: 202, maxHeightPx: 200 })).toBe(true);
expect(isComposerPromptScrollable({ scrollHeightPx: 400, maxHeightPx: Number.NaN })).toBe(
false,
);
});
});
31 changes: 31 additions & 0 deletions apps/web/src/custom/composerDensity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,37 @@ export function isPromptHeightWrapped(
return promptHeightPx > lineHeight * 1.5;
}

/**
* Attribute toggled on the prompt editor when its content has actually hit
* `max-height` and needs a scrollbar. See `isComposerPromptScrollable`.
*/
export const COMPOSER_PROMPT_SCROLLABLE_ATTR = "data-composer-prompt-scrollable";

/**
* Whether the prompt editor should expose a vertical scrollbar.
*
* Upstream's `overflow-y: auto` is correct once the editor has hit `max-h-50`,
* but the fork's 14px/16px line box lets Geist ink overflow the line box by a
* pixel or two. That inflates `scrollHeight` above `clientHeight` on a single
* unwrapped line, so `overflow-y: auto` paints a thumb with nothing to scroll.
*
* Compare against the capped max height, not against client height: glyph ink
* on a short prompt is noise; content past the max is the real scroll case.
*/
export function isComposerPromptScrollable(input: {
scrollHeightPx: number;
maxHeightPx: number;
}): boolean {
if (!Number.isFinite(input.maxHeightPx) || input.maxHeightPx <= 0) {
return false;
}
if (!Number.isFinite(input.scrollHeightPx) || input.scrollHeightPx <= 0) {
return false;
}
// 1px of slack absorbs sub-pixel scrollHeight rounding at the max edge.
return input.scrollHeightPx > input.maxHeightPx + 1;
}

/**
* The wrap latch, and the reason it has to exist.
*
Expand Down
41 changes: 37 additions & 4 deletions apps/web/src/custom/useComposerPromptWrapLatch.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { useEffect, useState } from "react";

import { isPromptHeightWrapped, nextWrapLatch } from "./composerDensity";
import {
COMPOSER_PROMPT_SCROLLABLE_ATTR,
isComposerPromptScrollable,
isPromptHeightWrapped,
nextWrapLatch,
} from "./composerDensity";

export interface ComposerPromptWrapLatch {
/** Latched: the prompt has wrapped and has not been cleared since. */
Expand All @@ -9,6 +14,17 @@ export interface ComposerPromptWrapLatch {
attachPromptElement: (element: HTMLDivElement | null) => void;
}

function syncComposerPromptScrollable(editable: HTMLElement): void {
Comment thread
NoahHendrickson marked this conversation as resolved.
Resolved
const maxHeightPx = Number.parseFloat(window.getComputedStyle(editable).maxHeight);
editable.toggleAttribute(
COMPOSER_PROMPT_SCROLLABLE_ATTR,
isComposerPromptScrollable({
scrollHeightPx: editable.scrollHeight,
maxHeightPx,
}),
);
}

/**
* Watches the prompt editor and reports whether it has wrapped past one line —
* the signal that grows the slim shell into the tall one.
Expand Down Expand Up @@ -60,6 +76,9 @@ export function useComposerPromptWrapLatch(
isPromptEmpty: editable.textContent?.trim().length === 0,
}),
);
// Height can clamp at max-h while content keeps growing, so the wrap
// measurement alone would miss the transition into a scrollable prompt.
syncComposerPromptScrollable(editable);
};
const observer = new ResizeObserver((entries) => {
const entry = entries[0];
Expand All @@ -76,23 +95,37 @@ export function useComposerPromptWrapLatch(
observer.observe(editable);
return () => {
observer.disconnect();
editable.removeAttribute(COMPOSER_PROMPT_SCROLLABLE_ATTR);
};
}, [promptElement]);

// Releasing the latch cannot be left to the observer alone: clearing a
// one-line prompt in the tall shell changes no height, so no resize fires and
// the composer would stay tall for the rest of the thread.
//
// The same gap exists for scrollability: pasting past max-h does not change
// the clamped height, so ResizeObserver stays quiet while scrollHeight grows.
useEffect(() => {
if (prompt.trim().length === 0) {
setIsPromptWrapped(false);
}
}, [prompt]);
const editable = promptElement?.querySelector<HTMLElement>('[data-testid="composer-editor"]');
if (editable) {
syncComposerPromptScrollable(editable);
}
}, [prompt, promptElement]);

// A new draft is a new composing session, so the latch starts clean whether
// or not the incoming prompt happens to be empty.
// or not the incoming prompt happens to be empty. Drop scrollability here
// rather than re-measuring: Lexical may still hold the previous draft's
// DOM when draftKey flips, and a sync against that would keep the scrollbar
// on a short incoming draft. The prompt effect re-applies it once content
// catches up.
useEffect(() => {
Comment thread
NoahHendrickson marked this conversation as resolved.
Resolved
setIsPromptWrapped(false);
}, [draftKey]);
const editable = promptElement?.querySelector<HTMLElement>('[data-testid="composer-editor"]');
editable?.removeAttribute(COMPOSER_PROMPT_SCROLLABLE_ATTR);
}, [draftKey, promptElement]);

return { isPromptWrapped, attachPromptElement: setPromptElement };
}
15 changes: 14 additions & 1 deletion apps/web/src/theme.custom.css
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,22 @@
/* The prompt sits on a 16px line box at 14px type. Upstream's min-height holds
the editor open at ~3 lines, which would make the slim shell impossible and
leave the tall one hollow; one line is the drawn height and it grows from
there. 16px is kept below sm so iOS does not zoom the viewport on focus. */
there. 16px is kept below sm so iOS does not zoom the viewport on focus.

Overflow is hidden until the editor has actually hit max-h. Upstream's
overflow-y: auto is correct once content is capped, but Geist ink overflows
the tight 14/16 line box by a pixel or two and that alone made auto paint a
scrollbar on a single unwrapped line. useComposerPromptWrapLatch sets
data-composer-prompt-scrollable when scrollHeight clears the max. */
:root[data-fork="noahhendrickson-t3code"] [data-fork-composer-box] [data-testid="composer-editor"] {
min-height: 0;
overflow-y: hidden;
}

:root[data-fork="noahhendrickson-t3code"]
[data-fork-composer-box]
[data-testid="composer-editor"][data-composer-prompt-scrollable] {
overflow-y: auto;
}

/* The placeholder has to move with the prompt, or the two disagree about both
Expand Down
Loading