From 0ae2a62229fcec18440d980e7d34450c46dde101 Mon Sep 17 00:00:00 2001 From: Jokin Date: Sun, 16 Aug 2026 03:20:55 +0200 Subject: [PATCH] Stage 4a: context menu is pure 7.css; retire the lookalike and the false claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The menu now renders 7.css's real contract — ul[role=menu] > li[role=menuitem] > button for actions, input[type=checkbox] + label for the checkable column toggles, and dividers as .has-divider on the PRECEDING item (7.css draws them as an ::after bevel, not as a node; the service's separator entries collapse onto their predecessor at render time). With the markup right, the framework supplies everything the hand-written version imitated or lacked: the left gutter stripe, bevelled separators, the gradient selection band, the checked box, and disabled/submenu styling we previously had none of. Deleted: the w7-menu lookalike (both the button-slab stripper in `skin` and the whole hand-authored Aero set in `skinfix`) and its w7-menu/w7-menu-sep hooks. The "7.css has no menu component" claims in styles.css and architecture.md were FALSE — dist/gui/menu.css is a full component; the old markup (div > button) simply matched none of it. Both corrected to state the real contract and the trap. Keyboard now follows the platform instead of a highlight class: arrows move REAL focus between items — 7.css styles :focus-visible identically to :hover, and keyboard-initiated programmatic focus counts — and Enter/Space activate buttons natively; only labels need a synthetic click. This also makes the menu honest to screen readers. Dark mode: menu.css hardcodes light literals (color:initial, white bevel halves, the #b3d3f9 checked-box ring); restated in skinfix's dark arm like the ListView's. Also fixed, caught BY the harness: the Stage 3 frame sizing rule was keyed on bare `.window`, forcing 100vh height onto every 7.css window (the harness's demo windows blew up to full page). It is now keyed on `w7-frame`, a hook only the three real window roots carry. Verified zoomed in light and dark against 7.css's reference markup: gutter, dividers, checkmark box, grey right-aligned accelerator all match; bundle 490.92 kB. Co-Authored-By: Claude --- docs/architecture.md | 30 ++-- ui/dev/harness.html | 34 ++-- ui/src/app/app.html | 2 +- .../connection-manager.html | 2 +- ui/src/app/context-menu.ts | 156 ++++++++++-------- ui/src/app/editor-window/editor-window.html | 2 +- ui/src/styles.css | 79 ++++----- 7 files changed, 152 insertions(+), 153 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 016279f..8c71cfc 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -304,22 +304,22 @@ one) as ~20 `--w7-*` overrides, duplicated forced + system like every other dark block. `--w7-font` is restated as a px `font` shorthand so `data-text` still scales and 7.css's `pt` shorthand stops resetting weight/line-height. -**Markup hooks** are additive and inert under `modern`: `role="progressbar"` -(+aria) on the transfer bars, `has-scrollbar` on scroll containers (7.css only -styles `::-webkit-scrollbar` under that class), `w7-toolbar` on the three -header rows (their buttons go flat — Explorer's command bar was flat, unlike -dialog push buttons), `w7-btn` on the 10 dialog action buttons, `w7-menu` (+ -`w7-menu-sep`) on the HTML context menu. +**Markup hooks**: `role="progressbar"` (+aria) on the transfer bars, +`has-scrollbar` on scroll containers (7.css only styles `::-webkit-scrollbar` +under that class), `w7-toolbar` on the three header rows (their buttons go +flat — Explorer's command bar was flat, unlike dialog push buttons), `w7-btn` +on the 10 dialog action buttons. -**Context menu**: 7.css ships no menu component, so the Aero menu is -hand-authored across both layers — the row ` - -
- - - +
ours — context-menu.ts output (buttons, divider, accelerator, checkables)
+ diff --git a/ui/src/app/app.html b/ui/src/app/app.html index f5da772..ec12cfd 100644 --- a/ui/src/app/app.html +++ b/ui/src/app/app.html @@ -1,7 +1,7 @@ -
+
@if (!rpc.available) {
diff --git a/ui/src/app/connection-manager/connection-manager.html b/ui/src/app/connection-manager/connection-manager.html index eee5f9f..803b37b 100644 --- a/ui/src/app/connection-manager/connection-manager.html +++ b/ui/src/app/connection-manager/connection-manager.html @@ -1,4 +1,4 @@ -
+
: separators collapse onto the item BEFORE them. */ +interface MenuRow { + item: MenuItem; + hasDivider: boolean; +} + /** * The panel right-click menu. Replaces electrobun's native ContextMenu, which * is a no-op on Linux — see ContextMenuService for the why. Emits the same * `ctx-*` / `col:*` action strings the native menu did. + * + * The markup is 7.css's menu contract EXACTLY — `ul[role=menu] > + * li[role=menuitem] > button` for actions, `> input[type=checkbox] + label` + * for the checkable column toggles, and dividers as `.has-divider` on the + * PRECEDING item (7.css draws them as an ::after bevel, not as a node). Get + * any of that wrong and 7.css silently styles nothing: an earlier version + * rendered div > button, concluded "7.css has no menu component", and + * hand-built a lookalike. It does have one; the selectors just never matched. + * + * Keyboard follows the platform: arrows move REAL focus between items (7.css + * styles :focus-visible identically to :hover), Enter/Space on a button + * activate it natively; only labels need a synthetic click. */ @Component({ selector: 'app-context-menu', - imports: [NgIcon], template: ` @if (menu.request(); as req) {
- + } `, // Closing on scroll/blur matches native menu behavior: the anchor row moves @@ -77,24 +78,35 @@ const EDGE_GAP = 6; export class ContextMenu { readonly menu = inject(ContextMenuService); private readonly focus = inject(FocusService); - private readonly menuEl = viewChild>('menuEl'); + private readonly menuEl = viewChild>('menuEl'); - readonly activeIndex = signal(-1); + protected readonly menuWidth = MENU_W; + + /** Model items → rendered rows: a separator becomes has-divider on its predecessor. */ + readonly rows = computed(() => { + const items = this.menu.request()?.items ?? []; + const rows: MenuRow[] = []; + for (const item of items) { + if (item.type === 'separator') { + if (rows.length) rows[rows.length - 1].hasDivider = true; + } else { + rows.push({ item, hasDivider: false }); + } + } + return rows; + }); /** Click point, flipped/clamped so the menu never overflows the viewport. */ readonly position = computed(() => { const req = this.menu.request(); if (!req) return { x: 0, y: 0 }; + const rows = this.rows(); const height = - req.items.reduce((sum, i) => sum + (i.type === 'separator' ? SEPARATOR_H : ITEM_H), 0) + 8; + rows.length * ITEM_H + rows.filter((r) => r.hasDivider).length * DIVIDER_H + 6; const x = - req.x + MENU_W > window.innerWidth - EDGE_GAP - ? Math.max(EDGE_GAP, req.x - MENU_W) - : req.x; + req.x + MENU_W > window.innerWidth - EDGE_GAP ? Math.max(EDGE_GAP, req.x - MENU_W) : req.x; const y = - req.y + height > window.innerHeight - EDGE_GAP - ? Math.max(EDGE_GAP, req.y - height) - : req.y; + req.y + height > window.innerHeight - EDGE_GAP ? Math.max(EDGE_GAP, req.y - height) : req.y; return { x, y }; }); @@ -108,11 +120,13 @@ export class ContextMenu { close(): void { if (!this.menu.request()) return; this.menu.close(); - this.activeIndex.set(-1); // a fresh menu starts with nothing highlighted this.focus.bump(); } - pick(item: MenuItem): void { + pick(item: MenuItem, event?: Event): void { + // labels: don't let the click also toggle the real checkbox — the fresh + // `checked` state comes back through settings when the menu reopens + event?.preventDefault(); if (!item.action) return; const action = item.action; this.close(); @@ -126,7 +140,6 @@ export class ContextMenu { } onKeydown(event: KeyboardEvent): void { - const items = this.menu.request()?.items ?? []; switch (event.key) { case 'Escape': event.preventDefault(); @@ -135,33 +148,38 @@ export class ContextMenu { break; case 'ArrowDown': event.preventDefault(); - this.step(items, 1); + this.step(1); break; case 'ArrowUp': event.preventDefault(); - this.step(items, -1); + this.step(-1); break; case 'Enter': case ' ': { - event.preventDefault(); - const item = items[this.activeIndex()]; - if (item) this.pick(item); + // buttons activate natively from their own focus; labels don't, so + // synthesize the click (which routes through pick via the template) + const active = document.activeElement; + if (active instanceof HTMLLabelElement && this.menuEl()?.nativeElement.contains(active)) { + event.preventDefault(); + active.click(); + } break; } } } - /** Move the highlight by one selectable item, skipping separators, wrapping. */ - private step(items: MenuItem[], delta: number): void { - const count = items.length; - if (!count) return; - let index = this.activeIndex(); - for (let hops = 0; hops < count; hops++) { - index = (index + delta + count) % count; - if (items[index].type !== 'separator') { - this.activeIndex.set(index); - return; - } - } + /** + * Move REAL focus by one item, wrapping. 7.css styles :focus-visible the + * same as :hover, and keyboard-initiated programmatic focus counts as + * focus-visible, so the highlight comes from the framework for free. + */ + private step(delta: number): void { + const root = this.menuEl()?.nativeElement; + if (!root) return; + const targets = Array.from(root.querySelectorAll('li > button, li > label')); + if (!targets.length) return; + const current = targets.indexOf(document.activeElement as HTMLElement); + const next = current < 0 && delta < 0 ? targets.length - 1 : (current + delta + targets.length) % targets.length; + targets[next].focus(); } } diff --git a/ui/src/app/editor-window/editor-window.html b/ui/src/app/editor-window/editor-window.html index 694340d..305d799 100644 --- a/ui/src/app/editor-window/editor-window.html +++ b/ui/src/app/editor-window/editor-window.html @@ -1,4 +1,4 @@ -
+
@if (!ctx) {
diff --git a/ui/src/styles.css b/ui/src/styles.css index e66010c..cb9ec87 100644 --- a/ui/src/styles.css +++ b/ui/src/styles.css @@ -580,18 +580,6 @@ body { content: none; } - /* Context-menu rows are