mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-05-11 17:54:44 +00:00
494d3022f9
## Partial fix for #1128 — closes the gaps PR #1131 left behind PR #1131 was a partial fix for the packets-page layout chaos (merged 2026-05-06 ~01:55 UTC, then the issue was reopened by the maintainer). #1131 shipped Bug 4 (`--surface` definition), the `.path-popover` flip + lower z-index, the debounced re-measure for Bug 1, the `.filter-bar` row-gap + `.multi-select-trigger` truncation for Bug 3, the new z-index TOKENS, and a single-viewport E2E with five individual-component assertions. This PR closes everything else the issue body and the `specs/packets-layout-audit.md` audit asked for. ### What changed (per gap) **Gap A — apply the z-index scale (audit Section 2)** #1131 added `--z-dropdown` / `--z-popover` / `--z-modal` / `--z-tooltip` but explicitly left existing literal values in place. This PR renumbers the 7 dropdowns/popovers the audit named: | Selector | Before | After | |---|---:|---:| | `.col-toggle-menu` | 50 | `var(--z-dropdown)` (100) | | `.multi-select-menu` | 90 | `var(--z-dropdown)` | | `.region-dropdown-menu` | 90 | `var(--z-dropdown)` | | `.node-filter-dropdown` | 100 | `var(--z-dropdown)` | | `.fux-saved-menu` | `var(--z-tooltip)` (9200) | `var(--z-dropdown)` | | `.fux-ac-dropdown` | `var(--z-tooltip)` | `var(--z-dropdown)` | | `.hop-conflict-popover` | `var(--z-tooltip)` | `var(--z-popover)` (300) | `.fux-ctx-menu` deliberately retains the tooltip band — context menus must float above all toolbar UI. `.region-filter-options-menu` no longer exists in the source (was renamed `.region-dropdown-menu`). The `style.css` doc-block at the top is rewritten to record the applied scale and to point operators at the new lint. **Gap B — CSS-var lint (audit Section 5 #1, "single highest-value addition")** Adds `scripts/check-css-vars.js` (~70 lines). Walks `public/*.css`, extracts every `var(--name)` reference WITHOUT a fallback, asserts the name is defined in some `public/*.css`. References WITH a fallback are tolerated. Wired into CI in the `go-test` job before the JS unit tests. The red commit (`608d81f`) shipped this lint exiting 1 against the master tree — three undefined vars that bypassed earlier review: ``` public/style.css:2628 var(--text-primary) public/style.css:2675 var(--bg-hover) public/style.css:2924 var(--primary) ``` The green commit (`1369d1e`) defines those three as aliases in the :root block (`--text-primary` → `--text`, `--bg-hover` → `--hover-bg`, `--primary` → `--accent`). Light + dark themes inherit through the existing tokens. **Gap C — multi-viewport E2E (issue acceptance criterion)** Adds `test-issue-1128-multi-viewport-e2e.js` — sister of the existing single-viewport test. At each of three viewports (1280×900, 1080×800, 768×1024): - takes a screenshot to `e2e-screenshots/issue-1128-<viewport>.png` - asserts no two `.filter-group` siblings vertically overlap - on desktop+laptop, opens the Saved menu and the Types multi-select and asserts the dropdown does not vertically overlap any `.filter-group` below it Plus three viewport-agnostic assertions: - dropdown selectors compute z-index in `[100,199]` (`.col-toggle-menu`, `.multi-select-menu`, `.region-filter-options-menu`, `.fux-saved-menu`, `.fux-ac-dropdown`) - `.path-hops .hop / .hop-named / .arrow` compute `line-height ≤ 18px` - `.col-path` computes `height ≤ 28px` Wired into the e2e-test job after the existing #1128 test. **Gap D — Bug 5 polish (toolbar reorder)** Audit Section 3 Bug 5: swaps `filter-group-dropdowns` and `filter-group-toggles` in `public/packets.js` so time range + Group by Hash + ★ My Nodes sit next to the search input. Pure markup reorder. No CSS / no JS-handler changes. **Gap E — Bug 1 belt-and-suspenders** Audit Section 3 Bug 1 sub-bullets: - locks `.path-hops .hop / .hop-named / .arrow` to `line-height: 18px` so a chip with mixed font metrics cannot overflow the 22px host vertically and bleed into the row above - converts `.col-path { max-height: 28px }` → `height: 28px` because browsers widely ignore `max-height` on `<td>`s; the earlier rule was a no-op ### TDD discipline (red → green) ``` $ git log --oneline origin/master..HEAD68b0426fix(#1128): Bug 5 — toolbar group reorder (toggles before dropdowns)6d16e6ffix(#1128): apply z-index scale to dropdowns + Bug 1 chip line-height lockb9850c9fix(check-css-vars): strip /* ... */ comments before scanning1369d1efix(#1128): define --text-primary, --bg-hover, --primary aliases (lint green)0d4660ftest(#1128): multi-viewport E2E + wire CSS-var lint into CI (red commit)608d81ftest(#1128): add scripts/check-css-vars.js — fails on 3 undefined vars (red commit) ``` Both red commits (`608d81f`, `0d4660f`) were verified to fail locally before the green commits landed: - `608d81f` runs the lint and exits 1 on the three undefined vars listed above (proven against master). - `0d4660f` introduces the multi-viewport E2E and wires the lint into CI — the lint then fails the build on master, and the E2E z-scale assertion fails because pre-fix `.col-toggle-menu` is 50, the multi-selects are 90, etc. ### Acceptance criteria status From the original issue body: - ✅ Bug 4 root cause fixed (#1131 + this PR's lint guard) - ✅ Bug 1 chip-spill (debounced re-measure from #1131 + line-height lock + col-path height fix from this PR) - ✅ Bug 2 +N popover positioning (#1131) - ✅ Bug 3 toolbar overlap (#1131 + #1131 row-gap) - ✅ Bug 5 group reorder (this PR) - ✅ Z-index scale documented + applied (this PR) - ✅ E2E screenshots at multiple viewports (this PR) - ✅ Bounding-rect collision detection on visible interactive elements (this PR — `.filter-group` siblings + dropdown vs. toolbar) - ✅ CSS-var lint in CI (this PR) ### Why this is "Partial fix for #1128", not "Fixes #1128" Per `AGENTS.md` rule 34, automated closure is reserved for the operator after they verify on staging. The acceptance criteria above appear satisfied in code, but the user should confirm the visual outcome on staging before closing. ### Files changed - `scripts/check-css-vars.js` (new — ~70 lines) - `test-issue-1128-multi-viewport-e2e.js` (new) - `.github/workflows/deploy.yml` (lint step + e2e step wiring) - `public/style.css` (z-renumber, doc-block, Bug 1 polish, alias defs) - `public/packets.js` (Bug 5 reorder) Refs #1128, follows #1131 --------- Co-authored-by: Kpa-clawbot <bot@kpa-clawbot.local> Co-authored-by: openclaw-bot <bot@openclaw.local>