Fix wave: dropdown keyboard/focus, appearance resets, inline key editors, content-sized slider
- MenuShell: after placement, focus lands in the search when the menu has one, else the body (tabIndex -1) — the lists' mount-time focus ran during the hidden measuring pass and silently failed, so no dropdown ever opened focused. The body owns the arrow machine for searchless menus: ArrowDown/Up cycle options with wrap (first/last entry from the body, native-select convention), Home/End, Enter activates, preventDefault so arrows never scroll the page. - Appearance: the CSS-var applier only ever setProperty'd — a later write omitting a var left the stale inline value forever (empty background never reset, serif toggle-back looked dead). appliedNamesRef tracks what was set; staleVarNames (pure, exported) returns what to removeProperty. - CommitInput: controlled — after a commit the field snaps to the stored record, so the clamp on a -5 prose size is visible (12), Esc reverts. - Keys: Add-provider dropdown closes on pick; the paste row is inline (label | input | Save | Cancel); existing-row editors go inline with an explicit Cancel beside Save/Delete — folding is visible, not Esc-only. - Graph context: readout is content-sized (tabular-nums) — no fixed-width dead gap when "off"; the dictation strip fills whatever space remains. Gates: tsc clean, 866/866 tests (new staleVarNames suite), lint 0, build ok. Live-verified via Playwright: dropdown focus+arrow cycles+wrap+Enter pick, background set→empty reset, serif round-trip, visible clamp to 12, provider pick closes menu, inline editor + Cancel + Esc fold, slider width 270→277px.
This commit is contained in:
@@ -60,8 +60,12 @@ export function GraphContextControl({
|
||||
className="h-1 w-24 shrink-0 accent-primary"
|
||||
data-testid="graph-context-slider"
|
||||
/>
|
||||
{/* Content-sized (owner, fix wave): the control hugs its value —
|
||||
no fixed-width readout means no dead gap when "off", and the
|
||||
dictation strip in the stats bar's middle always fills whatever
|
||||
space the slider leaves. tabular-nums keeps the digits steady. */}
|
||||
<span
|
||||
className="w-16 shrink-0 tabular-nums"
|
||||
className="shrink-0 tabular-nums"
|
||||
data-testid="graph-context-value"
|
||||
>
|
||||
{value === 0 ? "off" : `${value} tok`}
|
||||
|
||||
@@ -125,6 +125,7 @@ export function MenuShell({
|
||||
}) {
|
||||
const menuRef = useRef<HTMLDivElement>(null);
|
||||
const [placed, setPlaced] = useState<MenuPlacement | null>(null);
|
||||
const focusedOnceRef = useRef(false);
|
||||
|
||||
// Announce open↔close for lockable surfaces (see MENU_OPEN_EVENT).
|
||||
useEffect(() => {
|
||||
@@ -134,6 +135,55 @@ export function MenuShell({
|
||||
};
|
||||
}, []);
|
||||
|
||||
// Focus on open (owner, fix wave): the search menus land focus IN the
|
||||
// search; menus without one land it on the body (tabIndex -1 below) so
|
||||
// the arrow machine has a home. Once only — re-placements (scroll/
|
||||
// resize) must never steal focus back mid-walk. The lists' own mount
|
||||
// focus runs during the hidden measuring pass, where focus() silently
|
||||
// fails (visibility: hidden subtrees are not focusable) — this seam is
|
||||
// the one that actually lands.
|
||||
useEffect(() => {
|
||||
if (placed === null || focusedOnceRef.current) return;
|
||||
focusedOnceRef.current = true;
|
||||
const el = menuRef.current;
|
||||
if (!el) return;
|
||||
const input = el.querySelector<HTMLElement>("input");
|
||||
(input ?? el).focus();
|
||||
}, [placed]);
|
||||
|
||||
// The standard arrow machine for menus WITHOUT a search (owner: when a
|
||||
// dropdown is expanded, the arrow keys cycle the options and never
|
||||
// scroll the screen): ArrowDown/Up move DOM focus among the option
|
||||
// buttons with WRAP, Home/End hit the ends; Enter activates the
|
||||
// focused button natively. Search menus own their arrows inside the
|
||||
// input (their clamped-walk machine, 2026-09-17) — those events never
|
||||
// bubble here, and the input/textarea guard is belt-and-braces.
|
||||
const onBodyKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => {
|
||||
if (e.target instanceof HTMLInputElement || e.target instanceof HTMLTextAreaElement) return;
|
||||
const { key } = e;
|
||||
if (key !== "ArrowDown" && key !== "ArrowUp" && key !== "Home" && key !== "End") return;
|
||||
const buttons = Array.from(
|
||||
menuRef.current?.querySelectorAll<HTMLElement>("button:not([disabled])") ?? [],
|
||||
);
|
||||
if (buttons.length === 0) return;
|
||||
e.preventDefault();
|
||||
if (key === "Home") {
|
||||
buttons[0]?.focus();
|
||||
return;
|
||||
}
|
||||
if (key === "End") {
|
||||
buttons[buttons.length - 1]?.focus();
|
||||
return;
|
||||
}
|
||||
const delta = key === "ArrowDown" ? 1 : -1;
|
||||
const idx = buttons.findIndex((b) => b === document.activeElement);
|
||||
// From the body (nothing focused yet): ArrowDown enters at the FIRST
|
||||
// option, ArrowUp at the last — the native-select convention.
|
||||
const base = idx === -1 ? (delta > 0 ? -1 : 0) : idx;
|
||||
const next = (base + delta + buttons.length) % buttons.length;
|
||||
buttons[next]?.focus();
|
||||
};
|
||||
|
||||
const anchor = () => {
|
||||
if (anchorRef) return anchorRef.current;
|
||||
if (anchorTestid !== undefined) return document.querySelector(`[data-testid="${anchorTestid}"]`);
|
||||
@@ -193,6 +243,8 @@ export function MenuShell({
|
||||
ref={menuRef}
|
||||
data-testid={testid}
|
||||
data-lh-menu=""
|
||||
tabIndex={-1}
|
||||
onKeyDown={onBodyKeyDown}
|
||||
style={{ position: "fixed", zIndex: 60, ...placed.style, maxHeight: placed.maxHeight, overflowY: "auto" }}
|
||||
className={menuClass}
|
||||
>
|
||||
|
||||
@@ -95,6 +95,18 @@ export function appearanceVars(a: ReturnType<typeof resolveAppearance>): Record<
|
||||
return vars;
|
||||
}
|
||||
|
||||
/** Pure: the previously-applied names ABSENT from the next var set —
|
||||
* these get removeProperty so the factory stylesheet value returns.
|
||||
* (Owner repro, fix wave: resets and toggle-backs never reverted because
|
||||
* application only ever SET properties — the removal half was missing.)
|
||||
* Exported for tests. */
|
||||
export function staleVarNames(
|
||||
prevNames: readonly string[],
|
||||
nextVars: Record<string, string>,
|
||||
): string[] {
|
||||
return prevNames.filter((n) => !(n in nextVars));
|
||||
}
|
||||
|
||||
interface AppearanceContextValue {
|
||||
/** Resolved appearance; factory until the record arrives. */
|
||||
appearance: ReturnType<typeof resolveAppearance>;
|
||||
@@ -118,11 +130,25 @@ export function AppearanceProvider({ children }: { children: React.ReactNode })
|
||||
// The vars apply on every change — stringified as the change signal.
|
||||
const vars = useMemo(() => appearanceVars(resolveAppearance(delta)), [delta]);
|
||||
const key = useMemo(() => JSON.stringify(vars), [vars]);
|
||||
// REMOVAL is half the job (owner repro, fix wave): a later write that
|
||||
// OMITS a var (empty field = factory, serif toggle back on) must clear
|
||||
// the previously-set inline value or the stale one wins forever —
|
||||
// backgrounds never reset, toggles looked dead. Every name this effect
|
||||
// has ever set is tracked; names absent from the new set are removed so
|
||||
// the factory stylesheet value returns.
|
||||
const appliedNamesRef = useRef<Set<string>>(new Set());
|
||||
useEffect(() => {
|
||||
if (appliedRef.current === key) return;
|
||||
appliedRef.current = key;
|
||||
const root = document.documentElement;
|
||||
for (const [name, value] of Object.entries(vars)) root.style.setProperty(name, value);
|
||||
for (const name of staleVarNames([...appliedNamesRef.current], vars)) {
|
||||
root.style.removeProperty(name);
|
||||
appliedNamesRef.current.delete(name);
|
||||
}
|
||||
for (const [name, value] of Object.entries(vars)) {
|
||||
root.style.setProperty(name, value);
|
||||
appliedNamesRef.current.add(name);
|
||||
}
|
||||
}, [key, vars]);
|
||||
|
||||
// One fetch per app load: the record (sanitized server-side) carries the
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
* control together) and the control itself locks — the row owns the dim.
|
||||
*/
|
||||
|
||||
import { useState } from "react";
|
||||
import { cn } from "@/lib/utils";
|
||||
|
||||
/** A section block: pronounced mono heading + content. */
|
||||
@@ -165,7 +166,10 @@ export function CommitInput({
|
||||
testid,
|
||||
type = "text",
|
||||
}: {
|
||||
/** "" = unset (the factory default). */
|
||||
/** "" = unset (the factory default). Mirrors the STORED record —
|
||||
* after a clamped/normalized commit this flows back and the field
|
||||
* snaps to the stored truth (owner repro: a negative size kept
|
||||
* displaying "-5" while 12 was stored). */
|
||||
value: string;
|
||||
placeholder?: string;
|
||||
onCommit: (next: string) => void;
|
||||
@@ -173,21 +177,32 @@ export function CommitInput({
|
||||
testid?: string;
|
||||
type?: "text" | "number";
|
||||
}) {
|
||||
// The draft exists only while editing; a blur shows the record's value
|
||||
// again — commit-on-blur/Enter (owner ruling), the field honest about
|
||||
// what landed.
|
||||
const [draft, setDraft] = useState(value);
|
||||
const [editing, setEditing] = useState(false);
|
||||
return (
|
||||
<input
|
||||
type={type}
|
||||
defaultValue={value}
|
||||
value={editing ? draft : value}
|
||||
placeholder={placeholder}
|
||||
aria-label={ariaLabel}
|
||||
data-testid={testid}
|
||||
onBlur={(e) => {
|
||||
const next = e.target.value.trim();
|
||||
onChange={(e) => {
|
||||
setEditing(true);
|
||||
setDraft(e.target.value);
|
||||
}}
|
||||
onBlur={() => {
|
||||
if (!editing) return;
|
||||
setEditing(false);
|
||||
const next = draft.trim();
|
||||
if (next !== value) onCommit(next);
|
||||
}}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter") e.currentTarget.blur();
|
||||
if (e.key === "Escape") {
|
||||
e.currentTarget.value = value;
|
||||
setEditing(false);
|
||||
e.currentTarget.blur();
|
||||
}
|
||||
}}
|
||||
|
||||
@@ -114,6 +114,7 @@ function KeyTableRow({
|
||||
onToggle,
|
||||
onSave,
|
||||
onDelete,
|
||||
onCancel,
|
||||
testid,
|
||||
}: {
|
||||
label: string;
|
||||
@@ -124,6 +125,9 @@ function KeyTableRow({
|
||||
onToggle: () => void;
|
||||
onSave: (value: string) => Promise<string | null>;
|
||||
onDelete?: () => Promise<void>;
|
||||
/** Folds the editor (owner, fix wave: the way OUT of a paste prompt
|
||||
* must be visible — Cancel beside Save/Delete, not only Esc). */
|
||||
onCancel: () => void;
|
||||
testid: string;
|
||||
}) {
|
||||
const envOwned = status.source === "env";
|
||||
@@ -141,6 +145,68 @@ function KeyTableRow({
|
||||
setBusy(false);
|
||||
}
|
||||
|
||||
// INLINE editing (owner, fix wave: "type the keys in line — more
|
||||
// table-like"): the expanded row becomes the editor itself — label
|
||||
// left, the paste field filling the middle, Save | Cancel (| Delete)
|
||||
// right — instead of a second line beneath the row.
|
||||
if (expanded && !envOwned) {
|
||||
return (
|
||||
<div data-testid={testid}>
|
||||
<div className="flex items-center gap-2 rounded-sm px-1 py-1.5" data-testid={`${testid}-editor`}>
|
||||
<span className="shrink-0 font-ui text-xs text-foreground">{label}</span>
|
||||
<input
|
||||
type="text"
|
||||
value={value}
|
||||
onChange={(e) => setValue(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter") void save();
|
||||
}}
|
||||
placeholder="Paste key…"
|
||||
aria-label={`${label} key`}
|
||||
data-testid={`${testid}-input`}
|
||||
disabled={busy}
|
||||
autoFocus
|
||||
className="h-7 min-w-0 flex-1 rounded-sm border border-border bg-background px-1.5 font-mono text-xs text-foreground outline-none placeholder:text-muted-foreground/60 focus:border-muted-foreground/50"
|
||||
/>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => void save()}
|
||||
disabled={busy || !value.trim()}
|
||||
data-testid={`${testid}-save`}
|
||||
className="shrink-0 text-xs text-foreground underline-offset-2 hover:underline disabled:text-muted-foreground/40 disabled:no-underline"
|
||||
>
|
||||
Save
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={onCancel}
|
||||
disabled={busy}
|
||||
data-testid={`${testid}-cancel`}
|
||||
className="shrink-0 text-xs text-muted-foreground/70 underline-offset-2 hover:text-foreground hover:underline"
|
||||
>
|
||||
Cancel
|
||||
</button>
|
||||
{status.present && status.source === "file" && onDelete && (
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => void onDelete()}
|
||||
disabled={busy}
|
||||
data-testid={`${testid}-delete`}
|
||||
className="shrink-0 text-xs text-muted-foreground/70 underline-offset-2 hover:text-red-500 hover:underline"
|
||||
>
|
||||
Delete
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
{error && (
|
||||
<p role="alert" data-testid={`${testid}-error`} className="px-1 text-xs text-red-500">
|
||||
{error}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div data-testid={testid} className={envOwned ? "opacity-40" : undefined}>
|
||||
<button
|
||||
@@ -170,43 +236,6 @@ function KeyTableRow({
|
||||
</span>
|
||||
</span>
|
||||
</button>
|
||||
{expanded && !envOwned && (
|
||||
<div className="mt-0.5 mb-1 flex items-center gap-2 pl-6" data-testid={`${testid}-editor`}>
|
||||
<input
|
||||
type="text"
|
||||
value={value}
|
||||
onChange={(e) => setValue(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter") void save();
|
||||
}}
|
||||
placeholder="Paste key…"
|
||||
aria-label={`${label} key`}
|
||||
data-testid={`${testid}-input`}
|
||||
disabled={busy}
|
||||
className="h-7 w-64 rounded-sm border border-border bg-background px-1.5 font-mono text-xs text-foreground outline-none placeholder:text-muted-foreground/60 focus:border-muted-foreground/50"
|
||||
/>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => void save()}
|
||||
disabled={busy || !value.trim()}
|
||||
data-testid={`${testid}-save`}
|
||||
className="text-xs text-foreground underline-offset-2 hover:underline disabled:text-muted-foreground/40 disabled:no-underline"
|
||||
>
|
||||
Save
|
||||
</button>
|
||||
{status.present && status.source === "file" && onDelete && (
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => void onDelete()}
|
||||
disabled={busy}
|
||||
data-testid={`${testid}-delete`}
|
||||
className="text-xs text-muted-foreground/70 underline-offset-2 hover:text-red-500 hover:underline"
|
||||
>
|
||||
Delete
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
{error && (
|
||||
<p role="alert" data-testid={`${testid}-error`} className="pl-6 text-xs text-red-500">
|
||||
{error}
|
||||
@@ -269,7 +298,15 @@ function AddProviderRow({
|
||||
preferredSide="down"
|
||||
menuClass="flex w-72 flex-col rounded-xl border border-border bg-card p-2 text-xs shadow-xl"
|
||||
>
|
||||
<ProviderMenuList providers={addable} onPick={onPick} />
|
||||
{/* Close on pick (owner, fix wave): a chosen provider ends
|
||||
the dropdown — the paste row is the next step. */}
|
||||
<ProviderMenuList
|
||||
providers={addable}
|
||||
onPick={(p) => {
|
||||
setOpen(false);
|
||||
onPick(p);
|
||||
}}
|
||||
/>
|
||||
</MenuShell>
|
||||
)}
|
||||
</div>
|
||||
@@ -367,6 +404,7 @@ export function KeysSection() {
|
||||
if (!err) setOpenRow(null); // fold on success — rest state
|
||||
return err;
|
||||
}}
|
||||
onCancel={() => setOpenRow(null)}
|
||||
onDelete={() => deleteKey(status.id)}
|
||||
testid={`key-${status.id.toLowerCase()}`}
|
||||
/>
|
||||
@@ -381,8 +419,8 @@ export function KeysSection() {
|
||||
/>
|
||||
)}
|
||||
{adding && (
|
||||
<div className="mt-0.5 mb-1 flex items-center gap-2 pl-6" data-testid={`key-${adding.id}-editor`}>
|
||||
<span className="font-ui text-xs text-foreground">{adding.label}</span>
|
||||
<div className="flex items-center gap-2 rounded-sm px-1 py-1.5" data-testid={`key-${adding.id}-editor`}>
|
||||
<span className="shrink-0 font-ui text-xs text-foreground">{adding.label}</span>
|
||||
<AddProviderEditor provider={adding} onSave={(v) => void saveNewProvider(adding, v)} onCancel={() => setAdding(null)} error={addError} />
|
||||
</div>
|
||||
)}
|
||||
@@ -402,6 +440,7 @@ export function KeysSection() {
|
||||
if (!err) setOpenRow(null);
|
||||
return err;
|
||||
}}
|
||||
onCancel={() => setOpenRow(null)}
|
||||
onDelete={() => deleteKey(status.id)}
|
||||
testid={`key-${status.id.toLowerCase()}`}
|
||||
/>
|
||||
@@ -452,14 +491,15 @@ function AddProviderEditor({
|
||||
placeholder="Paste key…"
|
||||
aria-label={`${provider.label} key`}
|
||||
data-testid={`key-${provider.id}-input`}
|
||||
className="h-7 w-64 rounded-sm border border-border bg-background px-1.5 font-mono text-xs text-foreground outline-none placeholder:text-muted-foreground/60 focus:border-muted-foreground/50"
|
||||
autoFocus
|
||||
className="h-7 min-w-0 flex-1 rounded-sm border border-border bg-background px-1.5 font-mono text-xs text-foreground outline-none placeholder:text-muted-foreground/60 focus:border-muted-foreground/50"
|
||||
/>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => value.trim() && onSave(value)}
|
||||
disabled={!value.trim()}
|
||||
data-testid={`key-${provider.id}-save`}
|
||||
className="text-xs text-foreground underline-offset-2 hover:underline disabled:text-muted-foreground/40 disabled:no-underline"
|
||||
className="shrink-0 text-xs text-foreground underline-offset-2 hover:underline disabled:text-muted-foreground/40 disabled:no-underline"
|
||||
>
|
||||
Save
|
||||
</button>
|
||||
@@ -467,7 +507,7 @@ function AddProviderEditor({
|
||||
type="button"
|
||||
onClick={onCancel}
|
||||
data-testid={`key-${provider.id}-cancel`}
|
||||
className="text-xs text-muted-foreground/70 underline-offset-2 hover:text-foreground hover:underline"
|
||||
className="shrink-0 text-xs text-muted-foreground/70 underline-offset-2 hover:text-foreground hover:underline"
|
||||
>
|
||||
Cancel
|
||||
</button>
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
|
||||
import assert from "node:assert/strict";
|
||||
import { describe, it } from "node:test";
|
||||
import { appearanceVars, derivedPalette, resolveAppearance } from "../components/settings/appearance.tsx";
|
||||
import { appearanceVars, derivedPalette, resolveAppearance, staleVarNames } from "../components/settings/appearance.tsx";
|
||||
import {
|
||||
composePromptSwitch,
|
||||
composeSearchEngine,
|
||||
@@ -105,6 +105,42 @@ describe("appearanceVars — the mapping the provider applies", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("staleVarNames — the removal half of application (owner repro: resets never reverted)", () => {
|
||||
it("a full delta applied, then factory: every delta-only name goes stale", () => {
|
||||
const fullDelta = appearanceVars(
|
||||
resolveAppearance({
|
||||
background: "#101820",
|
||||
fontFamily: "Georgia",
|
||||
sessionNameSerif: false,
|
||||
mdRendering: { headingWeight: 700, boldColor: "#ff0000", italicColor: "#00ff00" },
|
||||
}),
|
||||
);
|
||||
const applied = Object.keys(fullDelta);
|
||||
const factory = appearanceVars(resolveAppearance(undefined));
|
||||
const stale = staleVarNames(applied, factory);
|
||||
assert.ok(stale.includes("--background"));
|
||||
assert.ok(stale.includes("--font-prose"));
|
||||
assert.ok(stale.includes("--font-title"), "serif toggle back ON must clear the title override");
|
||||
assert.ok(stale.includes("--lh-ink-strong"));
|
||||
assert.ok(stale.includes("--lh-ink-italic"));
|
||||
assert.ok(!stale.includes("--lh-prose-size"), "always-applied tokens never go stale");
|
||||
assert.ok(!stale.includes("--lh-heading-weight"), "always-applied tokens never go stale");
|
||||
});
|
||||
|
||||
it("an empty background commit (null = factory) frees the palette vars", () => {
|
||||
const withBg = appearanceVars(resolveAppearance({ background: "#101010" }));
|
||||
const emptied = appearanceVars(resolveAppearance({ background: null }));
|
||||
const stale = staleVarNames(Object.keys(withBg), emptied);
|
||||
assert.ok(stale.includes("--background"));
|
||||
assert.ok(stale.includes("--card"), "the derived palette follows the background out");
|
||||
assert.equal(emptied["--background"], undefined);
|
||||
});
|
||||
|
||||
it("no previous applications → nothing stale", () => {
|
||||
assert.deepEqual(staleVarNames([], appearanceVars(resolveAppearance(undefined))), []);
|
||||
});
|
||||
});
|
||||
|
||||
describe("agent section compose helpers — the replace-whole PUT body", () => {
|
||||
it("composeTool carries the rest of the section verbatim", () => {
|
||||
const before = { tools: { demo: false }, prompts: { "system-prompt.md": true } };
|
||||
|
||||
Reference in New Issue
Block a user