feat(overlay): give long menus collapsible groups and one shared device list

The device menu had outgrown a flat list, and two menus were improvising a
delete glyph the icon set did not have.

`MenuGroup` is a single-open accordion entry; `MenuNode` takes an arbitrary row.
`items()` filters rows inside collapsed groups, so keyboard navigation does not
walk through entries nobody can see, and `isMenuItem` is exported —
`font-menu.test.ts` was hand-rolling a `"label" in e` predicate that never
tested `run`.

Two placement fixes, found once the menus were tall enough to show them.
`onReflow` ignores scroll events originating inside the popover, which had been
snapping a tall menu back to the top on every wheel tick. `placePopover` resets
`left` to `0px` before measuring, because a stale value from the previous open
shrank the width available to a content-sized box.

`frame-chrome.ts` is mostly extraction. The frame verbs move to
`frame-verbs.ts` and the custom W×H form to `device-menu.ts`, carrying their
toasts and the delete-undo with them, so the frame bar, the frame list and the
keyboard are the same act rather than three implementations of it.
`deviceGroups()` seeds open on the frame's own family, and the zoom menu is
rebuilt per open.

Delete's glyph moves from `minus` to `trash`. The imported set has no delete
mark, so the two call sites had improvised `−` and `✕` — one of which reads as
half of an add. Eleventh hand-drawn local mark; ICONS.md carries the rationale.
This commit is contained in:
Nayan
2026-08-16 13:05:37 +05:30
parent 880f0ea4f1
commit b0c4e38d37
10 changed files with 771 additions and 189 deletions
+13 -1
View File
@@ -447,6 +447,7 @@ icons:
gutter: local/gutter.svg
lock: local/lock.svg
unlock: local/unlock.svg
trash: local/trash.svg
claude: local/claude.svg
codex: local/codex.svg
opencode: local/opencode.svg
@@ -601,7 +602,7 @@ is the control, not the bundler.
## Marks we draw ourselves
Ten slugs under `assets/local/` are not from the imported set. They go through the same
Eleven slugs under `assets/local/` are not from the imported set. They go through the same
manifest and the same normalisation as everything else — which is the point.
They previously lived as hand-authored strings in the overlay's `icons.ts`,
inset "roughly 4..20" (a **16-unit** span) to match the set by eye. The set's
@@ -656,6 +657,17 @@ conventions with nothing reconciling them.
and sits next to the eye and the layer-kind glyphs, which are filled. These
are the mask's own silhouette painted directly, with the shackle as an
even-odd counter, so the pair matches the family it lives in.
- **`trash`** — a bin, because the imported set has no delete glyph. It publishes
`general/remove.svg` (a bare horizontal rule → `minus`) and `general/x.svg`
(→ `close`), and both were pressed into service: frame delete drew a `−` and
the same action in the frame list drew a `✕`, so one verb had two glyphs and
neither said "destroy". `−` is still right where it pairs with a `+` over a
list — remove a fill, remove a shadow — and that grammar is deliberately left
alone; this is for the deletes that are not the other half of an add.
Three parts on the family's terms: a staple handle, a lid bar, and a body whose
two slots are even-odd counters rather than drawn strokes, so nothing here is a
hairline that has to survive being scaled to `xs`.
- **`claude` / `codex` / `opencode`** — third-party product marks, for the agent
picker. Unlike the rest these are not ours to redraw; they name a backend in
the UI. Path data from `@lobehub/icons-static-svg`; the marks remain the
@@ -0,0 +1,5 @@
<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
<path d="M10 4h4a1 1 0 0 1 1 1v1.25h-1.5V5.5h-3v.75H9V5a1 1 0 0 1 1-1z" fill="currentColor" fill-opacity="0.9"/>
<path d="M5 6.25h14a.75.75 0 0 1 0 1.5H5a.75.75 0 0 1 0-1.5z" fill="currentColor" fill-opacity="0.9"/>
<path fill-rule="evenodd" clip-rule="evenodd" d="M7 9h10v9a2 2 0 0 1-2 2H9a2 2 0 0 1-2-2V9zm3 2.5a.75.75 0 0 1 .75.75v4.5a.75.75 0 0 1-1.5 0v-4.5a.75.75 0 0 1 .75-.75zm4 0a.75.75 0 0 1 .75.75v4.5a.75.75 0 0 1-1.5 0v-4.5a.75.75 0 0 1 .75-.75z" fill="currentColor" fill-opacity="0.9"/>
</svg>

After

Width:  |  Height:  |  Size: 603 B

