mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-05-11 21:24:42 +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>
132 lines
5.0 KiB
JavaScript
132 lines
5.0 KiB
JavaScript
#!/usr/bin/env node
|
|
/*
|
|
* scripts/check-css-vars.js — issue #1128 (audit Section 5 #1)
|
|
*
|
|
* Walks every public/*.css file (definitions + uses) AND every
|
|
* public/**\/*.{js,html} file (uses only) and asserts that every
|
|
* `var(--name)` reference WITHOUT a fallback resolves to a `--name:`
|
|
* definition in SOME public/*.css file.
|
|
*
|
|
* Why JS/HTML are scanned: the original Bug 4 in #1128 came from
|
|
* filter-ux.js shipping `style="background: var(--surface)"` while
|
|
* --surface was undefined. CSS-only scanning misses inline styles
|
|
* embedded in JS template literals and HTML attributes.
|
|
*
|
|
* References WITH a fallback like `var(--maybe, var(--always))` are
|
|
* tolerated; the fallback chain keeps them safe. Definitions still
|
|
* only come from CSS files (JS/HTML cannot define custom properties
|
|
* without runtime parsing we do not attempt).
|
|
*
|
|
* Exit code: 0 = clean, 1 = one or more undefined vars (with locations).
|
|
*
|
|
* Usage:
|
|
* node scripts/check-css-vars.js # default (lints public/)
|
|
* node scripts/check-css-vars.js --dir x # lint a different directory
|
|
*/
|
|
'use strict';
|
|
const fs = require('fs');
|
|
const path = require('path');
|
|
|
|
let dir = 'public';
|
|
for (let i = 2; i < process.argv.length; i++) {
|
|
if (process.argv[i] === '--dir' && process.argv[i + 1]) { dir = process.argv[++i]; }
|
|
}
|
|
|
|
if (!fs.existsSync(dir)) {
|
|
console.error('check-css-vars: directory not found: ' + dir);
|
|
process.exit(2);
|
|
}
|
|
|
|
// Recursively walk dir, returning files matching one of the given extensions.
|
|
// Skips node_modules and any vendor/ directory by name to keep the lint fast
|
|
// and focused on first-party code.
|
|
const SKIP_DIRS = new Set(['node_modules', 'vendor', '.git']);
|
|
function walk(root, exts) {
|
|
const out = [];
|
|
const stack = [root];
|
|
while (stack.length) {
|
|
const cur = stack.pop();
|
|
let entries;
|
|
try { entries = fs.readdirSync(cur, { withFileTypes: true }); }
|
|
catch (e) { continue; }
|
|
for (const ent of entries) {
|
|
const full = path.join(cur, ent.name);
|
|
if (ent.isDirectory()) {
|
|
if (SKIP_DIRS.has(ent.name)) continue;
|
|
stack.push(full);
|
|
} else if (ent.isFile()) {
|
|
const ext = path.extname(ent.name).toLowerCase();
|
|
if (exts.includes(ext)) out.push(full);
|
|
}
|
|
}
|
|
}
|
|
return out;
|
|
}
|
|
|
|
const cssFiles = walk(dir, ['.css']);
|
|
const codeFiles = walk(dir, ['.js', '.html', '.htm']);
|
|
|
|
if (!cssFiles.length) {
|
|
console.error('check-css-vars: no .css files found in ' + dir);
|
|
process.exit(2);
|
|
}
|
|
|
|
const defined = new Set();
|
|
const uses = []; // { file, line, name }
|
|
|
|
const defRe = /(?:^|[^a-zA-Z0-9_-])(--[a-zA-Z0-9_-]+)\s*:/g;
|
|
// Match var(--name) ONLY when the closing ')' immediately follows the name
|
|
// (optional whitespace). Anything else (a comma → fallback) is exempt.
|
|
const useRe = /var\(\s*(--[a-zA-Z0-9_-]+)\s*\)/g;
|
|
|
|
function scanCss(f) {
|
|
// Strip /* ... */ comments before scanning so doc-blocks that mention
|
|
// var(--name) as prose don't trigger false positives. Replace each
|
|
// comment span with whitespace to keep line numbers stable.
|
|
const raw = fs.readFileSync(f, 'utf8');
|
|
const stripped = raw.replace(/\/\*[\s\S]*?\*\//g, (m) => m.replace(/[^\n]/g, ' '));
|
|
const lines = stripped.split('\n');
|
|
for (let i = 0; i < lines.length; i++) {
|
|
const line = lines[i];
|
|
let m;
|
|
defRe.lastIndex = 0;
|
|
while ((m = defRe.exec(line)) !== null) defined.add(m[1]);
|
|
useRe.lastIndex = 0;
|
|
while ((m = useRe.exec(line)) !== null) uses.push({ file: f, line: i + 1, name: m[1] });
|
|
}
|
|
}
|
|
|
|
function scanCode(f) {
|
|
// For JS / HTML we collect USES only. We also strip /* */ and //
|
|
// line comments (JS) and <!-- --> (HTML) so doc prose mentioning
|
|
// var(--name) doesn't false-positive. Definitions in JS/HTML are
|
|
// not supported (would require runtime parsing).
|
|
let raw = fs.readFileSync(f, 'utf8');
|
|
raw = raw.replace(/\/\*[\s\S]*?\*\//g, (m) => m.replace(/[^\n]/g, ' '));
|
|
raw = raw.replace(/<!--[\s\S]*?-->/g, (m) => m.replace(/[^\n]/g, ' '));
|
|
// Strip // line comments (best-effort; harmless if it nicks a URL since
|
|
// we only care about var(--…) tokens that follow on the same line).
|
|
raw = raw.replace(/(^|[^:])\/\/[^\n]*/g, (m, p1) => p1 + ' '.repeat(m.length - p1.length));
|
|
const lines = raw.split('\n');
|
|
for (let i = 0; i < lines.length; i++) {
|
|
const line = lines[i];
|
|
let m;
|
|
useRe.lastIndex = 0;
|
|
while ((m = useRe.exec(line)) !== null) uses.push({ file: f, line: i + 1, name: m[1] });
|
|
}
|
|
}
|
|
|
|
for (const f of cssFiles) scanCss(f);
|
|
for (const f of codeFiles) scanCode(f);
|
|
|
|
const undef = uses.filter(u => !defined.has(u.name));
|
|
if (undef.length) {
|
|
console.error('check-css-vars: ' + undef.length + ' undefined CSS variable reference(s):');
|
|
for (const u of undef) console.error(' ' + u.file + ':' + u.line + ' var(' + u.name + ')');
|
|
console.error('Fix: define the variable in :root, or use var(' + undef[0].name + ', <fallback>).');
|
|
process.exit(1);
|
|
}
|
|
console.log('check-css-vars: OK — ' + uses.length + ' var() refs across ' +
|
|
(cssFiles.length + codeFiles.length) + ' files (' + cssFiles.length + ' css, ' +
|
|
codeFiles.length + ' js/html), ' + defined.size + ' definitions, 0 undefined.');
|