From f8c177d0505df072fbc0def03bc1534a88c93305 Mon Sep 17 00:00:00 2001 From: Chat-1 Date: Tue, 8 Sep 2026 16:13:50 -0700 Subject: [PATCH 01/64] feat(chat): open the composer card's submenus on hover and focus The provider and model flyouts were click-only, which the card's own comment justified by the Model row's fetch (pi shells out to `pi --list-models`, up to 15s) and by the safe-triangle machinery a hover menu needs. Both are now paid for, so the flyouts follow the desktop convention instead: getting INTO a menu is a click, moving around inside one is not. - Rows open their flyout after a 150ms hover-intent delay, and immediately on keyboard focus (`:focus-visible`, so a click's own toggle is not raced). Click still toggles -- it is the only way in on a touch screen. - Every card row takes the hover, including the rows that open nothing, which therefore close what is open. Outside the card nothing changes: only a click takes the stack down. - `isInSafeTriangle` (workspace_ui, pure and beside `placeFlyout`) is what stops the rows lying between the pointer and an open flyout from stealing it mid-travel. The wedge runs from the pointer's exit point on the owning row to the flyout's near edge, and expires after 400ms so a row reached diagonally can still be opened by parking on it. - The offerable-model fetch now runs at most once per card-open, which is what makes the Model row cheap to pass over; the card's own open already warms it. - Openable rows carry `aria-haspopup` and `aria-expanded`. Verified in Fortress against the live chat: hover-open, the triangle holding across a diagonal trip, a genuine row switch, close-on-inert-row, drift-away leaving the stack up, click-outside closing it, and Tab opening a row's menu. --- .../apps/chat/frontend/src/views/ModelBar.ts | 370 ++++++++++++++---- .../libs/workspace_ui/src/flyout-position.ts | 50 ++- 2 files changed, 342 insertions(+), 78 deletions(-) diff --git a/system/apps/chat/frontend/src/views/ModelBar.ts b/system/apps/chat/frontend/src/views/ModelBar.ts index 8dc7d9314b..ddd8d478d7 100644 --- a/system/apps/chat/frontend/src/views/ModelBar.ts +++ b/system/apps/chat/frontend/src/views/ModelBar.ts @@ -12,6 +12,14 @@ * * The provider row is the one that always renders. A provider is a property of the ACCOUNT, * not of the model, so it survives all three of the states in which there is no model to show. + * + * The card opens on a CLICK of the chip and its side flyouts open on HOVER, which is the + * division every desktop menu makes: getting into a menu is a decision, moving around inside + * one is not. Inside the card the pointer therefore decides what is showing -- every row takes + * the hover, including the rows that open nothing and so close what is open -- and the safe + * triangle (`isInSafeTriangle`) is what keeps the rows lying between the pointer and an open + * flyout from stealing it on the way across. Outside the card nothing changes: only a click + * takes the stack down. */ import m from "mithril"; @@ -24,7 +32,8 @@ import type { ModelIdentity } from "../models/ModelSettings"; import { accountForAgent, getAccounts, getDefaultAccountId, openProviderChooser } from "../models/Providers"; import type { ProviderAccount } from "../models/Providers"; import { startChatOnAccount } from "../shell"; -import { placeFlyout } from "@imbue/workspace-ui/src/flyout-position"; +import { isInSafeTriangle, placeFlyout } from "@imbue/workspace-ui/src/flyout-position"; +import type { FlyoutPoint } from "@imbue/workspace-ui/src/flyout-position"; import { Portal } from "@imbue/workspace-ui/src/portal"; import { hoverTooltipAttrs } from "@imbue/workspace-ui/src/components/hoverTooltip"; import { icon } from "@imbue/workspace-ui/src/components/icons"; @@ -66,6 +75,21 @@ const CARD_MARGIN = 8; * is a `closest` call rather than three element references that can go stale. */ const POPOVER_ATTR = "data-model-popover"; +/** How long the pointer rests on a row before that row's flyout opens. + * + * Shorter than the tooltip's 250ms on purpose: a tooltip is an aside nobody asked for, so it + * waits until it is clearly wanted, while a submenu IS what the pointer came across the card + * for. Long enough that a sweep to a row further along opens nothing on the way. */ +const SUBMENU_HOVER_DELAY_MS = 150; + +/** How long the safe triangle survives once the pointer is inside it. + * + * Crossing the card is a flick and is over well inside this. A pointer still in the wedge + * after it has stopped to read the row it is parked on, so the wedge lets go and that row + * gets to open its own flyout -- otherwise a row reached diagonally could never be opened by + * hover at all. */ +const SAFE_TRIANGLE_GRACE_MS = 400; + /** The slider's filled portion, deepening with effort. */ function effortFillColor(fraction: number): string { return `hsl(152 39% ${Math.round(70 - 40 * fraction)}%)`; @@ -92,6 +116,26 @@ export function ModelBar(): m.Component<{ agentId: string }> { let cardAnchor: DOMRect | null = null; let flyout: "model" | "providers" | null = null; let flyoutRowBottom = 0; + // The open flyout's measured box, remeasured on every redraw it survives, because its + // height follows its content (a filtered model list is shorter). It is the safe triangle's + // base, so a stale one would protect the wrong wedge. + let flyoutRect: DOMRect | null = null; + // The safe triangle's apex: the last point the pointer occupied on the row that opened the + // flyout, i.e. where it set off from. Null when there is nothing to travel to, or once the + // pointer has arrived -- or once it has spent `SAFE_TRIANGLE_GRACE_MS` in the wedge without + // arriving, which the timer beside it decides. + let safeApex: FlyoutPoint | null = null; + let safeApexTimer: number | null = null; + // The row a hover is waiting on, held as the ELEMENT rather than as which-flyout-it-opens so + // that "already waiting on this row" is one identity test -- the rows that open nothing all + // answer `null` to that question and would collide. + let hoverIntentRow: HTMLElement | null = null; + let hoverIntentTimer: number | null = null; + // Whether this card has already fetched its offerable models. The card's own open warms them + // (see the trigger), and with hover-opened flyouts a pointer crossing the Model row would + // otherwise re-run a `pi --list-models` that takes up to 15s. Fresh per card-open is what + // matters -- a /login between two opens still shows up -- so this resets with the card. + let offeredFetchedForCard = false; // The provider rows' own transient state -- an armed "Remove?", an open rename field. // Cleared whenever the flyout or the card closes, so someone who clicked the bin to see // what it did does not come back later to a primed one. @@ -137,10 +181,18 @@ export function ModelBar(): m.Component<{ agentId: string }> { } } + /** Load this agent's offerable models once per card-open. See `offeredFetchedForCard`. */ + function warmOfferedModels(agentId: string): void { + if (offeredFetchedForCard) return; + offeredFetchedForCard = true; + void fetchOfferedModels(agentId); + } + function openCard(trigger: HTMLElement): void { cardAnchor = trigger.getBoundingClientRect(); setFlyout(null); modelQuery = ""; + offeredFetchedForCard = false; } function closeCard(): void { @@ -148,6 +200,7 @@ export function ModelBar(): m.Component<{ agentId: string }> { // A drag that never released (the card can be torn down mid-gesture) would otherwise // still be driving the label and the thumb the next time the card opens. draggingEffortIndex = null; + cancelHoverIntent(); setFlyout(null); } @@ -163,10 +216,125 @@ export function ModelBar(): m.Component<{ agentId: string }> { rowState.renamingId = null; rowState.renameDraft = ""; launchPromptAccountId = null; + // A new flyout is a new box in a new place: nothing has travelled towards it yet, and + // the old one's measurements describe a box that is gone. + clearSafeApex(); + flyoutRect = null; } flyout = next; } + /** Drop a hover that has not opened anything yet. */ + function cancelHoverIntent(): void { + if (hoverIntentTimer !== null) { + window.clearTimeout(hoverIntentTimer); + hoverIntentTimer = null; + } + hoverIntentRow = null; + } + + /** Forget the trip: no wedge, and no clock counting one down. */ + function clearSafeApex(): void { + if (safeApexTimer !== null) { + window.clearTimeout(safeApexTimer); + safeApexTimer = null; + } + safeApex = null; + } + + /** Whether the pointer is on its way to the flyout that is already open, rather than + * changing its mind about which row it wants. + * + * Only ever true while a flyout is up AND the pointer has been on the row that opened it, + * so the wedge cannot outlive the trip it was measured for. */ + function isTravellingToFlyout(point: FlyoutPoint): boolean { + if (flyout === null || safeApex === null || flyoutRect === null || cardAnchor === null) return false; + // Which of the flyout's edges faces the card -- `placeFlyout` puts it on either side. Read + // off the two boxes' centers rather than off the near edges, which overlap by design. + const trailing = flyoutRect.left + flyoutRect.width / 2 > cardLeft(cardAnchor) + css.CARD_WIDTH / 2; + return isInSafeTriangle(point, safeApex, { + edgeX: trailing ? flyoutRect.left : flyoutRect.right, + top: flyoutRect.top, + bottom: flyoutRect.bottom, + }); + } + + /** Show `next` (or nothing) because the pointer or the keyboard settled on its row. */ + function openFlyoutFromRow(next: "model" | "providers" | null, row: HTMLElement, onOpen?: () => void): void { + if (next === flyout) return; + if (next !== null) { + flyoutRowBottom = row.getBoundingClientRect().bottom; + } + setFlyout(next); + if (next !== null) { + modelQuery = ""; + onOpen?.(); + } + } + + /** Every card row's pointer and keyboard handling, in one recipe. + * + * `which` is the flyout the row opens, or null for a row that opens nothing -- and a row + * that opens nothing still takes the hover, closing whatever is open. Within the card the + * pointer decides what is showing; only outside it does a click still have to. + * + * `mousemove` rather than `mouseenter`, for two reasons. It keeps the safe triangle's apex + * on the pointer's actual last position over the owning row instead of on wherever it first + * crossed the edge. And a row entered THROUGH the triangle -- protected, so it opened + * nothing -- gets another chance as soon as the pointer moves off the wedge, which a single + * enter event cannot give it. */ + function hoverRowAttrs(opts: { which: "model" | "providers" | null; onOpen?: () => void }): m.Attributes { + return { + onmousemove: (event: MouseEvent) => { + const row = event.currentTarget as HTMLElement; + const point: FlyoutPoint = { x: event.clientX, y: event.clientY }; + // On the row whose flyout is up: this is the trip's starting point, right up until the + // pointer leaves. Every move here restarts it, so the apex is the true exit point and + // the grace clock only ever runs on a trip that has actually set off. + if (opts.which !== null && opts.which === flyout) { + clearSafeApex(); + safeApex = point; + cancelHoverIntent(); + return; + } + if (opts.which === null && flyout === null) return; + if (isTravellingToFlyout(point)) { + // Started across. Arriving cancels this (the flyout's own `mouseenter`); parking in + // the wedge instead lets it run out, and the row underneath gets its turn. + if (safeApexTimer === null) { + safeApexTimer = window.setTimeout(() => { + safeApexTimer = null; + safeApex = null; + }, SAFE_TRIANGLE_GRACE_MS); + } + return; + } + // Already counting down on this very row; restarting the clock would mean a pointer + // that keeps twitching never opens anything. + if (hoverIntentRow === row) return; + cancelHoverIntent(); + hoverIntentRow = row; + hoverIntentTimer = window.setTimeout(() => { + hoverIntentTimer = null; + hoverIntentRow = null; + openFlyoutFromRow(opts.which, row, opts.onOpen); + m.redraw(); + }, SUBMENU_HOVER_DELAY_MS); + }, + // Keyboard focus opens immediately -- there is no aiming to wait for, and a tab that has + // landed on a row is as deliberate as an intent delay could ever prove. `focusin`, which + // bubbles, so a row whose focusable part is a child (the effort slider) is covered too. + // `:focus-visible` keeps a mouse click out of this path: it focuses the row as well, and + // opening from here would race the click's own toggle. + onfocusin: (event: FocusEvent) => { + const focused = event.target as HTMLElement | null; + if (focused === null || !focused.matches(":focus-visible")) return; + cancelHoverIntent(); + openFlyoutFromRow(opts.which, event.currentTarget as HTMLElement, opts.onOpen); + }, + }; + } + /** A click outside the card, its flyout and its trigger closes the whole stack -- and only * a click does; a pointer that merely drifts off leaves everything up. * @@ -207,20 +375,28 @@ export function ModelBar(): m.Component<{ agentId: string }> { class: opts.openable ? css.ROW : css.ROW_INERT, // A stable hook so a test can address a row by what it is rather than by its classes. "data-card-row": opts.which, + // The row is a disclosure, and a hover menu has to say so out loud: the chevron is the + // only other clue, and it is decoration to a screen reader. + "aria-haspopup": opts.openable ? "true" : undefined, + "aria-expanded": opts.openable ? (flyout === opts.which ? "true" : "false") : undefined, ...tooltipAttrs(opts.tooltip), - // CLICK, not hover. Opening the model flyout fetches this agent's offerable models, - // which for pi shells out to `pi --list-models` (up to 15s) and for codex connects to - // its daemon -- on hover that would fire on every pointer sweep across the card. - // Clicking also spares us the safe-triangle hover-aim machinery a hover menu needs. + // HOVER opens these, after `SUBMENU_HOVER_DELAY_MS`, with the safe triangle covering + // the trip across the card -- see `hoverRowAttrs`. + // + // What used to make that too expensive was the fetch behind the Model row: pi shells + // out to `pi --list-models` (up to 15s) and codex connects to its daemon, and on hover + // that fired on every pointer sweep. It no longer can -- `warmOfferedModels` runs at + // most once per card-open, and the card's own open already warms it. + ...(opts.openable + ? hoverRowAttrs({ which: opts.which, onOpen: opts.onOpen }) + : hoverRowAttrs({ which: null })), + // Click still toggles, and is the only way in on a touch screen, where there is no + // hover to intend anything with. onclick: (event: MouseEvent) => { if (!opts.openable) return; - flyoutRowBottom = (event.currentTarget as HTMLElement).getBoundingClientRect().bottom; + cancelHoverIntent(); const opening = flyout !== opts.which; - setFlyout(opening ? opts.which : null); - if (opening) { - modelQuery = ""; - opts.onOpen?.(); - } + openFlyoutFromRow(opening ? opts.which : null, event.currentTarget as HTMLElement, opts.onOpen); }, }, [ @@ -276,46 +452,57 @@ export function ModelBar(): m.Component<{ agentId: string }> { // -- see 2 above); mid-drag from the position, which indexes `shown` by construction // because the input's own min/max are its bounds. const level = draggingEffortIndex === null ? (opts.current ?? shown[committed].level) : shown[position].level; - return m("div", { class: css.ROW_STATIC, "data-card-row": "effort", ...tooltipAttrs(opts.tooltip) }, [ - m("span", { class: css.ROW_LABEL }, "Effort"), - m("span", { class: css.ROW_VALUE_STATIC }, [ - m("span", { class: css.EFFORT_VALUE }, capitalizeEffort(level)), - m("span", { class: css.SLIDER_WRAP }, [ - // A dot at each level: without them the slider is a bare line and the levels it can - // land on are guesswork. - m( - "span", - { class: css.SLIDER_TICKS }, - shown.map((effort) => m("span", { key: effort.level, class: css.SLIDER_TICK })), - ), - m("input", { - type: "range", - "aria-label": "Reasoning effort", - class: css.SLIDER, - min: 0, - max: shown.length - 1, - step: 1, - disabled: !opts.interactive, - // Mithril re-asserts `value` on every redraw, which would snap the thumb back - // under the pointer mid-drag on any harness that does not move the chip - // optimistically -- codex is exactly that. Holding the dragged index locally and - // clearing it on release keeps the thumb where the finger is. - value: position, - style: - `background: linear-gradient(to right, ${effortFillColor(pct / 100)} ${pct}%, ` + - `var(--color-fill-active) ${pct}%)`, - oninput: (event: Event) => { - draggingEffortIndex = Number((event.target as HTMLInputElement).value); - }, - onchange: (event: Event) => { - const picked = shown[Number((event.target as HTMLInputElement).value)]; - draggingEffortIndex = null; - if (picked !== undefined) opts.onPick(picked.level); - }, - }), + return m( + "div", + { + class: css.ROW_STATIC, + "data-card-row": "effort", + ...tooltipAttrs(opts.tooltip), + // Reaching for the slider is leaving the flyout behind, so it goes away -- the same + // rule every row in the card follows. + ...hoverRowAttrs({ which: null }), + }, + [ + m("span", { class: css.ROW_LABEL }, "Effort"), + m("span", { class: css.ROW_VALUE_STATIC }, [ + m("span", { class: css.EFFORT_VALUE }, capitalizeEffort(level)), + m("span", { class: css.SLIDER_WRAP }, [ + // A dot at each level: without them the slider is a bare line and the levels it can + // land on are guesswork. + m( + "span", + { class: css.SLIDER_TICKS }, + shown.map((effort) => m("span", { key: effort.level, class: css.SLIDER_TICK })), + ), + m("input", { + type: "range", + "aria-label": "Reasoning effort", + class: css.SLIDER, + min: 0, + max: shown.length - 1, + step: 1, + disabled: !opts.interactive, + // Mithril re-asserts `value` on every redraw, which would snap the thumb back + // under the pointer mid-drag on any harness that does not move the chip + // optimistically -- codex is exactly that. Holding the dragged index locally and + // clearing it on release keeps the thumb where the finger is. + value: position, + style: + `background: linear-gradient(to right, ${effortFillColor(pct / 100)} ${pct}%, ` + + `var(--color-fill-active) ${pct}%)`, + oninput: (event: Event) => { + draggingEffortIndex = Number((event.target as HTMLInputElement).value); + }, + onchange: (event: Event) => { + const picked = shown[Number((event.target as HTMLInputElement).value)]; + draggingEffortIndex = null; + if (picked !== undefined) opts.onPick(picked.level); + }, + }), + ]), ]), - ]), - ]); + ], + ); } /** Fast mode: a switch. @@ -329,34 +516,43 @@ export function ModelBar(): m.Component<{ agentId: string }> { tooltip: string | null; onToggle: () => void; }): m.Vnode { - return m("div", { class: css.ROW_STATIC, ...tooltipAttrs(opts.tooltip) }, [ - m("span", { class: css.ROW_LABEL }, "Fast Mode"), - m( - "span", - { class: css.ROW_VALUE_STATIC }, + return m( + "div", + { + class: css.ROW_STATIC, + "data-card-row": "fast", + ...tooltipAttrs(opts.tooltip), + ...hoverRowAttrs({ which: null }), + }, + [ + m("span", { class: css.ROW_LABEL }, "Fast Mode"), m( - "button", - { - type: "button", - role: "switch", - class: `${css.SWITCH} ${opts.on ? css.SWITCH_ON : css.SWITCH_OFF}`, - "aria-label": "Fast Mode", - "aria-checked": opts.on ? "true" : "false", - disabled: !opts.interactive, - onclick: () => { - if (opts.interactive) opts.onToggle(); - }, - }, + "span", + { class: css.ROW_VALUE_STATIC }, m( - "span", - { class: `${css.SWITCH_KNOB} ${opts.on ? css.SWITCH_KNOB_ON : css.SWITCH_KNOB_OFF}` }, - opts.on - ? m("span", { class: css.SWITCH_CHECK }, m.trust(icon("check", { size: 12, strokeWidth: 3.5 }))) - : null, + "button", + { + type: "button", + role: "switch", + class: `${css.SWITCH} ${opts.on ? css.SWITCH_ON : css.SWITCH_OFF}`, + "aria-label": "Fast Mode", + "aria-checked": opts.on ? "true" : "false", + disabled: !opts.interactive, + onclick: () => { + if (opts.interactive) opts.onToggle(); + }, + }, + m( + "span", + { class: `${css.SWITCH_KNOB} ${opts.on ? css.SWITCH_KNOB_ON : css.SWITCH_KNOB_OFF}` }, + opts.on + ? m("span", { class: css.SWITCH_CHECK }, m.trust(icon("check", { size: 12, strokeWidth: 3.5 }))) + : null, + ), ), ), - ), - ]); + ], + ); } /** The card's viewport left, clamped so it cannot hang off either edge. */ @@ -400,12 +596,26 @@ export function ModelBar(): m.Component<{ agentId: string }> { /** The shell every flyout renders into, so both register the same outside-click element. */ function flyoutShell(children: m.Children): m.Vnode { + // The safe triangle's base is this box, and its height follows its contents -- so measure + // on arrival AND on every redraw that changes them (a filtered list is shorter, and a + // triangle pointing at the box's old bottom would guard rows nobody is heading through). + const measure = (flyoutVnode: m.VnodeDOM): void => { + flyoutRect = (flyoutVnode.dom as HTMLElement).getBoundingClientRect(); + }; return m( "div", { class: css.FLYOUT, [POPOVER_ATTR]: "flyout", style: flyoutPlacement(), + oncreate: measure, + onupdate: measure, + // Arrived. The trip is over, so the wedge that protected it closes and the card's rows + // answer the pointer normally again the moment it goes back. + onmouseenter: () => { + cancelHoverIntent(); + clearSafeApex(); + }, }, children, ); @@ -421,6 +631,7 @@ export function ModelBar(): m.Component<{ agentId: string }> { type: "button", class: css.ROW, "data-card-row": "stop-agent", + ...hoverRowAttrs({ which: null }), onclick: (event: MouseEvent) => { event.stopPropagation(); closeCard(); @@ -611,6 +822,9 @@ export function ModelBar(): m.Component<{ agentId: string }> { onremove() { document.removeEventListener("mousedown", handleOutsideMousedown); + // A pending hover would otherwise fire into a torn-down component and redraw it. + cancelHoverIntent(); + clearSafeApex(); }, view(vnode) { @@ -661,9 +875,11 @@ export function ModelBar(): m.Component<{ agentId: string }> { openCard(event.currentTarget as HTMLElement); // Warm the model list the moment the CARD opens, not when the flyout does: the // fetch is the slow part (pi shells out to `pi --list-models`), and by the time a - // pointer has crossed the card it is usually already back. + // pointer has crossed the card it is usually already back. With hover-opened + // flyouts this is what makes the Model row cheap to pass over -- by the time the + // hover lands, the list is warm and its own request is a no-op. if (catalog?.picker_mode === "search" || catalog?.picker_mode === "dynamic") { - void fetchOfferedModels(agentId); + warmOfferedModels(agentId); } }, }, @@ -721,7 +937,7 @@ export function ModelBar(): m.Component<{ agentId: string }> { openable: interactive, tooltip: readOnlyTooltip, onOpen: () => { - if (searchable || dynamic) void fetchOfferedModels(agentId); + if (searchable || dynamic) warmOfferedModels(agentId); }, }) : null, diff --git a/system/libs/workspace_ui/src/flyout-position.ts b/system/libs/workspace_ui/src/flyout-position.ts index 8dff8c6772..c6e5a0dcde 100644 --- a/system/libs/workspace_ui/src/flyout-position.ts +++ b/system/libs/workspace_ui/src/flyout-position.ts @@ -1,5 +1,6 @@ /** - * Pure geometry for the combo card's side flyout. + * Pure geometry for the combo card's side flyout: where it sits (`placeFlyout`), and the + * wedge a pointer on its way to it is allowed to cross (`isInSafeTriangle`). * * The flyout's BASE sits level with the row that opened it and the list grows UPWARD. That is * not the ordinary top-align-and-cap-downward rule, and the reason is that this card @@ -40,6 +41,53 @@ export interface FlyoutPlacement { side: "trailing" | "leading"; } +/** A viewport point -- where the pointer is, or where it was. */ +export interface FlyoutPoint { + x: number; + y: number; +} + +/** The flyout edge a pointer travelling towards it must cross: the side FACING the card, + * and that side's full vertical span. */ +export interface SafeTriangleBase { + edgeX: number; + top: number; + bottom: number; +} + +/** + * The safe triangle: is `point` inside the wedge between `apex` and the open flyout's near edge? + * + * A hover menu has one hard problem. The flyout opens beside the card, so the pointer has to + * travel diagonally to reach it -- and on the way it crosses the card's OTHER rows, each of + * which would otherwise take the hover and replace the flyout being aimed at. Waiting longer + * before switching does not fix it: the pointer is genuinely resting on those rows. + * + * What tells travel apart from a change of mind is direction, and the triangle is direction + * made testable. Its apex is the last point the pointer occupied on the row that opened the + * flyout; its base is the flyout's near edge. Every path from that point to that edge stays + * inside it, and a pointer heading anywhere else leaves it almost at once. + * + * Kept here beside `placeFlyout`, and pure for the same reason: the caller measures. + */ +export function isInSafeTriangle(point: FlyoutPoint, apex: FlyoutPoint, base: SafeTriangleBase): boolean { + const vertices: readonly FlyoutPoint[] = [apex, { x: base.edgeX, y: base.top }, { x: base.edgeX, y: base.bottom }]; + // Inside iff `point` sits on the same side of all three edges, walked in order -- so the + // cross products never disagree in sign. A degenerate triangle (an apex already on the + // edge, or a flyout of no height) contains only its own line, which reads as "not + // travelling" and simply leaves the rows unprotected. + let anyPositive = false; + let anyNegative = false; + for (let index = 0; index < vertices.length; index++) { + const from = vertices[index]; + const to = vertices[(index + 1) % vertices.length]; + const cross = (to.x - from.x) * (point.y - from.y) - (to.y - from.y) * (point.x - from.x); + if (cross > 0) anyPositive = true; + if (cross < 0) anyNegative = true; + } + return !(anyPositive && anyNegative); +} + export function placeFlyout(input: FlyoutPlacementInput): FlyoutPlacement { const { cardLeft, cardWidth, rowBottom, flyoutWidth, maxFlyoutHeight } = input; const { viewportWidth, viewportHeight, margin, overlap } = input; From 1c7df08772cbc27d26dbe1a04a776dd8b765a068 Mon Sep 17 00:00:00 2001 From: Chat-1 Date: Tue, 8 Sep 2026 16:58:25 -0700 Subject: [PATCH 02/64] feat(chat): align a flyout's first row with the row that opened it Two changes to how the composer card's flyouts behave, both consequences of their now opening on hover. 1. A flyout closes when the pointer leaves the card-and-flyout stack. A menu summoned by hover has to be dismissed by hover, or it hangs over the transcript until something is clicked -- which is the very thing a hover menu is meant to spare the user. The CARD still takes a click to dismiss, because a click is what opened it. `SUBMENU_LEAVE_DELAY_MS` forgives the seam between the two boxes, which fires a leave before the matching enter. 2. A flyout's FIRST ROW now lines up with the row that opened it, instead of its base standing on that row's bottom edge. `placeFlyout` takes `rowTop` plus the content height and returns a `top`; it slides the box UP when the alignment would push it off the bottom, by exactly as much as it takes to fit and no further. Only a list too tall for the window at all is capped, and then it scrolls -- so the original reason for growing upward (a thousand-model catalog must not be squeezed into the space below a low row) still holds, by sliding rather than by anchoring. The offset between a box and its first row is its border AND its padding. `FLYOUT_PADDING` is both; counting only the padding lands every flyout a pixel low, which is how this was found. `flyoutContentHeight` derives the wanted height from the row height that already defines the ten-row cap, so the two cannot drift apart. Verified in Fortress against the live chat: the Provider and Model menus each land 0.0px off their row, a 420px-tall window slides the menu wholly on screen without shrinking it, leaving the stack closes the menu, crossing the seam does not, and a click outside still takes everything down. --- .../apps/chat/frontend/src/views/ModelBar.ts | 100 ++++++++++++++---- .../frontend/src/views/modelCardStyles.ts | 24 ++++- .../workspace_ui/src/flyout-position.test.ts | 89 ++++++++++++---- .../libs/workspace_ui/src/flyout-position.ts | 58 ++++++---- 4 files changed, 212 insertions(+), 59 deletions(-) diff --git a/system/apps/chat/frontend/src/views/ModelBar.ts b/system/apps/chat/frontend/src/views/ModelBar.ts index ddd8d478d7..454ce80ab5 100644 --- a/system/apps/chat/frontend/src/views/ModelBar.ts +++ b/system/apps/chat/frontend/src/views/ModelBar.ts @@ -15,11 +15,15 @@ * * The card opens on a CLICK of the chip and its side flyouts open on HOVER, which is the * division every desktop menu makes: getting into a menu is a decision, moving around inside - * one is not. Inside the card the pointer therefore decides what is showing -- every row takes - * the hover, including the rows that open nothing and so close what is open -- and the safe - * triangle (`isInSafeTriangle`) is what keeps the rows lying between the pointer and an open - * flyout from stealing it on the way across. Outside the card nothing changes: only a click - * takes the stack down. + * one is not. Each half is then dismissed the way it was summoned, and that is the whole rule: + * + * - The FLYOUTS follow the pointer. Every row takes the hover, including the rows that open + * nothing and so close what is open; the safe triangle (`isInSafeTriangle`) keeps the rows + * lying between the pointer and an open flyout from stealing it on the way across; and a + * pointer that leaves the card-and-flyout stack altogether closes the flyout behind it. + * - The CARD follows the click. Drifting off it leaves it standing, because nothing about + * where the pointer went says the choice it was opened to make has been abandoned. A click + * outside takes the whole stack down. */ import m from "mithril"; @@ -90,6 +94,13 @@ const SUBMENU_HOVER_DELAY_MS = 150; * hover at all. */ const SAFE_TRIANGLE_GRACE_MS = 400; +/** How long an open flyout survives the pointer leaving the card-and-flyout stack. + * + * Long enough to forgive the seam between the two boxes and a corner clipped on the way + * across, short enough that a flyout does not sit over the transcript once the pointer has + * gone somewhere else entirely. */ +const SUBMENU_LEAVE_DELAY_MS = 220; + /** The slider's filled portion, deepening with effort. */ function effortFillColor(fraction: number): string { return `hsl(152 39% ${Math.round(70 - 40 * fraction)}%)`; @@ -115,7 +126,9 @@ export function ModelBar(): m.Component<{ agentId: string }> { // laid out by their parent -- see `openCard`. let cardAnchor: DOMRect | null = null; let flyout: "model" | "providers" | null = null; - let flyoutRowBottom = 0; + // Viewport y of the TOP of the row the open flyout belongs to: the line its first row is + // drawn against. + let flyoutRowTop = 0; // The open flyout's measured box, remeasured on every redraw it survives, because its // height follows its content (a filtered model list is shorter). It is the safe triangle's // base, so a stale one would protect the wrong wedge. @@ -131,6 +144,10 @@ export function ModelBar(): m.Component<{ agentId: string }> { // answer `null` to that question and would collide. let hoverIntentRow: HTMLElement | null = null; let hoverIntentTimer: number | null = null; + // Counting down to closing the flyout because the pointer has left the stack. The card and + // the flyout are two separate boxes, so crossing between them fires a leave before the + // matching enter -- the delay is what stops that seam reading as a departure. + let stackLeaveTimer: number | null = null; // Whether this card has already fetched its offerable models. The card's own open warms them // (see the trigger), and with hover-opened flyouts a pointer crossing the Model row would // otherwise re-run a `pi --list-models` that takes up to 15s. Fresh per card-open is what @@ -201,6 +218,7 @@ export function ModelBar(): m.Component<{ agentId: string }> { // still be driving the label and the thumb the next time the card opens. draggingEffortIndex = null; cancelHoverIntent(); + cancelStackLeave(); setFlyout(null); } @@ -233,6 +251,31 @@ export function ModelBar(): m.Component<{ agentId: string }> { hoverIntentRow = null; } + /** The pointer is back inside the stack (or the stack is gone): nothing to close. */ + function cancelStackLeave(): void { + if (stackLeaveTimer !== null) { + window.clearTimeout(stackLeaveTimer); + stackLeaveTimer = null; + } + } + + /** The pointer has left the card or the flyout. + * + * A flyout opened by hover has to close when the hover ends -- otherwise it hangs over the + * transcript until something is clicked, which is exactly what a hover menu is supposed to + * spare the user. The CARD is a different matter: it was opened by a click, so it takes a + * click to dismiss, and the pointer wandering off does not count. */ + function scheduleStackLeave(): void { + if (flyout === null) return; + cancelStackLeave(); + stackLeaveTimer = window.setTimeout(() => { + stackLeaveTimer = null; + cancelHoverIntent(); + setFlyout(null); + m.redraw(); + }, SUBMENU_LEAVE_DELAY_MS); + } + /** Forget the trip: no wedge, and no clock counting one down. */ function clearSafeApex(): void { if (safeApexTimer !== null) { @@ -263,7 +306,7 @@ export function ModelBar(): m.Component<{ agentId: string }> { function openFlyoutFromRow(next: "model" | "providers" | null, row: HTMLElement, onOpen?: () => void): void { if (next === flyout) return; if (next !== null) { - flyoutRowBottom = row.getBoundingClientRect().bottom; + flyoutRowTop = row.getBoundingClientRect().top; } setFlyout(next); if (next !== null) { @@ -573,14 +616,17 @@ export function ModelBar(): m.Component<{ agentId: string }> { ); } - /** Where a flyout sits: beside the card, standing on the row that opened it. */ - function flyoutPlacement(): string { + /** Where a flyout sits: beside the card, its first row level with the row that opened it -- + * unless holding that line would push it off the bottom, in which case it slides up. */ + function flyoutPlacement(rowCount: number, hasSearchField: boolean): string { const anchor = cardAnchor; if (anchor === null) return ""; const placed = placeFlyout({ cardLeft: cardLeft(anchor), cardWidth: css.CARD_WIDTH, - rowBottom: flyoutRowBottom, + rowTop: flyoutRowTop, + flyoutPadding: css.FLYOUT_PADDING, + contentHeight: css.flyoutContentHeight(rowCount, hasSearchField), flyoutWidth: css.FLYOUT_WIDTH, maxFlyoutHeight: css.FLYOUT_MAX_HEIGHT, viewportWidth: window.innerWidth, @@ -589,13 +635,16 @@ export function ModelBar(): m.Component<{ agentId: string }> { overlap: css.FLYOUT_OVERLAP, }); return ( - `left: ${placed.left}px; bottom: ${placed.bottom}px; ` + + `left: ${placed.left}px; top: ${placed.top}px; ` + `width: ${css.FLYOUT_WIDTH}px; max-height: ${placed.maxHeight}px;` ); } - /** The shell every flyout renders into, so both register the same outside-click element. */ - function flyoutShell(children: m.Children): m.Vnode { + /** The shell every flyout renders into, so both register the same outside-click element. + * + * `rowCount` and `hasSearchField` are what the placement needs to know whether the box can + * hold its alignment -- the caller counts, because only it knows what it is about to draw. */ + function flyoutShell(rowCount: number, hasSearchField: boolean, children: m.Children): m.Vnode { // The safe triangle's base is this box, and its height follows its contents -- so measure // on arrival AND on every redraw that changes them (a filtered list is shorter, and a // triangle pointing at the box's old bottom would guard rows nobody is heading through). @@ -607,15 +656,17 @@ export function ModelBar(): m.Component<{ agentId: string }> { { class: css.FLYOUT, [POPOVER_ATTR]: "flyout", - style: flyoutPlacement(), + style: flyoutPlacement(rowCount, hasSearchField), oncreate: measure, onupdate: measure, // Arrived. The trip is over, so the wedge that protected it closes and the card's rows // answer the pointer normally again the moment it goes back. onmouseenter: () => { + cancelStackLeave(); cancelHoverIntent(); clearSafeApex(); }, + onmouseleave: scheduleStackLeave, }, children, ); @@ -693,7 +744,9 @@ export function ModelBar(): m.Component<{ agentId: string }> { const rows = getAccounts(); const defaultId = getDefaultAccountId(); const prompted = rows.find((row) => row.id === launchPromptAccountId) ?? null; - return flyoutShell([ + // The account rows (or the one line standing in for them when there are none), plus the + // "+ Add a provider" row under them. The launch prompt is a dialog on top, not a row. + return flyoutShell(Math.max(1, rows.length) + 1, false, [ // Built as one list rather than with a conditional hole beside it: mithril refuses a // fragment that mixes keyed vnodes with a null, and every row here is keyed. m( @@ -753,7 +806,10 @@ export function ModelBar(): m.Component<{ agentId: string }> { const filtered = query === "" ? all : all.filter((option) => option.label.toLowerCase().includes(query)); const visible = filtered.slice(0, MODEL_SEARCH_CAP); const loading = (searchable || dynamic) && (offeredLoading || !offeredLoaded); - return flyoutShell([ + const hasSearchField = searchable || all.length > 8; + // Loading and empty each draw a single line where the list would be. + const rowCount = loading || visible.length === 0 ? 1 : visible.length; + return flyoutShell(rowCount, hasSearchField, [ // One list or the other, never a hole beside keyed rows -- mithril refuses a fragment // that mixes the two, and it throws during the DOM diff rather than at build time. m( @@ -790,12 +846,13 @@ export function ModelBar(): m.Component<{ agentId: string }> { ); }), ), - // BELOW the list, not above it: the flyout is anchored at its base and grows upward, so - // the bottom is the edge that stays put next to the row you came from. + // BELOW the list. A long catalog's flyout is the one that slides down to the bottom of + // the window, so its foot is the edge nearest the composer the pointer came from -- and + // the field stays put there while the list scrolls above it. // // The shared input recipe, with the magnifier laid over its left padding: the field owns // its own frame and focus ring, so nothing here re-styles either. - searchable || all.length > 8 + hasSearchField ? m("div", { class: css.SEARCH_WRAP }, [ m("span", { class: css.SEARCH_ICON }, m.trust(icon("search", { size: 13 }))), m("input", { @@ -824,6 +881,7 @@ export function ModelBar(): m.Component<{ agentId: string }> { document.removeEventListener("mousedown", handleOutsideMousedown); // A pending hover would otherwise fire into a torn-down component and redraw it. cancelHoverIntent(); + cancelStackLeave(); clearSafeApex(); }, @@ -916,6 +974,10 @@ export function ModelBar(): m.Component<{ agentId: string }> { class: css.CARD, [POPOVER_ATTR]: "card", style: cardPlacement(cardAnchor), + // The other half of the stack, for the same leave rule: moving between the card and + // its flyout is not leaving, but moving off both of them is. + onmouseenter: cancelStackLeave, + onmouseleave: scheduleStackLeave, }, m("div", { class: css.CARD_INNER }, [ menuRow({ diff --git a/system/apps/chat/frontend/src/views/modelCardStyles.ts b/system/apps/chat/frontend/src/views/modelCardStyles.ts index 76fb5ec204..13733d311d 100644 --- a/system/apps/chat/frontend/src/views/modelCardStyles.ts +++ b/system/apps/chat/frontend/src/views/modelCardStyles.ts @@ -28,8 +28,28 @@ export const FLYOUT_OVERLAP = 4; * guessed so it stays true if the row height changes. */ const FLYOUT_ROW_HEIGHT = 32; const FLYOUT_VISIBLE_ROWS = 10; -/** Rows, plus the search field standing under them, plus the shell's own padding. */ -export const FLYOUT_MAX_HEIGHT = FLYOUT_ROW_HEIGHT * FLYOUT_VISIBLE_ROWS + 44; +/** The distance from a flyout's outer top edge to the top of its first row: `menuCardClass` + * gives it a 1px border AND `py-1`, and both sit above the row. The flyout is placed by that + * first ROW rather than by the box around it, so the placement backs off by exactly this -- + * miss the border and every flyout lands a pixel low. */ +const FLYOUT_BORDER = 1; +const FLYOUT_INNER_PADDING = 4; +export const FLYOUT_PADDING = FLYOUT_BORDER + FLYOUT_INNER_PADDING; +/** `SEARCH_WRAP`'s `mt-1.5` plus `SEARCH_INPUT_EXTRA`'s `h-8`. */ +const SEARCH_FIELD_HEIGHT = 6 + 32; + +/** How tall a flyout of `rowCount` rows wants to be, measured the way the browser measures a + * bordered box: both borders and both paddings, which is what `2 * FLYOUT_PADDING` is. + * + * The placement slides the box up when this will not fit below the row it belongs to, so the + * arithmetic has to match what the DOM actually lays out -- it is the same row height and the + * same chrome the cap below is built from, and both move together if either changes. */ +export function flyoutContentHeight(rowCount: number, hasSearchField: boolean): number { + return 2 * FLYOUT_PADDING + rowCount * FLYOUT_ROW_HEIGHT + (hasSearchField ? SEARCH_FIELD_HEIGHT : 0); +} + +/** Ten rows, plus the search field standing under them, plus the box's own padding. */ +export const FLYOUT_MAX_HEIGHT = flyoutContentHeight(FLYOUT_VISIBLE_ROWS, true); // --- the composer trigger ------------------------------------------------------------------ export const TRIGGER = diff --git a/system/libs/workspace_ui/src/flyout-position.test.ts b/system/libs/workspace_ui/src/flyout-position.test.ts index 75a7a16647..f65e958677 100644 --- a/system/libs/workspace_ui/src/flyout-position.test.ts +++ b/system/libs/workspace_ui/src/flyout-position.test.ts @@ -1,14 +1,17 @@ import { describe, expect, it } from "vitest"; -import { placeFlyout } from "./flyout-position"; +import { isInSafeTriangle, placeFlyout } from "./flyout-position"; -/** A card open near the bottom of a 1280x800 window, which is where the composer puts it. */ +/** A card open near the bottom of a 1280x800 window, which is where the composer puts it. + * `rowTop` is the row that opened the flyout; `contentHeight` is a short four-row list. */ const BASE = { cardLeft: 400, cardWidth: 340, - rowBottom: 560, + rowTop: 528, + flyoutPadding: 5, + contentHeight: 138, flyoutWidth: 300, - maxFlyoutHeight: 334, + maxFlyoutHeight: 368, viewportWidth: 1280, viewportHeight: 800, margin: 8, @@ -16,21 +19,40 @@ const BASE = { }; describe("placeFlyout", () => { - it("tucks under the card's right edge and stands on the row", () => { - expect(placeFlyout(BASE)).toMatchObject({ left: 736, bottom: 240, side: "trailing" }); + it("tucks under the card's right edge and lines its first row up with the row", () => { + // top is `rowTop - flyoutPadding`, which puts the flyout's first ROW on `rowTop`. + expect(placeFlyout(BASE)).toMatchObject({ left: 736, top: 523, side: "trailing", isSlid: false }); }); - it("grows upward rather than being squeezed by the space below the row", () => { - // The whole reason this file exists: the card opens from the composer at - // the bottom of the panel, so downward there is nothing -- 560px of room sits ABOVE. - const nearBottom = placeFlyout({ ...BASE, rowBottom: 780 }); - expect(nearBottom.bottom).toBe(20); - expect(nearBottom.maxHeight).toBe(334); + it("slides up rather than being squeezed by the space below the row", () => { + // The whole reason this file exists: the card opens from the composer at the bottom of the + // panel, so a ten-row catalog opened from a low row has nothing below it to grow into. + const tall = placeFlyout({ ...BASE, rowTop: 700, contentHeight: 368 }); + expect(tall.isSlid).toBe(true); + // Slid up by exactly enough to stand on the bottom margin, and no shorter for it. + expect(tall.top).toBe(800 - 8 - 368); + expect(tall.maxHeight).toBe(368); }); - it("caps the height when the row is near the top instead of overflowing", () => { - const nearTop = placeFlyout({ ...BASE, rowBottom: 100 }); - expect(nearTop.maxHeight).toBe(92); + it("slides by only as much as it has to", () => { + // 21px short of fitting, so it moves 21px -- not to some fixed anchor. + const barely = placeFlyout({ ...BASE, rowTop: 680 }); + expect(barely.top).toBe(800 - 8 - 138); + expect(barely.isSlid).toBe(true); + }); + + it("holds the alignment whenever the box fits", () => { + const roomy = placeFlyout({ ...BASE, rowTop: 200 }); + expect(roomy.top).toBe(195); + expect(roomy.isSlid).toBe(false); + }); + + it("caps a list too tall for the window, and measures the slide against the cap", () => { + const huge = placeFlyout({ ...BASE, rowTop: 700, contentHeight: 5000, maxFlyoutHeight: 5000 }); + // Capped to the window less both margins... + expect(huge.maxHeight).toBe(784); + // ...and slid to the top margin, rather than to where 5000px of content would have put it. + expect(huge.top).toBe(8); }); it("flips to the leading side when the trailing side would not fit", () => { @@ -45,9 +67,38 @@ describe("placeFlyout", () => { expect(placed.left + BASE.flyoutWidth).toBeLessThanOrEqual(400); }); - it("keeps the base on screen when the row is off it", () => { - const placed = placeFlyout({ ...BASE, rowBottom: -50 }); - expect(placed.bottom).toBe(792); - expect(placed.maxHeight).toBe(0); + it("keeps the box on screen when the row is off it", () => { + const placed = placeFlyout({ ...BASE, rowTop: -50 }); + expect(placed.top).toBe(8); + }); +}); + +/** A flyout on the card's trailing side: its near edge is a vertical line at x=740, running + * from y=300 down to y=560, and the pointer set off from a row at (700, 550). */ +const APEX = { x: 700, y: 550 }; +const EDGE = { edgeX: 740, top: 300, bottom: 560 }; + +describe("isInSafeTriangle", () => { + it("holds a pointer heading up and across towards the flyout", () => { + expect(isInSafeTriangle({ x: 720, y: 480 }, APEX, EDGE)).toBe(true); + }); + + it("lets go of a pointer heading straight up the card instead", () => { + expect(isInSafeTriangle({ x: 640, y: 480 }, APEX, EDGE)).toBe(false); + }); + + it("lets go once the pointer is past the flyout's edge", () => { + expect(isInSafeTriangle({ x: 900, y: 480 }, APEX, EDGE)).toBe(false); + }); + + it("holds the apex itself and the edge's corners", () => { + expect(isInSafeTriangle(APEX, APEX, EDGE)).toBe(true); + expect(isInSafeTriangle({ x: 740, y: 300 }, APEX, EDGE)).toBe(true); + expect(isInSafeTriangle({ x: 740, y: 560 }, APEX, EDGE)).toBe(true); + }); + + it("holds nothing off the line when the flyout has no height to aim at", () => { + const flat = { edgeX: 740, top: 400, bottom: 400 }; + expect(isInSafeTriangle({ x: 720, y: 480 }, APEX, flat)).toBe(false); }); }); diff --git a/system/libs/workspace_ui/src/flyout-position.ts b/system/libs/workspace_ui/src/flyout-position.ts index c6e5a0dcde..261af209ff 100644 --- a/system/libs/workspace_ui/src/flyout-position.ts +++ b/system/libs/workspace_ui/src/flyout-position.ts @@ -2,14 +2,16 @@ * Pure geometry for the combo card's side flyout: where it sits (`placeFlyout`), and the * wedge a pointer on its way to it is allowed to cross (`isInSafeTriangle`). * - * The flyout's BASE sits level with the row that opened it and the list grows UPWARD. That is - * not the ordinary top-align-and-cap-downward rule, and the reason is that this card - * opens from the composer at the BOTTOM of the panel: a list capped by the space below its row - * would have roughly three rows to work with, far too few for a thousand-model catalog. - * Growing up gives it the whole window instead. + * The flyout's FIRST ROW lines up with the row that opened it, so the row you pointed at and + * the list it produced read as one line continuing sideways. That alignment is the default and + * the flyout keeps it whenever it can. * - * The search field belongs at the bottom of that column for the same reason -- it stays put, - * next to the row you came from, while the list extends away from your hand. + * When it cannot -- the card opens from the composer at the BOTTOM of the panel, so a long + * model list starting level with a low row would run off the screen -- the flyout SLIDES UP, + * by exactly as much as it takes to fit, and no further. It never gives up height to hold the + * alignment: a list squeezed into the space below its own row would have about three rows to + * work with, far too few for a thousand-model catalog, so the slide is what buys it the whole + * window. Only a list too tall for the window at all is capped, and then it scrolls. * * Kept free of the DOM so it is unit-testable; the caller measures and feeds it in. */ @@ -18,9 +20,16 @@ export interface FlyoutPlacementInput { /** Viewport left of the card, and its width. */ cardLeft: number; cardWidth: number; - /** Viewport y of the BOTTOM edge of the row that opened the flyout: the base to sit on. */ - rowBottom: number; + /** Viewport y of the TOP edge of the row that opened the flyout: what the flyout's first + * row lines up with. */ + rowTop: number; flyoutWidth: number; + /** The distance from the flyout's outer top edge to the top of its first row -- its border + * and its padding -- so `rowTop` aligns the ROW rather than the box that carries it. */ + flyoutPadding: number; + /** How tall the flyout wants to be, from what it is about to hold. Decides whether it has + * to slide, and by how much. */ + contentHeight: number; /** The tallest the flyout may be before the viewport caps it. */ maxFlyoutHeight: number; viewportWidth: number; @@ -33,12 +42,14 @@ export interface FlyoutPlacementInput { export interface FlyoutPlacement { left: number; - /** Distance from the viewport's BOTTOM to the flyout's base -- it is anchored there and - * grows upward, so this is what stays fixed as the content changes. */ - bottom: number; + /** Viewport y of the flyout's TOP edge. */ + top: number; /** A cap, not a height: the content decides, up to this. */ maxHeight: number; side: "trailing" | "leading"; + /** Whether the alignment had to give way to fit the box on screen. Nothing positions off + * this; it is here so a test can say which of the two rules it is exercising. */ + isSlid: boolean; } /** A viewport point -- where the pointer is, or where it was. */ @@ -89,8 +100,8 @@ export function isInSafeTriangle(point: FlyoutPoint, apex: FlyoutPoint, base: Sa } export function placeFlyout(input: FlyoutPlacementInput): FlyoutPlacement { - const { cardLeft, cardWidth, rowBottom, flyoutWidth, maxFlyoutHeight } = input; - const { viewportWidth, viewportHeight, margin, overlap } = input; + const { cardLeft, cardWidth, rowTop, flyoutPadding, flyoutWidth } = input; + const { contentHeight, maxFlyoutHeight, viewportWidth, viewportHeight, margin, overlap } = input; const trailing = cardLeft + cardWidth - overlap; const leading = cardLeft + overlap - flyoutWidth; @@ -103,13 +114,22 @@ export function placeFlyout(input: FlyoutPlacementInput): FlyoutPlacement { // characters stay readable. const left = Math.min(Math.max(wanted, margin), Math.max(margin, viewportWidth - margin - flyoutWidth)); - // The base never leaves the viewport, and never sits so low the flyout has nowhere to grow. - const base = Math.min(Math.max(rowBottom, margin), viewportHeight - margin); + // The height the box will actually occupy: what it wants, capped by its own ten-row limit + // and by the window. The slide is measured against THIS rather than against the content, + // so a list already capped to a scroller does not slide for height it will never use. + const cap = Math.max(0, Math.min(maxFlyoutHeight, viewportHeight - 2 * margin)); + const height = Math.min(contentHeight, cap); + // Aligned: the box sits `flyoutPadding` above the row, which puts its first row ON the row. + const aligned = rowTop - flyoutPadding; + // The lowest top that still leaves the whole box on screen. Sliding UP to reach it is what + // a low row gets instead of a squeezed list. + const lowestFitting = viewportHeight - margin - height; + const top = Math.max(margin, Math.min(aligned, lowestFitting)); return { left, - bottom: viewportHeight - base, - // Everything between the base and the top margin is available to grow into. - maxHeight: Math.max(0, Math.min(maxFlyoutHeight, base - margin)), + top, + maxHeight: cap, side, + isSlid: top !== aligned, }; } From 91be055a37cb4528ed5ab71dba66346debb90ac8 Mon Sep 17 00:00:00 2001 From: Chat-1 Date: Tue, 8 Sep 2026 18:38:31 -0700 Subject: [PATCH 03/64] fix(chat): cancel a pending hover-open when the pointer leaves the card Sweeping up from the chip and off the top of the card opened the Provider menu ~150ms later, with the pointer already up the transcript. The card is entered at the BOTTOM and left at the TOP, so Provider is the last row the pointer touches on the way out -- and leaving cancelled the pending CLOSE while leaving the pending OPEN to fire. A hover the pointer did not stay for is not an intent to open, so `handleStackLeave` now cancels it unconditionally (the close is still conditional on something being open). Also moves the model search field ABOVE its list, where the typing starts, rather than under the list it filters. Its old place was justified by the grow-upward layout that the previous commit replaced. `SEARCH_WRAP` carries its margin below it now; above, the flyout's own padding is the gap, which also lets the field take the row-alignment line. Verified in Fortress: a fast swipe out (up, and down through the chip) opens nothing at 250ms/750ms/1.25s, resting on a row still opens it, swiping out with a menu up closes it, the field lands 0.0px off the Model row above the first model row, keeps focus, and stays put as typing filters the list. The search field was exercised under a temporarily lowered row threshold, since claude's catalog is too short to show it; the threshold was restored and the rebuilt bundle hash matches the pre-experiment build byte for byte. --- .../apps/chat/frontend/src/views/ModelBar.ts | 66 +++++++++++-------- .../frontend/src/views/modelCardStyles.ts | 13 ++-- 2 files changed, 44 insertions(+), 35 deletions(-) diff --git a/system/apps/chat/frontend/src/views/ModelBar.ts b/system/apps/chat/frontend/src/views/ModelBar.ts index 454ce80ab5..039d47ab1e 100644 --- a/system/apps/chat/frontend/src/views/ModelBar.ts +++ b/system/apps/chat/frontend/src/views/ModelBar.ts @@ -259,13 +259,21 @@ export function ModelBar(): m.Component<{ agentId: string }> { } } - /** The pointer has left the card or the flyout. + /** The pointer has left the card or the flyout. Two things follow. * - * A flyout opened by hover has to close when the hover ends -- otherwise it hangs over the - * transcript until something is clicked, which is exactly what a hover menu is supposed to - * spare the user. The CARD is a different matter: it was opened by a click, so it takes a - * click to dismiss, and the pointer wandering off does not count. */ - function scheduleStackLeave(): void { + * Nothing it was about to open still opens. Sweeping up from the chip enters the card at + * the BOTTOM and leaves at the TOP, so the Provider row is the last one the pointer touches + * on the way out -- and without this its flyout appeared `SUBMENU_HOVER_DELAY_MS` later, + * with the pointer already somewhere up the transcript. A hover that the pointer did not + * stay for is not an intent to open. + * + * And an open flyout follows the pointer out, because a menu opened by hover has to be + * dismissed by hover, or it hangs over the transcript until something is clicked -- which is + * exactly what a hover menu is supposed to spare the user. The CARD is a different matter: + * it was opened by a click, so it takes a click to dismiss, and drifting off does not + * count. */ + function handleStackLeave(): void { + cancelHoverIntent(); if (flyout === null) return; cancelStackLeave(); stackLeaveTimer = window.setTimeout(() => { @@ -666,7 +674,7 @@ export function ModelBar(): m.Component<{ agentId: string }> { cancelHoverIntent(); clearSafeApex(); }, - onmouseleave: scheduleStackLeave, + onmouseleave: handleStackLeave, }, children, ); @@ -810,6 +818,27 @@ export function ModelBar(): m.Component<{ agentId: string }> { // Loading and empty each draw a single line where the list would be. const rowCount = loading || visible.length === 0 ? 1 : visible.length; return flyoutShell(rowCount, hasSearchField, [ + // ABOVE the list: the field is where the pointer arrives and where the typing starts, so + // it sits at the head of the flyout rather than under a list it filters. It stays put + // while the list scrolls beneath it. + // + // The shared input recipe, with the magnifier laid over its left padding: the field owns + // its own frame and focus ring, so nothing here re-styles either. + hasSearchField + ? m("div", { class: css.SEARCH_WRAP }, [ + m("span", { class: css.SEARCH_ICON }, m.trust(icon("search", { size: 13 }))), + m("input", { + class: inputClass({ extra: css.SEARCH_INPUT_EXTRA }), + type: "text", + placeholder: "Search models", + value: modelQuery, + oncreate: (inputVnode: m.VnodeDOM) => (inputVnode.dom as HTMLInputElement).focus(), + oninput: (event: Event) => { + modelQuery = (event.target as HTMLInputElement).value; + }, + }), + ]) + : null, // One list or the other, never a hole beside keyed rows -- mithril refuses a fragment // that mixes the two, and it throws during the DOM diff rather than at build time. m( @@ -846,27 +875,6 @@ export function ModelBar(): m.Component<{ agentId: string }> { ); }), ), - // BELOW the list. A long catalog's flyout is the one that slides down to the bottom of - // the window, so its foot is the edge nearest the composer the pointer came from -- and - // the field stays put there while the list scrolls above it. - // - // The shared input recipe, with the magnifier laid over its left padding: the field owns - // its own frame and focus ring, so nothing here re-styles either. - hasSearchField - ? m("div", { class: css.SEARCH_WRAP }, [ - m("span", { class: css.SEARCH_ICON }, m.trust(icon("search", { size: 13 }))), - m("input", { - class: inputClass({ extra: css.SEARCH_INPUT_EXTRA }), - type: "text", - placeholder: "Search models", - value: modelQuery, - oncreate: (inputVnode: m.VnodeDOM) => (inputVnode.dom as HTMLInputElement).focus(), - oninput: (event: Event) => { - modelQuery = (event.target as HTMLInputElement).value; - }, - }), - ]) - : null, ]); } @@ -977,7 +985,7 @@ export function ModelBar(): m.Component<{ agentId: string }> { // The other half of the stack, for the same leave rule: moving between the card and // its flyout is not leaving, but moving off both of them is. onmouseenter: cancelStackLeave, - onmouseleave: scheduleStackLeave, + onmouseleave: handleStackLeave, }, m("div", { class: css.CARD_INNER }, [ menuRow({ diff --git a/system/apps/chat/frontend/src/views/modelCardStyles.ts b/system/apps/chat/frontend/src/views/modelCardStyles.ts index 13733d311d..7f60e2e33e 100644 --- a/system/apps/chat/frontend/src/views/modelCardStyles.ts +++ b/system/apps/chat/frontend/src/views/modelCardStyles.ts @@ -35,8 +35,8 @@ const FLYOUT_VISIBLE_ROWS = 10; const FLYOUT_BORDER = 1; const FLYOUT_INNER_PADDING = 4; export const FLYOUT_PADDING = FLYOUT_BORDER + FLYOUT_INNER_PADDING; -/** `SEARCH_WRAP`'s `mt-1.5` plus `SEARCH_INPUT_EXTRA`'s `h-8`. */ -const SEARCH_FIELD_HEIGHT = 6 + 32; +/** `SEARCH_INPUT_EXTRA`'s `h-8` plus `SEARCH_WRAP`'s `mb-1.5` under it. */ +const SEARCH_FIELD_HEIGHT = 32 + 6; /** How tall a flyout of `rowCount` rows wants to be, measured the way the browser measures a * bordered box: both borders and both paddings, which is what `2 * FLYOUT_PADDING` is. @@ -129,13 +129,14 @@ export const SWITCH_KNOB_OFF = "translate-x-[2px]"; export const SWITCH_CHECK = "text-accent"; // --- the flyouts --------------------------------------------------------------------------- -/** Same shared chrome as the card. The flex column caps the scroll region under the pinned +/** Same shared chrome as the card. The flex column caps the scroll region beneath the pinned * search field. */ export const FLYOUT = menuCardClass("fixed flex flex-col overflow-hidden text-(length:--font-size-row)"); -/** The search field standing under the list. The wrapper positions the magnifier over the +/** The search field at the head of the list. The wrapper positions the magnifier over the * field's own left padding; the field itself is the shared `inputClass`, so its frame, focus - * ring and placeholder match every other text field in the workspace. */ -export const SEARCH_WRAP = "relative mx-1.5 mt-1.5"; + * ring and placeholder match every other text field in the workspace. The margin is BELOW it, + * separating it from the list it filters -- above it the flyout's own padding is the gap. */ +export const SEARCH_WRAP = "relative mx-1.5 mb-1.5"; export const SEARCH_ICON = "pointer-events-none absolute left-2.5 top-1/2 z-(--z-content) -translate-y-1/2 text-faint"; /** Room for the magnifier, and the dense-chrome row size the rest of the flyout sits at. */ export const SEARCH_INPUT_EXTRA = "h-8 py-0 pl-8 text-(length:--font-size-row)"; From 1f83414d07863cdb0457886a061a3edd45786f7c Mon Sep 17 00:00:00 2001 From: Chat-1 Date: Tue, 8 Sep 2026 19:31:23 -0700 Subject: [PATCH 04/64] style(menus): make the row highlight an inset rounded slab Measured off the mock: the highlight is inset 4px from the card's edges with a 4px radius -- the radius the card's own 8px corner leaves once you step 4px inward, so the slab reads as concentric with the card rather than pasted into it. The colours are unchanged; the mock's greys differ from our tokens by ~1% alpha, which is the Display P3 profile on the screenshot, not a design change. Changed in the shared recipe (`menuRowClass`) rather than forked per app, so the chat card, its flyouts, the provider chooser, the tab kebab menu, the rail's row menus and the launcher's filter menu all move together -- which also means this restyles the workspace UI's own menus, not just chat's. The two hand-rolled copies of the row shape (the flyout rows' selected/locked variants, and the chooser's picker options) carry the same slab. The width is spelled `w-[calc(100%-0.5rem)]`, and both alternatives are wrong: `w-full` plus margins measures 100%+8px and overflows, while `auto` does not fill, because a