@@ -361,6 +361,7 @@ export const EDITOR_ICONS = {
"tool-move": { box: 24, body: `<g transform="translate(2.59 2.589) scale(0.753)"><mask id="ap-tool-move-mask0_66_4378" style="mask-type:luminance" maskUnits="userSpaceOnUse" x="4" y="4" width="17" height="17"><path d="M4 4H21V21H4V4Z" fill="white"/><path fill-rule="evenodd" clip-rule="evenodd" d="M19.29 12.06C19.49 12 19.67 11.88 19.79 11.72C19.92 11.55 19.99 11.35 20 11.15C20.01 10.94 19.95 10.73 19.84 10.56C19.72 10.39 19.56 10.25 19.37 10.18L6.37 5.07C6.18 5 5.99 4.98 5.8 5.02C5.61 5.06 5.43 5.15 5.29 5.29C5.15 5.43 5.06 5.61 5.02 5.8C4.98 5.99 5 6.18 5.07 6.37L10.18 19.37C10.25 19.56 10.39 19.72 10.56 19.84C10.73 19.95 10.94 20.01 11.15 20C11.35 19.99 11.55 19.92 11.72 19.79C11.88 19.67 12 19.49 12.06 19.29L13.76 13.76L19.29 12.06Z" fill="black"/></mask><g mask="url(#ap-tool-move-mask0_66_4378)"><path d="M13.76 13.76L13.47 12.81C13.31 12.86 13.17 12.94 13.06 13.06C12.94 13.17 12.86 13.31 12.81 13.47L13.76 13.76ZM19 11.11L19.59 13.02C19.98 12.9 20.33 12.65 20.59 12.33C20.84 12 20.98 11.6 21 11.18L19 11.11ZM19 11.11L21 11.18C21.01 10.77 20.9 10.36 20.67 10.01C20.45 9.67 20.12 9.4 19.73 9.25L19 11.11ZM6 6L19 11.11L19.73 9.25L6.73 4.14L6 6ZM6 6L6.73 4.14C6.37 4 5.97 3.96 5.59 4.04C5.21 4.12 4.86 4.31 4.59 4.59L6 6ZM6 6L4.59 4.59C4.31 4.86 4.12 5.21 4.04 5.59C3.96 5.97 4 6.37 4.14 6.73L6 6ZM11.11 19L6 6L4.14 6.73L9.25 19.73L11.11 19ZM11.11 19L9.25 19.73C9.4 20.12 9.67 20.45 10.01 20.67C10.36 20.9 10.77 21.02 11.18 21L11.11 19ZM11.11 19L11.18 21C11.6 20.98 12 20.84 12.33 20.59C12.65 20.33 12.9 19.99 13.02 19.59L11.11 19ZM12.81 13.47L11.11 19L13.02 19.59L14.72 14.06L12.81 13.47ZM19 11.11L13.47 12.81L14.06 14.72L19.59 13.02L19 11.11Z" fill="currentColor" fill-opacity="0.9"/></g></g>` },
"tool-scale": { box: 24, body: `<g transform="translate(2.4 2.4) scale(0.8)"><path fill-rule="evenodd" clip-rule="evenodd" d="M5 6.5C5 6.1 5.16 5.72 5.44 5.44C5.72 5.16 6.1 5 6.5 5H17.5C17.9 5 18.28 5.16 18.56 5.44C18.84 5.72 19 6.1 19 6.5V17.5C19 17.9 18.84 18.28 18.56 18.56C18.28 18.84 17.9 19 17.5 19H15.5C15.37 19 15.24 19.05 15.15 19.15C15.05 19.24 15 19.37 15 19.5C15 19.63 15.05 19.76 15.15 19.85C15.24 19.95 15.37 20 15.5 20H17.5C18.16 20 18.8 19.74 19.27 19.27C19.74 18.8 20 18.16 20 17.5V6.5C20 5.84 19.74 5.2 19.27 4.73C18.8 4.26 18.16 4 17.5 4H6.5C5.84 4 5.2 4.26 4.73 4.73C4.26 5.2 4 5.84 4 6.5V8.5C4 8.63 4.05 8.76 4.15 8.85C4.24 8.95 4.37 9 4.5 9C4.63 9 4.76 8.95 4.85 8.85C4.95 8.76 5 8.63 5 8.5V6.5ZM11 8.5C11 8.37 11.05 8.24 11.15 8.15C11.24 8.05 11.37 8 11.5 8H15.5C15.63 8 15.76 8.05 15.85 8.15C15.95 8.24 16 8.37 16 8.5V12.5C16 12.63 15.95 12.76 15.85 12.85C15.76 12.95 15.63 13 15.5 13C15.37 13 15.24 12.95 15.15 12.85C15.05 12.76 15 12.63 15 12.5V9.71L8.85 15.85C8.76 15.95 8.63 16 8.5 16C8.37 16 8.24 15.95 8.15 15.85C8.05 15.76 8 15.63 8 15.5C8 15.37 8.05 15.24 8.15 15.15L14.29 9H11.5C11.37 9 11.24 8.95 11.15 8.85C11.05 8.76 11 8.63 11 8.5ZM4 11.5C4 11.1 4.16 10.72 4.44 10.44C4.72 10.16 5.1 10 5.5 10H10.5C10.63 10 10.76 10.05 10.85 10.15C10.95 10.24 11 10.37 11 10.5C11 10.63 10.95 10.76 10.85 10.85C10.76 10.95 10.63 11 10.5 11H5.5C5.37 11 5.24 11.05 5.15 11.15C5.05 11.24 5 11.37 5 11.5V18.5C5 18.63 5.05 18.76 5.15 18.85C5.24 18.95 5.37 19 5.5 19H12.5C12.63 19 12.76 18.95 12.85 18.85C12.95 18.76 13 18.63 13 18.5V13.5C13 13.37 13.05 13.24 13.15 13.15C13.24 13.05 13.37 13 13.5 13C13.63 13 13.76 13.05 13.85 13.15C13.95 13.24 14 13.37 14 13.5V18.5C14 18.9 13.84 19.28 13.56 19.56C13.28 19.84 12.9 20 12.5 20H5.5C5.1 20 4.72 19.84 4.44 19.56C4.16 19.28 4 18.9 4 18.5V11.5Z" fill="currentColor" fill-opacity="0.9"/></g>` },
"tool-text": { box: 24, body: `<g transform="translate(-1.334 -0.8) scale(1.067)"><path d="M8.5 6C8.78 6 9 6.22 9 6.5V17.5C9 17.78 8.78 18 8.5 18C8.22 18 8 17.78 8 17.5V6.5C8 6.22 8.22 6 8.5 6ZM13.5 8C13.7 8 13.88 8.12 13.96 8.3L16.1 13.3L16.96 15.3L16.99 15.4C17.04 15.63 16.92 15.86 16.7 15.96C16.48 16.06 16.22 15.98 16.09 15.79L16.04 15.7L15.31 14H11.69L10.96 15.7C10.85 15.95 10.56 16.07 10.3 15.96C10.05 15.85 9.93 15.56 10.04 15.3L10.9 13.3L13.04 8.3L13.07 8.24C13.16 8.09 13.32 8 13.5 8ZM12.12 13H14.88L13.5 9.77L12.12 13Z" fill="currentColor" fill-opacity="0.9"/></g>` },
"trash": { box: 24, body: `<g transform="translate(2.4 2.4) scale(0.8)"><path d="M10 4h4a1 1 0 0 1 1 1v1.25h-1.5V5.5h-3v0.75H9V5a1 1 0 0 1 1-1z" fill="currentColor" fill-opacity="0.9"/><path d="M5 6.25h14a0.750.75 0 0 1 0 1.5H5a0.750.75 0 0 1 0-1.5z" fill="currentColor" fill-opacity="0.9"/><path fill-rule="evenodd" clip-rule="evenodd" d="M7 9h10v9a2 2 0 0 1-2 2H9a2 2 0 0 1-2-2V9zm3 2.5a0.750.75 0 0 1 0.750.75v4.5a0.750.75 0 0 1-1.5 0v-4.5a0.750.75 0 0 1 0.75-0.75zm4 0a0.750.75 0 0 1 0.750.75v4.5a0.750.75 0 0 1-1.5 0v-4.5a0.750.75 0 0 1 0.75-0.75z" fill="currentColor" fill-opacity="0.9"/></g>` },
"unlock": { box: 24, body: `<g transform="translate(0.185 0.677) scale(0.985)"><path fill-rule="evenodd" clip-rule="evenodd" d="M12 5C13.66 5 15 6.34 15 8V8.5C15 8.78 14.78 9 14.5 9C14.22 9 14 8.78 14 8.5V8C14 6.9 13.1 6 12 6C10.9 6 10 6.9 10 8V11H15.5C16.33 11 17 11.67 17 12.5V16.5C17 17.33 16.33 18 15.5 18H8.5C7.67 18 7 17.33 7 16.5V12.5C7 11.67 7.67 11 8.5 11H9V8C9 6.34 10.34 5 12 5Z" fill="currentColor" fill-opacity="0.9"/></g>` },
"var-add": { box: 24, body: `<g transform="translate(-1.382 -1.382) scale(1.164)"><path fill-rule="evenodd" clip-rule="evenodd" d="M11.5 6C11.63 6 11.76 6.05 11.85 6.15C11.95 6.24 12 6.37 12 6.5V11H16.5C16.63 11 16.76 11.05 16.85 11.15C16.95 11.24 17 11.37 17 11.5C17 11.63 16.95 11.76 16.85 11.85C16.76 11.95 16.63 12 16.5 12H12V16.5C12 16.63 11.95 16.76 11.85 16.85C11.76 16.95 11.63 17 11.5 17C11.37 17 11.24 16.95 11.15 16.85C11.05 16.76 11 16.63 11 16.5V12H6.5C6.37 12 6.24 11.95 6.15 11.85C6.05 11.76 6 11.63 6 11.5C6 11.37 6.05 11.24 6.15 11.15C6.24 11.05 6.37 11 6.5 11H11V6.5C11 6.37 11.05 6.24 11.15 6.15C11.24 6.05 11.37 6 11.5 6Z" fill="currentColor" fill-opacity="0.9"/></g>` },
"var-apply": { box: 24, body: `<g transform="translate(5.351 5.345) scale(1.108)"><path fill-rule="evenodd" clip-rule="evenodd" d="M2 3.69L6 1.38L10 3.69V8.31L6 10.62L2 8.31V3.69ZM6 0.23L11 3.12V8.89L6 11.78L1 8.89V3.12L6 0.23V0.23ZM7 6C7 6.55 6.55 7 6 7C5.45 7 5 6.55 5 6C5 5.45 5.45 5 6 5C6.55 5 7 5.45 7 6Z" fill="currentColor" fill-opacity="0.9"/></g>` },
+131
View File
@@ -0,0 +1,131 @@
/**
* "Which viewport?", as menu content.
*
* Four surfaces ask that question — the bar's `+`, the bar's dimensions button,
* the frame list's `+`, and a row's `⋯` — and until now the answer was written
* twice: `frame-chrome.ts` built an accordion of `<button>`s against its own
* world-anchored menu, while `frames-panel.ts` pushed the same presets into
* `createMenu` as a flat header plus twenty-two rows. Two lists over one set of
* devices, one of them structured and one of them a scroll.
*
* This is the half that can be shared: the *entries*, not the menu. The bar's
* menus place themselves against canvas geometry and re-anchor on every pan, so
* they cannot be `createMenu` clients (see the note in `popover-host.ts`) — but
* the custom-size form has nothing to do with placement, and the group structure
* is just `PRESET_GROUPS` shaped for one consumer or the other.
*/
import { cls, el } from "../dom";
import type { MenuGroup } from "../popover-host";
import {
type DevicePreset,
type Frame,
framePreset,
groupOfPreset,
PRESET_GROUPS,
} from "./frames";
/** Below this a "frame" is not a viewport, it is a mistake. */
const MIN_SIDE = 120;
/**
* The device list as collapsible `createMenu` groups.
*
* `open` is seeded from the frame's own device rather than left to fall back to
* the first group, so opening the menu on a tablet shows you tablets. Passing no
* frame — the add-frame case, where there is no current device — takes the
* fallback, which is what `seedOpenGroup` does with an all-false set.
*
* The buckets come from `PRESET_GROUPS` rather than the flat `PRESETS` for the
* reason that list gives: the groups are the source of truth, so a menu built
* from them cannot drift out of step with the one on the canvas.
*/
export function deviceGroups(
pick: (preset: DevicePreset) => void,
current: Frame | null = null
): MenuGroup[] {
const currentId = current ? (framePreset(current)?.id ?? null) : null;
const openId = current ? (groupOfPreset(current.presetId)?.id ?? null) : null;
return PRESET_GROUPS.map((group) => ({
group: group.id,
items: group.presets.map((preset) => ({
hint: `${preset.width} × ${preset.height}`,
label: preset.label,
on: preset.id === currentId,
run: () => pick(preset),
})),
label: group.label,
open: group.id === openId,
}));
}
/**
* The width × height escape hatch from the preset list.
*
* `close` is a parameter rather than a call to some menu's own closer because
* the two callers close differently — the bar's menu is a `hidden` class it owns
* itself, the panel's is a popover on the host's stack — and the form has no
* business knowing which.
*
* Both `Enter` and `Escape` are handled on the field, not left to the menu.
* `Keys` skips every binding without `allowWhileTyping` while a field has focus,
* so with this one focused the menu's own Escape never runs and the only way out
* would be the mouse. `keys.ts` prescribes exactly this — field-local commit and
* cancel — and `renameFrame` already does it.
*/
export function customSizeRow(
apply: (width: number, height: number) => void,
close: () => void,
start: { height: number; width: number } = { height: 800, width: 1280 }
): HTMLElement {
const num = (placeholder: string, value: number): HTMLInputElement =>
el("input", {
class: cls("fc-menu-num"),
min: String(MIN_SIDE),
placeholder,
type: "number",
value: String(Math.round(value)),
}) as HTMLInputElement;
const w = num("W", start.width);
const h = num("H", start.height);
const commit = (): void => {
const width = Number.parseInt(w.value, 10);
const height = Number.parseInt(h.value, 10);
if (width >= MIN_SIDE && height >= MIN_SIDE) {
apply(width, height);
close();
}
};
for (const input of [w, h]) {
input.addEventListener("keydown", (e) => {
if (e.key === "Enter") {
e.preventDefault();
commit();
return;
}
if (e.key === "Escape") {
e.preventDefault();
e.stopPropagation();
close();
}
});
// A press in the field is not a choice, and the row it sits in is inside a
// menu that closes on one.
input.addEventListener("click", (e) => e.stopPropagation());
input.addEventListener("pointerdown", (e) => e.stopPropagation());
}
return el("div", { class: cls("fc-menu-custom") }, [
w,
el("span", { class: cls("fc-menu-dim"), text: "×" }),
h,
el("button", {
class: cls("fc-menu-go"),
onClick: (e: Event) => {
e.stopPropagation();
commit();
},
text: "Set",
type: "button",
}),
]);
}
+81 -165
View File
@@ -12,9 +12,10 @@ import { clear, cls, el, PREFIX } from "../dom";
import { type IconName, icon } from "../icons";
import { keys } from "../keys";
import { placePopover } from "../popover";
import { createMenu, type MenuHandle } from "../popover-host";
import { createMenu, type MenuEntry } from "../popover-host";
import { isElement } from "../realm";
import { toast } from "../toast";
import { customSizeRow } from "./device-menu";
import { deleteFrame, duplicateFrame, reloadFrame } from "./frame-verbs";
import {
type DevicePreset,
type Frame,
@@ -23,7 +24,6 @@ import {
groupOfPreset,
MAX_FRAMES,
PRESET_GROUPS,
type RemovedFrame,
} from "./frames";
import { frameScreenRect } from "./space";
import type { CanvasViewport } from "./viewport";
@@ -119,7 +119,6 @@ export class FrameChrome {
private readonly addMenu: HTMLElement;
private readonly addBtn: HTMLElement;
private readonly zoomLabel: HTMLElement;
private readonly zoomMenu: MenuHandle;
/** The selection-contingent verb group — see `buildFrameTools`. */
private readonly frameTools: HTMLElement;
private readonly dimsMenu: HTMLElement;
@@ -158,31 +157,18 @@ export class FrameChrome {
// The readout is a menu, not a reset button. "Click the percentage to go
// back to 100%" is a hidden affordance; the list also surfaces zoom-to-
// selection, which had no discoverable entry point at all.
this.zoomMenu = createMenu([
{ hint: "⌘+", label: "Zoom in", run: () => this.zoomBy(1) },
{ hint: "⌘−", label: "Zoom out", run: () => this.zoomBy(-1) },
{
hint: "⇧1",
label: "Zoom to fit",
run: () => this.runZoom(() => this.deps.viewport.zoomToFit()),
},
{
hint: "⇧2",
label: "Zoom to selection",
run: () => this.runZoom(() => this.deps.viewport.zoomToSelection()),
},
{
hint: "⌘0",
label: "Zoom to 100%",
run: () => this.runZoom(() => this.deps.viewport.zoomTo100()),
},
]);
this.zoomLabel = el("button", {
class: cls("fbar-zoom"),
"data-tip": "Zoom",
onClick: (e: Event) => {
e.stopPropagation();
this.zoomMenu.open(this.zoomLabel, "above");
// Built per open rather than once. It carried a minimap checkbox whose
// `on` was read at render time, so a menu built in the constructor
// showed that state forever; the checkbox is gone, but the rows still
// close over live viewport state and this costs nothing. Re-opening
// still toggles shut — that is keyed on the *anchor* in `openPopover`'s
// stack, not on the handle.
createMenu(this.zoomEntries()).open(this.zoomLabel, "above");
},
text: "100%",
type: "button",
@@ -195,7 +181,11 @@ export class FrameChrome {
this.toolbar = el("div", { class: cls("fbar") }, [
this.addBtn,
el("div", { class: cls("fbar-sep") }),
this.barButton("fit", "Zoom to fit (⇧1)", () => {
// "Zoom to fit", not "Zoom to fit (⇧1)". `barButton` uses the label as the
// tip, and `Tooltips.show` resolves the chord by matching that string
// against a binding's `label` — so spelling the chord into the text both
// duplicated it and stopped the real chip from ever being found.
this.barButton("fit", "Zoom to fit", () => {
this.deps.viewport.zoomToFit();
this.deps.viewport.save();
}),
@@ -394,6 +384,42 @@ export class FrameChrome {
this.frameTools.remove();
}
/**
* The zoom readout's menu.
*
* A menu rather than a reset button: "click the percentage to go back to
* 100%" is a hidden affordance, and the list also surfaces zoom-to-selection,
* which had no discoverable entry point at all.
*
* It used to carry a "Show minimap" checkbox as its last row, and that row was
* the only way to bring the map back once its close button had been used —
* a control whose off switch sat on the card itself and whose on switch was
* buried under a percentage readout. The map does not hide any more, so
* neither the row nor the deps behind it exist. See `minimap.ts`.
*/
private zoomEntries(): MenuEntry[] {
const entries: MenuEntry[] = [
{ hint: "⌘+", label: "Zoom in", run: () => this.zoomBy(1) },
{ hint: "⌘−", label: "Zoom out", run: () => this.zoomBy(-1) },
{
hint: "⇧1",
label: "Zoom to fit",
run: () => this.runZoom(() => this.deps.viewport.zoomToFit()),
},
{
hint: "⇧2",
label: "Zoom to selection",
run: () => this.runZoom(() => this.deps.viewport.zoomToSelection()),
},
{
hint: "⌘0",
label: "Zoom to 100%",
run: () => this.runZoom(() => this.deps.viewport.zoomTo100()),
},
];
return entries;
}
private runZoom(fn: () => void): void {
fn();
this.deps.viewport.save();
@@ -433,7 +459,7 @@ export class FrameChrome {
"div",
{
class: cls("fc-label"),
"data-tip": "Drag to move · double-click to rename",
"data-tip": "Drag to move, double-click to rename",
onClick: () => this.deps.frames.setActive(frame.id),
onDblclick: () => this.renameFrame(frame),
},
@@ -573,9 +599,14 @@ export class FrameChrome {
/**
* A width × height row that commits on Enter or on its button.
*
* The fields start at the frame's *current* size, which is what makes this a
* nudge rather than a form: almost every custom size is an adjustment of the
* size you already have — 1440 wide but shorter, this phone but taller — and
* The form itself is `device-menu.ts`'s — the frame list's `+` grew one too,
* and two copies of a two-field form is two places for the minimum to drift.
* This wrapper is the part that is genuinely this file's: which closer to hand
* it, and where the fields start.
*
* They start at the frame's *current* size, which is what makes this a nudge
* rather than a form: almost every custom size is an adjustment of the size
* you already have — 1440 wide but shorter, this phone but taller — and
* starting from a fixed 1280 × 800 meant retyping both numbers to change one.
* `syncCustomRow` re-seeds them each time a menu opens, because the menu is
* built once and the frame keeps changing under it.
@@ -585,98 +616,23 @@ export class FrameChrome {
*/
private customRow(
apply: (width: number, height: number) => void,
start: { width: number; height: number } = { height: 800, width: 1280 }
start?: { height: number; width: number }
): HTMLElement {
const num = (placeholder: string, value: number): HTMLInputElement =>
el("input", {
class: cls("fc-menu-num"),
min: "120",
placeholder,
type: "number",
value: String(Math.round(value)),
}) as HTMLInputElement;
const w = num("W", start.width);
const h = num("H", start.height);
const commit = (): void => {
const width = Number.parseInt(w.value, 10);
const height = Number.parseInt(h.value, 10);
if (width >= 120 && height >= 120) {
apply(width, height);
this.closeMenu();
}
};
for (const input of [w, h]) {
input.addEventListener("keydown", (e) => {
if (e.key === "Enter") {
e.preventDefault();
commit();
return;
}
// Escape has to be handled here, not by the menu's own binding.
// `Keys` skips every binding without `allowWhileTyping` while a field
// has focus, so with this field focused the menu's Escape never runs and
// the only way out would be the mouse. `keys.ts` prescribes exactly this
// — field-local commit and cancel — and `renameFrame` already does it.
if (e.key === "Escape") {
e.preventDefault();
e.stopPropagation();
this.closeMenu();
}
});
input.addEventListener("click", (e) => e.stopPropagation());
}
return el("div", { class: cls("fc-menu-custom") }, [
w,
el("span", { class: cls("fc-menu-dim"), text: "×" }),
h,
el("button", {
class: cls("fc-menu-go"),
onClick: (e: Event) => {
e.stopPropagation();
commit();
},
text: "Set",
type: "button",
}),
]);
return customSizeRow(apply, () => this.closeMenu(), start);
}
// -- Verbs -------------------------------------------------------------------
/*
* One closure per verb.
* The verbs themselves live in `frame-verbs.ts`, shared with the frame list.
*
* These used to have two doors — the frame's own menu and the bar group — and
* the pairing exists because a verb whose receipt depends on which door you
* came through is a verb with two behaviours. The menu is gone; the split
* stays because the bar has no `Frame` in hand (it acts on whatever is
* selected) while `undo` and the keyboard paths do.
* What stays here is the *active-frame* half of each pair, and the split is
* the same one it has always been: the bar has no `Frame` in hand — it acts
* on whatever is selected — while the keyboard paths and the list do. Moving
* the bodies out is what stops a third door from growing its own wording, or
* its own missing undo; see the header of that file.
*/
private reloadFrame(frame: Frame): void {
this.deps.frames.reload(frame);
// A fast reload of a static route repaints identically, so without this
// there is no way to tell it happened.
toast(`Reloaded ${frame.name}`);
}
private duplicateFrame(frame: Frame): void {
const { name } = frame;
// `duplicate` → `add` returns null at the cap and nobody reads it. The `+`
// button has `fbar-off` to say so, and `syncFrameTools` gives the bar's
// Duplicate the same treatment; a menu row has nothing, so for that door the
// refusal has to be spoken.
const copy = this.deps.frames.duplicate(frame.id);
if (copy) {
// `placeNext` puts the copy right of the *rightmost* frame, which above
// roughly 60% zoom is off-screen — today this does its work entirely out
// of sight.
toast(`Duplicated ${name}`);
} else {
toast(`Frame limit reached (${MAX_FRAMES})`, { tone: "error" });
}
}
private rotateActiveFrame(): void {
const frame = this.deps.frames.active;
if (frame) {
@@ -689,66 +645,22 @@ export class FrameChrome {
private reloadActiveFrame(): void {
const frame = this.deps.frames.active;
if (frame) {
this.reloadFrame(frame);
reloadFrame(this.deps.frames, frame);
}
}
private duplicateActiveFrame(): void {
const frame = this.deps.frames.active;
if (frame) {
this.duplicateFrame(frame);
duplicateFrame(this.deps.frames, frame);
}
}
// -- Delete ------------------------------------------------------------------
/**
* Remove a frame, and offer it back.
*
* One closure behind three doors — this menu row, the bar group's `−`, and ⌫ —
* so neither the receipt nor the undo can depend on which one you reached for.
* That is the same arrangement `AirshipApp.undoEdit` has for ⌘Z and the bar's
* Undo button, and for the same reason.
*
* A deleted frame is obviously gone, so the toast is not here on the
* legibility clause; it is here because it *carries* the undo. `History`
* journals element ops only, and teaching it a frame op would put the model
* that replays DOM mutations in charge of rebuilding an iframe realm — so the
* affordance rides on the receipt instead. See the header of `toast.ts`.
*
* No confirmation, deliberately. A modal in front of a reversible act taxes
* the ninety-nine deletes you meant in order to catch the one you did not; the
* Undo catches that one and does not charge for the rest.
*/
private deleteFrame(frame: Frame): void {
const { name } = frame;
const removed = this.deps.frames.remove(frame.id);
if (!removed) {
return;
}
toast(`Deleted ${name}`, {
action: { label: "Undo", run: () => this.restoreFrame(removed) },
});
}
/** The Undo button's other half. Silent on success — see below. */
private restoreFrame(removed: RemovedFrame): void {
if (this.deps.frames.restoreRemoved(removed)) {
// The frame reappearing, selected, is the feedback. This is the
// legibility clause working the way it is supposed to.
return;
}
// The only way to get here is to have filled the canvas since the delete,
// and a refusal always speaks — a button that has already faded has no
// disabled state to lean on.
toast(`Frame limit reached (${MAX_FRAMES})`, { tone: "error" });
}
/** What ⌫ and the bar's `−` resolve to. */
private deleteActiveFrame(): void {
const frame = this.deps.frames.active;
if (frame) {
this.deleteFrame(frame);
deleteFrame(this.deps.frames, frame);
}
}
@@ -785,17 +697,21 @@ export class FrameChrome {
* own rule is that a separator goes with the group it divides, and as a child
* it hides with the group instead of leaving a hairline hanging off the Hand.
*
* On the glyphs — the imported set publishes no trash, no refresh and no copy, so
* three of these are chosen rather than found:
* On the glyphs — the imported set publishes no refresh and no copy, so two of
* these are chosen rather than found:
*
* - `minus` is the literal counterpart of the `plus` three controls along in
* the same bar. `+` adds a frame, `−` removes the selected one; that grammar
* is already established here and needs no new artwork.
* - `rotate-ccw` for Reload, because a circular arrow *is* the reload mark. It
* is also the Undo glyph — deliberately, and it is not a collision: Undo
* lives in `editOnlyBar` and this lives in `viewOnlyBar`, and `syncBar`
* guarantees the two are never on screen together.
* - `doc-plus` for Duplicate: a frame is a document, and this makes one more.
*
* Delete was a third. It drew `minus`, on the argument that `−` is the literal
* counterpart of the `+` three controls along — which is a real grammar, but
* the wrong one: `+`/`−` over a *list* adds and removes a row, and this
* destroys a frame. The same verb in the frame list drew `✕`, so one action
* had two glyphs and neither said "destroy". The set now carries a `trash` of
* our own; see ICONS.md for why it was drawn rather than found.
*/
private buildFrameTools(): HTMLElement {
return el(
@@ -815,7 +731,7 @@ export class FrameChrome {
this.reloadActiveFrame();
}),
this.dupBtn,
this.barButton("minus", "Delete frame", (e) => {
this.barButton("trash", "Delete frame", (e) => {
e.stopPropagation();
this.deleteActiveFrame();
}),
+101
View File
@@ -0,0 +1,101 @@
/**
* What can be done *to* a frame, as free functions — delete, duplicate, reload.
*
* These lived as private methods on `FrameChrome`, which was right while the
* frame's own furniture and the bar group were the only doors onto them. The
* frame list is a third, and it is the one that made the arrangement a hazard
* rather than merely a shape: a panel calling `frames.remove` directly is one
* line, looks correct, and silently ships a delete with **no undo** — the
* affordance does not live in the model, it rides on the toast (see the note on
* `deleteFrame`). Nothing about `FrameManager`'s API would have stopped that,
* and nothing would have reported it afterwards.
*
* So the receipts move here with the verbs. A frame deleted from the list and a
* frame deleted with ⌫ are now the same act with the same wording and the same
* way back, because they are the same function — which is the arrangement
* `AirshipApp.undoEdit` already has for ⌘Z and the bar's Undo, and for the same
* reason.
*
* Free functions rather than a class: there is no state here, only a
* `FrameManager` passed in. `toast` is imported directly for the reason its own
* header gives — it has no per-instance state and *is* the DOM, so there is
* nothing to invert.
*/
import { toast } from "../toast";
import {
type Frame,
type FrameManager,
MAX_FRAMES,
type RemovedFrame,
} from "./frames";
/**
* Put a removed frame back — the Undo button's half of `deleteFrame`.
*
* Silent on success, because the frame reappearing where it was *is* the
* feedback. The only way to reach the refusal is to have filled the canvas
* since the delete, and a refusal always speaks: a toast action that has
* already faded has no disabled state to lean on.
*/
export function restoreFrame(
frames: FrameManager,
removed: RemovedFrame
): void {
if (frames.restoreRemoved(removed)) {
return;
}
toast(`Frame limit reached (${MAX_FRAMES})`, { tone: "error" });
}
/**
* Remove a frame, and offer it back.
*
* A deleted frame is obviously gone, so the toast is not here on the legibility
* clause; it is here because it *carries* the undo. `History` journals element
* ops only, and teaching it a frame op would put the model that replays DOM
* mutations in charge of rebuilding an iframe realm — so the affordance rides
* on the receipt instead. See the header of `toast.ts`.
*
* No confirmation, deliberately. A modal in front of a reversible act taxes the
* ninety-nine deletes you meant in order to catch the one you did not; the Undo
* catches that one and does not charge for the rest.
*/
export function deleteFrame(frames: FrameManager, frame: Frame): void {
const { name } = frame;
const removed = frames.remove(frame.id);
if (!removed) {
return;
}
toast(`Deleted ${name}`, {
action: { label: "Undo", run: () => restoreFrame(frames, removed) },
});
}
/**
* Copy a frame, device and all.
*
* `duplicate` → `add` returns null at the cap and the geometry gives no hint:
* `placeNext` puts the copy right of the *rightmost* frame, which above roughly
* 60% zoom is off-screen, so this verb does its work entirely out of sight
* either way. The bar's Duplicate is disabled at the cap and the `+` button has
* `fbar-off` to say so, but a menu row has nothing — so for those doors the
* refusal has to be spoken.
*/
export function duplicateFrame(frames: FrameManager, frame: Frame): void {
const { name } = frame;
if (frames.duplicate(frame.id)) {
toast(`Duplicated ${name}`);
} else {
toast(`Frame limit reached (${MAX_FRAMES})`, { tone: "error" });
}
}
/**
* Reload the app inside a frame. A fast reload of a static route repaints
* identically, so without the receipt there is no way to tell it happened.
*/
export function reloadFrame(frames: FrameManager, frame: Frame): void {
frames.reload(frame);
toast(`Reloaded ${frame.name}`);
}
@@ -1,5 +1,6 @@
import type { DesignToken } from "@airship/protocol/tokens";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { isMenuItem } from "../../popover-host";
import { setRuntimeTokens } from "../../tokens/registry";
import type { TokenSlot } from "./context";
import { fontMenuEntries } from "./text";
@@ -72,11 +73,15 @@ function menu(bound: string | null, onPick = vi.fn()) {
return { apply, entries, onPick, unlink };
}
/** Only the clickable rows — headers and separators carry no label. */
/**
* Only the clickable rows.
*
* Through the shared guard rather than `"label" in e`, which stopped being a
* complete test the day menus grew collapsible groups: a group carries a label
* too, so the hand-rolled predicate quietly widened to include one.
*/
const items = (entries: ReturnType<typeof menu>["entries"]) =>
entries.filter(
(e): e is Extract<typeof e, { label: string }> => "label" in e
);
entries.filter(isMenuItem);
/** The menu flattened to strings, so order and grouping read at a glance. */
const labels = (entries: ReturnType<typeof menu>["entries"]) =>
+208
View File
@@ -3,6 +3,8 @@ import { cls, el } from "./dom";
import { keys } from "./keys";
import {
closeOpenPopover,
createMenu,
type MenuGroup,
mountPopoverHost,
openPopover,
type PopoverHandle,
@@ -304,3 +306,209 @@ describe("a popover's own keys", () => {
}
});
});
/*
* The reflow listener is registered in capture phase, which is what lets it see
* scrolls inside `.insp-body` — and, for a long time, scrolls inside the popover
* itself. `reposition` is `placePopover`, whose first act is to clear the
* `max-height` it wrote so it can measure the content unconstrained. Under a
* scrolled menu that let the box grow to full height, so the browser clamped
* `scrollTop` back to zero before the cap went on again: every wheel tick over
* the frame menu's twenty-seven rows snapped it to the top, which reads as a
* menu that simply refuses to scroll.
*/
describe("a scrollable popover", () => {
it("keeps its scroll position when the scroll came from inside it", () => {
const { handle } = open(anchor());
const shell = shells()[0] as HTMLElement;
// happy-dom does no layout, so `scrollTop` will not stick on its own.
let top = 0;
Object.defineProperty(shell, "scrollTop", {
configurable: true,
get: () => top,
set: (v: number) => {
top = v;
},
});
const spy = vi.spyOn(handle, "reposition");
shell.scrollTop = 40;
shell.dispatchEvent(new Event("scroll", { bubbles: true }));
expect(shell.scrollTop).toBe(40);
expect(spy).not.toHaveBeenCalled();
});
it("still re-places when something else scrolls", () => {
// The behaviour the guard must not cost: a scroller the popover is anchored
// inside still moves the anchor, and the popover has to follow it.
const scroller = el("div");
document.body.append(scroller);
const trigger = el("button", { type: "button" });
scroller.append(trigger);
const { handle } = open(trigger);
const spy = vi.spyOn(handle, "reposition");
scroller.dispatchEvent(new Event("scroll", { bubbles: true }));
expect(spy).toHaveBeenCalled();
});
});
/*
* The accordion.
*
* A flat `pop-head` is right for three verbs and wrong for twenty-two device
* presets, where the menu becomes a twenty-seven-item column that gets capped
* and scrolled — so Delete ends up below the fold of a menu you opened to reach
* it. What is worth pinning down is the part that is not visual: exactly one
* group is open, a shut group's rows stay in the DOM but out of the keyboard
* cursor, and a disclosure is not a choice.
*/
describe("a menu with collapsible groups", () => {
const GROUPS: MenuGroup[] = [
{
group: "phone",
items: [
{ label: "iPhone", run: () => undefined },
{ label: "Pixel", run: () => undefined },
],
label: "Phone",
},
{
group: "desktop",
// Deliberately not "Desktop": a row sharing its group's label would make
// the cursor test unable to tell a reachable row from a header.
items: [{ label: "MacBook", run: () => undefined }],
label: "Desktop",
},
];
const body = (content: ParentNode, id: string) =>
content.querySelector<HTMLElement>(
`[data-group="${id}"] .${cls("pop-group-body")}`
);
const head = (content: ParentNode, id: string) =>
content.querySelector<HTMLElement>(
`[data-group="${id}"] .${cls("pop-group-head")}`
);
const openGroups = (content: ParentNode) =>
["phone", "desktop"].filter(
(id) => head(content, id)?.getAttribute("aria-expanded") === "true"
);
/**
* Open a grouped menu and hand back its content.
*
* The content, not the shell: `openPopover` scopes the arrow-key bindings to
* exactly that element, so a keypress dispatched from anywhere else is outside
* the scope and does nothing.
*/
function grouped(entries = GROUPS): HTMLElement {
const handle = createMenu(entries);
handle.open(anchor(), "below");
const content = shells()[0]?.querySelector<HTMLElement>(
`.${cls("pop-menu")}`
);
if (!content) {
throw new Error("no menu");
}
return content;
}
function key(from: Node, name: string): void {
from.dispatchEvent(
new KeyboardEvent("keydown", {
bubbles: true,
cancelable: true,
key: name,
})
);
}
it("opens with exactly one group expanded", () => {
// Never all-shut: a menu that opens as two headers and nothing to read makes
// you work out that they are headers before it will show you anything.
expect(openGroups(grouped())).toEqual(["phone"]);
});
it("honours the seeded group rather than always taking the first", () => {
const content = grouped([GROUPS[0], { ...GROUPS[1], open: true }]);
expect(openGroups(content)).toEqual(["desktop"]);
});
it("expands one at a time", () => {
const content = grouped();
head(content, "desktop")?.click();
expect(openGroups(content)).toEqual(["desktop"]);
});
it("collapses the open group when its own header is clicked", () => {
const content = grouped();
head(content, "phone")?.click();
expect(openGroups(content)).toEqual([]);
});
it("hides a shut group's rows but keeps them in the DOM", () => {
// Detaching them would put them out of reach of anything that reads the
// built menu back — the trap `frame-chrome.ts` documents for its own
// accordion, where a collapsed group's current-device mark went stale.
const content = grouped();
head(content, "desktop")?.click();
expect(body(content, "phone")?.classList.contains(cls("hidden"))).toBe(
true
);
expect(
content.querySelector(`[data-group="phone"] .${cls("pop-item")}`)
).not.toBeNull();
});
it("keeps the keyboard cursor out of a collapsed group", () => {
const content = grouped();
// Phone is open, so its two rows are reachable and Desktop's one is not.
// Both headers always are — they are the only way to open a group by key.
key(content, "ArrowDown");
const seen: string[] = [];
for (let i = 0; i < 4; i += 1) {
seen.push(
content
.querySelector<HTMLElement>("[data-pop-active]")
?.textContent?.trim() ?? "?"
);
key(content, "ArrowDown");
}
expect(seen).not.toContain("MacBook");
expect(seen).toContain("iPhone");
});
it("does not close the menu when a group is toggled", () => {
// A row's click closes the popover before it runs; a disclosure must not,
// or opening a group would dismiss the menu you were reading.
const content = grouped();
head(content, "desktop")?.click();
expect(shells()).toHaveLength(1);
});
it("closes the menu when a row inside a group is chosen", () => {
const run = vi.fn();
const content = grouped([
{ group: "phone", items: [{ label: "iPhone", run }], label: "Phone" },
]);
content.querySelector<HTMLElement>(`.${cls("pop-item")}`)?.click();
expect(run).toHaveBeenCalledTimes(1);
expect(shells()).toHaveLength(0);
});
it("drops a `node` entry in as its own row", () => {
// The escape hatch for the custom width × height form.
const custom = el("div", { class: "custom-form" });
const handle = createMenu([GROUPS[0], { node: custom }]);
handle.open(anchor(), "below");
expect(shells()[0].querySelector(".custom-form")).toBe(custom);
});
});
+211 -19
View File
@@ -299,8 +299,27 @@ function install(opts: PopoverOptions, handle: PopoverHandle): () => void {
*
* This is also the backstop for an anchor removed by something that did not
* route through `DesignPanel.renderBody`.
*
* **The popover's own scrolling is not a reflow, and treating it as one made
* every tall menu unscrollable.** Capture is what makes that a real hazard:
* the listener sees scroll events from *inside* this shell as well as around
* it. `reposition` is `place`, and `placePopover` starts by clearing
* `max-height` so it can measure the content unconstrained — which lets the
* box grow to full height, so the browser clamps `scrollTop` back toward zero
* before the cap is written again. The frame menu's twenty-seven rows scrolled
* and then snapped to the top on every wheel tick. This popover's own scroll
* cannot move its own anchor, so skipping it is not a heuristic: there is
* nothing to re-place.
*
* `handle.element`, not `levelOf` — the test has to be "*this* shell", not
* "any open shell". Popovers nest, and a nested one's anchor lives inside its
* parent's body: when that parent scrolls, the child's anchor really has
* moved, and the child really does have to follow it.
*/
const onReflow = (): void => {
const onReflow = (e: Event): void => {
if (e.type === "scroll" && handle.element.contains(e.target as Node)) {
return;
}
if (!opts.anchor.isConnected) {
handle.close("anchor-gone");
return;
@@ -399,8 +418,21 @@ function install(opts: PopoverOptions, handle: PopoverHandle): () => void {
* menu feel broken.
*/
/**
* The rows the cursor may land on.
*
* Filtered rather than un-marked, and that is the whole reason a collapsed group
* keeps its rows in the DOM at all. Stripping `data-pop-item` on collapse would
* work for the cursor and break everything else that reads the built menu back —
* the same trap `frame-chrome.ts` documents for its own accordion, where a
* `[data-preset]` sweep has to reach rows inside a shut group to keep their
* current-device mark from going stale. Hidden rows measure zero for
* `placePopover` either way.
*/
function items(content: HTMLElement): HTMLElement[] {
return Array.from(content.querySelectorAll<HTMLElement>(ITEM));
return Array.from(content.querySelectorAll<HTMLElement>(ITEM)).filter(
(node) => !node.closest(`.${cls("pop-group-body")}.${cls("hidden")}`)
);
}
function activeItem(content: HTMLElement): HTMLElement | null {
@@ -454,14 +486,159 @@ export interface MenuItem {
run: () => void;
}
/**
* A collapsible run of rows, one open at a time within a menu.
*
* A flat header is the right answer for three verbs under "Selection"; it is the
* wrong one for the twenty-two device presets, where the menu becomes a
* twenty-seven-item column that `placePopover` caps and scrolls. Grouping turns
* that into four lines you choose from.
*
* `open` seeds which one starts expanded. Exactly one does — an accordion that
* opens with everything shut presents a stack of headers and nothing to read —
* so an `open` that matches nothing falls back to the first group.
*/
export interface MenuGroup {
/** Stable id, written to `data-group`. What `open` is matched against. */
group: string;
items: MenuItem[];
label: string;
open?: boolean;
}
/**
* Arbitrary content, dropped in as its own row.
*
* The escape hatch for the one thing a list of buttons cannot express — the
* custom width × height form that ends both device menus. Deliberately narrow:
* anything that is a *row* should be a `MenuItem`, or the keyboard cursor and
* the disabled/selected grammar have to be reinvented per call site.
*/
export interface MenuNode {
node: HTMLElement;
}
/**
* A menu row. Headers and separators carry no `data-pop-item`, so the roving
* cursor steps over them without the keyboard code needing to know they exist.
* A group's *header* does carry one — it is the only way to open the group from
* the keyboard — and its rows carry theirs but are filtered out while shut. See
* {@link items}.
*/
export type MenuEntry = MenuItem | { separator: true } | { header: string };
export type MenuEntry =
| MenuItem
| { separator: true }
| { header: string }
| MenuGroup
| MenuNode;
function isMenuItem(entry: MenuEntry): entry is MenuItem {
return "label" in entry;
/**
* Is this entry a clickable row?
*
* Exported because narrowing a `MenuEntry` by hand is now a trap: `"label" in
* entry` was a complete test while rows were the only labelled thing, and a
* group has a label too. Callers that duck-typed it silently started admitting
* groups. `run` is what actually separates them.
*/
export function isMenuItem(entry: MenuEntry): entry is MenuItem {
return "label" in entry && "run" in entry;
}
function isMenuGroup(entry: MenuEntry): entry is MenuGroup {
return "group" in entry;
}
/**
* One collapsible group: a disclosure button and a body of rows.
*
* **A `<button>` and a `<div>`, not `<details>`/`<summary>`.** Two reasons, both
* learned elsewhere in this codebase rather than here. `chat/disclosure.ts`'s
* header has the first: host-page resets reach `<details>` and there is no way
* to opt out of the marker in every engine. `frame-chrome.ts`'s accordion has
* the second and it is the one that would bite here — `<summary>` swallows the
* activation of every nested control, and a group body is nothing *but* nested
* buttons.
*
* **Hidden, not detached.** A collapsed body keeps its rows in the tree so
* anything that reads the built menu back still finds them; `items` filters them
* out of the keyboard cursor instead. Both `hidden` and `aria-expanded` are
* seeds only — `openGroup` owns them from the first click on.
*/
function buildGroup(
entry: MenuGroup,
buildRow: (item: MenuItem) => HTMLElement,
menu: HTMLElement
): HTMLElement {
const head = el(
"button",
{
"aria-expanded": "false",
class: cls("pop-group-head"),
// On the header, so a group can be opened without a pointer. Its rows
// carry theirs too, and are filtered out of the cursor while shut.
"data-pop-item": "",
onClick: (e: Event) => {
// The row handlers close the menu; a disclosure must not, and must not
// be read as a choice by anything listening above it either.
e.stopPropagation();
openGroup(
menu,
head.getAttribute("aria-expanded") === "true" ? null : entry.group
);
},
type: "button",
},
[icon("chev-right", "xs"), el("span", { text: entry.label })]
);
return el("div", { class: cls("pop-group"), "data-group": entry.group }, [
head,
el(
"div",
{ class: `${cls("pop-group-body")} ${cls("hidden")}` },
entry.items.map(buildRow)
),
]);
}
/**
* Expand one group and shut the rest, or shut them all for `null`.
*
* Single-open because the whole point is to make a long list short again; two
* groups expanded at once is most of the way back to the flat column. Written
* across the built DOM rather than held as state, for the reason
* `frame-chrome.ts` gives for the same choice: the open group is re-derived on
* every open and left alone in between, so a field would be a second copy of
* something the tree already says.
*/
function openGroup(menu: HTMLElement, id: string | null): void {
for (const group of menu.querySelectorAll<HTMLElement>(
`.${cls("pop-group")}`
)) {
const on = group.dataset.group === id;
group
.querySelector(`.${cls("pop-group-head")}`)
?.setAttribute("aria-expanded", String(on));
group
.querySelector(`.${cls("pop-group-body")}`)
?.classList.toggle(cls("hidden"), !on);
}
}
/**
* Open the group the caller asked for, or the first one.
*
* Never all-shut: a menu that opens as three headers and nothing to read makes
* the reader work out that they are headers before it will show them anything.
* `frame-chrome.ts`'s accordion holds the same rule, and its test suite fences
* it as `never opens with every group shut`.
*/
function seedOpenGroup(menu: HTMLElement, entries: MenuEntry[]): void {
const groups = entries.filter(isMenuGroup);
if (!groups.length) {
return;
}
const wanted = groups.find((g) => g.open) ?? groups[0];
openGroup(menu, wanted.group);
}
export interface MenuOpenOptions {
@@ -494,10 +671,12 @@ export interface MenuHandle {
* which is also what stops a rebuilt row from leaving its old menu behind.
*
* Still deliberately not a general menu system: no submenus. It does group,
* though — headers and separators, because a menu carrying "Undo", "Create
* pull request" and "Open in VS Code" in one undifferentiated column reads as a
* junk drawer. `frame-chrome.ts`'s device menu still is not a client of this:
* it places itself against world-space canvas geometry and re-anchors on pan.
* though — headers, separators, and single-open accordions, because a menu
* carrying "Undo", "Create pull request" and "Open in VS Code" in one
* undifferentiated column reads as a junk drawer, and one carrying
* twenty-two device presets reads as a scroll. `frame-chrome.ts`'s device menu
* still is not a client of this: it places itself against world-space canvas
* geometry and re-anchors on pan.
*/
export function createMenu(entries: MenuEntry[]): MenuHandle {
/*
@@ -513,15 +692,7 @@ export function createMenu(entries: MenuEntry[]): MenuHandle {
let handle: PopoverHandle | null = null;
const menu = el("div", { class: cls("pop-menu"), role: "menu" });
for (const entry of entries) {
if (!isMenuItem(entry)) {
menu.append(
"header" in entry
? el("div", { class: cls("pop-head"), text: entry.header })
: el("div", { class: cls("pop-sep"), role: "separator" })
);
continue;
}
const buildRow = (entry: MenuItem): HTMLElement => {
const disabled = Boolean(entry.disabled);
const row = el(
"button",
@@ -551,8 +722,29 @@ export function createMenu(entries: MenuEntry[]): MenuHandle {
if (entry.hint) {
row.append(el("span", { class: cls("pop-item-hint"), text: entry.hint }));
}
menu.append(row);
return row;
};
for (const entry of entries) {
if (isMenuItem(entry)) {
menu.append(buildRow(entry));
continue;
}
if (isMenuGroup(entry)) {
menu.append(buildGroup(entry, buildRow, menu));
continue;
}
if ("node" in entry) {
menu.append(entry.node);
continue;
}
menu.append(
"header" in entry
? el("div", { class: cls("pop-head"), text: entry.header })
: el("div", { class: cls("pop-sep"), role: "separator" })
);
}
seedOpenGroup(menu, entries);
return {
close: () => handle?.close(),
+11
View File
@@ -55,6 +55,17 @@ export function placePopover(
// Measure unconstrained, then cap to whatever room the chosen side has.
menu.style.maxHeight = "";
/*
* And unpositioned, for the same reason one axis over.
*
* The `left` written at the end of this function is never cleared, so a second
* placement measures against `containing block - previous left`: an absolutely
* positioned box shrink-to-fits into that remainder, and anything whose width
* comes from its content reports the remainder rather than what it wanted.
* Every popover today is saved by a fixed width or a `min-width`, but the
* tooltip is content-sized and wraps, and this host re-places on every scroll.
*/
menu.style.left = "0px";
/*
* Content plus the chrome around it, because the cap lands on the border box.
*