mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-08-28 13:24:10 +00:00
## Summary Polish follow-ups for the #1644/#1645 observer-comparison redesign — addresses all 5 parent visual-review findings + 3 Tufte additions in one PR. Fixes #1646. ## Coalesced fix list (status: ✅ all landed) | # | Tag | Item | Fix | Evidence | |---|---|---|---|---| | 1 | [both] | Native checkboxes were bare white squares against dark theme | Global `input[type=checkbox]/[type=radio] { accent-color: var(--accent) }` + `color-scheme: dark` on dark theme blocks. UA renders themed checkboxes everywhere now. | `screenshots-1646/after-observers-selected-dark.jpg` vs `before-observers-selected-dark.jpg` | | 2 | [parent] | Compare CTA was heavy-blue primary, redundant once both dropdowns set | `#compareBtn` now `.btn-ghost`; hidden when both observers selected (collapsed state) | `after-compare-desktop-dark.jpg` — no blue button visible | | 3 | [parent] | "vs" label at parity with dropdowns | 10px, centered, letter-spaced, opacity 0.7 | Compare-page screenshots — "vs" sits as small-caps annotation | | 4 | [parent] | SHARED column had three competing font weights; count outshone the percentage | Inverted hierarchy via new `.compare-strip-mid-pct` + `.compare-strip-mid-pct-unit`. 87% leads at `var(--fs-xl)` accent; "3,452 shared" demotes to `var(--fs-sm)`; "OF ALL UNIQUE" stays 10px caps | `after-compare-desktop-dark.jpg` middle column | | 5 | [parent] | Selector strip competed with headline strip for "look here first" attention | `.compare-controls.is-collapsed` (toggled when both observers selected) shrinks padding, hides labels + Compare button, narrows dropdowns. A/B swap still reachable | `after-compare-desktop-light.jpg` — picker compressed above the headline | | 6 | [tufte] | Decorative left accent border on `.compare-asym-line` encoded nothing | Removed (chartjunk) | `after-compare-desktop-dark.jpg` — squared cards | | 7 | [tufte] | Decorative left green border on `.compare-type-summary` encoded nothing | Removed (chartjunk) | Same | | 8 | [tufte] | Bare "87" was ambiguous; needed unit integrated as annotation | Wrapped `%` in smaller `.compare-strip-mid-pct-unit` — words and graphics co-located | Middle column hierarchy | ## Push-backs / scope discipline - **Did NOT remove the selector strip entirely.** Parent rule + operator UX: A/B swap must remain reachable. Collapsing > removing. - **Did NOT introduce a custom checkbox widget.** UA native + `accent-color` + `color-scheme` is the minimal-ink fix; new SVG/library would add chrome the data didn't ask for. - **Did NOT add new color tokens.** All restyling uses existing `--accent`, `--text`, `--text-muted`, `--surface-1/2`, `--border`. ## TDD - Red commit: `2863cfb3 test(#1646): RED — assertions for compare-polish ...` — `test-issue-1646-compare-polish.js` 9 assertions, all FAIL on master. - Green commits (3 logical groups): 1. `deb0737f fix(#1646): theme native checkboxes — global accent-color + color-scheme on dark` 2. `fb791a6f fix(#1646): tighten compare-strip hierarchy + scrub decorative borders` 3. `8033ac36 fix(#1646): ghost-style Compare CTA + collapse the picker once both observers chosen` Final: `node test-issue-1646-compare-polish.js` → 9/9 pass; `node test-issue-1644-redesign.js` → 13/13 pass (no regression); `node test-compare-overlap.js` → 6/6 pass; `node test-frontend-helpers.js` → 611/611 pass. ## Visual verification All staging-validated via local headless chromium against the hot-swapped files at `http://20.x.y.z` (staging). Surface matrix covered: - Observers list — desktop dark (with 2 rows checked) — themed accent on checkboxes - Observers list — mobile 375px dark - Compare page — desktop light + dark - Compare page — mobile 375px dark **Reviewer note: screenshot artifacts were captured locally (sandbox does not have a GitHub UI session for attachment upload).** Paths below — pull these from the same workspace location if you want to inspect: ``` screenshots-1646/before-observers-desktop-dark.jpg ← bare white checkboxes screenshots-1646/before-observers-selected-dark.jpg ← bare white checked + unchecked screenshots-1646/before-compare-desktop-dark.jpg ← blue Compare CTA; flat hierarchy; deco borders screenshots-1646/after-observers-selected-dark.jpg ← themed checkboxes screenshots-1646/after-observers-mobile-dark.jpg screenshots-1646/after-compare-desktop-light.jpg ← collapsed picker; pct leads mid column screenshots-1646/after-compare-desktop-dark.jpg screenshots-1646/after-compare-mobile-dark.jpg ``` No raw `MEDIA:` UUIDs in this body — that was the mistake on #1645 and is not being repeated. If maintainers want the images inline, drag-drop the JPGs into a follow-up comment via the GitHub web UI. ## Risk Low. Pure CSS + one class-toggle in `compare.js`'s `updateBtn` (idempotent, no race, no event loop change). `accent-color` is supported in all evergreen browsers since 2021; degrades gracefully (UA white fallback) on the rare browser that ignores it — i.e. exactly the current-master state. --------- Co-authored-by: openclaw-bot <openclaw-bot@users.noreply.github.com> Co-authored-by: openclaw-bot <bot@openclaw.local> Co-authored-by: clawbot <clawbot@users.noreply.github.com>
This commit is contained in:
Kpa-clawbot
co-authored by
openclaw-bot
openclaw-bot
clawbot
openclaw-bot
openclaw-bot
clawbot
parent
c93ae67ed0
commit
167af54eb8
+34
-16
@@ -132,6 +132,13 @@ if (typeof window !== 'undefined') {
|
||||
routeFilter = 'all';
|
||||
}
|
||||
|
||||
// #1646 round-2 — single shared "should we run a comparison?" predicate
|
||||
// used by every auto-run call site so guards cannot drift apart.
|
||||
// URL-prepopulated ?a=X&b=X (same observer in both slots) returns false.
|
||||
function isComparisonReady() {
|
||||
return !!(selA && selB && selA !== selB);
|
||||
}
|
||||
|
||||
async function loadObservers() {
|
||||
try {
|
||||
var data = await api('/observers', { ttl: CLIENT_TTL.observers });
|
||||
@@ -139,7 +146,7 @@ if (typeof window !== 'undefined') {
|
||||
return (a.name || a.id).localeCompare(b.name || b.id);
|
||||
});
|
||||
renderControls();
|
||||
if (selA && selB) runComparison();
|
||||
if (isComparisonReady()) runComparison();
|
||||
} catch (e) {
|
||||
document.getElementById('compareControls').innerHTML =
|
||||
'<div class="text-muted" style="padding:20px">Error loading observers: ' + escapeHtml(e.message) + '</div>';
|
||||
@@ -161,14 +168,15 @@ if (typeof window !== 'undefined') {
|
||||
'<div class="compare-selector">' +
|
||||
'<div class="compare-select-group">' +
|
||||
'<label for="compareObsA">Observer A</label>' +
|
||||
'<span class="compare-select-id" aria-hidden="true">A</span>' +
|
||||
'<select id="compareObsA" class="compare-select">' + optionsHtml + '</select>' +
|
||||
'</div>' +
|
||||
'<span class="compare-vs">vs</span>' +
|
||||
'<div class="compare-select-group">' +
|
||||
'<label for="compareObsB">Observer B</label>' +
|
||||
'<span class="compare-select-id" aria-hidden="true">B</span>' +
|
||||
'<select id="compareObsB" class="compare-select">' + optionsHtml + '</select>' +
|
||||
'</div>' +
|
||||
'<button id="compareBtn" class="compare-btn" disabled>Compare</button>' +
|
||||
'<div class="compare-select-group">' +
|
||||
'<label for="compareRouteFilter">Packet Type</label>' +
|
||||
'<select id="compareRouteFilter" class="compare-select">' +
|
||||
@@ -181,7 +189,6 @@ if (typeof window !== 'undefined') {
|
||||
|
||||
var ddA = document.getElementById('compareObsA');
|
||||
var ddB = document.getElementById('compareObsB');
|
||||
var btn = document.getElementById('compareBtn');
|
||||
|
||||
if (selA) ddA.value = selA;
|
||||
if (selB) ddB.value = selB;
|
||||
@@ -193,15 +200,26 @@ if (typeof window !== 'undefined') {
|
||||
if (comparisonResult) runComparison();
|
||||
});
|
||||
|
||||
// #1646 — single source of truth for "should we run a comparison?".
|
||||
// Called from change handlers and from the initial pre-populated
|
||||
// path in loadObservers(). The picker collapse rule (.is-collapsed)
|
||||
// is the ONLY DOM hook for state — no parallel data-collapsed attr.
|
||||
function updateBtn() {
|
||||
selA = ddA.value || null;
|
||||
selB = ddB.value || null;
|
||||
btn.disabled = !selA || !selB || selA === selB;
|
||||
var wrap = document.getElementById('compareControls');
|
||||
var ready = isComparisonReady();
|
||||
if (wrap) wrap.classList.toggle('is-collapsed', ready);
|
||||
renderBreadcrumbs();
|
||||
}
|
||||
ddA.addEventListener('change', updateBtn);
|
||||
ddB.addEventListener('change', updateBtn);
|
||||
btn.addEventListener('click', function () { runComparison(); });
|
||||
function onChange() {
|
||||
updateBtn();
|
||||
// change events only fire when value actually changes, so any
|
||||
// ready transition that lands here came from a real user action.
|
||||
if (isComparisonReady()) runComparison();
|
||||
}
|
||||
ddA.addEventListener('change', onChange);
|
||||
ddB.addEventListener('change', onChange);
|
||||
updateBtn();
|
||||
}
|
||||
|
||||
@@ -322,25 +340,25 @@ if (typeof window !== 'undefined') {
|
||||
content.innerHTML =
|
||||
'<div class="compare-results">' +
|
||||
// Headline strip — A | shared | B above a single proportional bar.
|
||||
// One row of large numbers + one shared-axis bar = the comparison
|
||||
// in a single glance. Replaces three card-boxes that forced the
|
||||
// eye to do mental subtraction.
|
||||
// All three cells lead with their percentage so the row reads in
|
||||
// one unit (Tufte: show data variation, not design variation).
|
||||
// #1646
|
||||
'<section class="compare-strip" aria-label="Packet overlap summary">' +
|
||||
'<div class="compare-strip-row">' +
|
||||
'<div class="compare-strip-side" data-view="onlyA" role="button" tabindex="0" aria-label="Show only ' + nameA + ' packets">' +
|
||||
'<div class="compare-strip-name">' + nameA + '</div>' +
|
||||
'<div class="compare-strip-count">' + stats.totalA.toLocaleString() + '</div>' +
|
||||
'<div class="compare-strip-sub">' + r.onlyA.length.toLocaleString() + ' only here (' + pctA + '%)</div>' +
|
||||
'<div class="compare-strip-side-pct">' + pctA + '<span class="compare-strip-side-pct-unit">%</span></div>' +
|
||||
'<div class="compare-strip-sub">' + r.onlyA.length.toLocaleString() + ' only here</div>' +
|
||||
'</div>' +
|
||||
'<div class="compare-strip-mid" data-view="both" role="button" tabindex="0" aria-label="Show shared packets">' +
|
||||
'<div class="compare-strip-mid-label">shared</div>' +
|
||||
'<div class="compare-strip-mid-pct">' + pctBoth + '<span class="compare-strip-mid-pct-unit">%</span></div>' +
|
||||
'<div class="compare-strip-mid-count">' + r.both.length.toLocaleString() + '</div>' +
|
||||
'<div class="compare-strip-sub">' + pctBoth + '% of all unique</div>' +
|
||||
'<div class="compare-strip-mid-label">of all unique</div>' +
|
||||
'</div>' +
|
||||
'<div class="compare-strip-side compare-strip-side-b" data-view="onlyB" role="button" tabindex="0" aria-label="Show only ' + nameB + ' packets">' +
|
||||
'<div class="compare-strip-name">' + nameB + '</div>' +
|
||||
'<div class="compare-strip-count">' + stats.totalB.toLocaleString() + '</div>' +
|
||||
'<div class="compare-strip-sub">' + r.onlyB.length.toLocaleString() + ' only here (' + pctB + '%)</div>' +
|
||||
'<div class="compare-strip-side-pct">' + pctB + '<span class="compare-strip-side-pct-unit">%</span></div>' +
|
||||
'<div class="compare-strip-sub">' + r.onlyB.length.toLocaleString() + ' only here</div>' +
|
||||
'</div>' +
|
||||
'</div>' +
|
||||
// Single shared-axis diff bar. Width is exact proportion.
|
||||
|
||||
+108
-21
@@ -212,6 +212,7 @@
|
||||
When changing dark theme variables, update BOTH blocks below. */
|
||||
@media (prefers-color-scheme: dark) {
|
||||
:root:not([data-theme="light"]) {
|
||||
color-scheme: dark;
|
||||
--status-green: #22c55e;
|
||||
--status-yellow: #eab308;
|
||||
--status-red: #ef4444;
|
||||
@@ -244,6 +245,7 @@
|
||||
}
|
||||
/* ⚠️ DARK THEME VARIABLES — KEEP IN SYNC with @media block above */
|
||||
[data-theme="dark"] {
|
||||
color-scheme: dark;
|
||||
/* Brighter link-strength hues for contrast on dark map tiles + surfaces. */
|
||||
--link-strong: #3fb950;
|
||||
--link-medium: #d29922;
|
||||
@@ -399,7 +401,6 @@ button.ch-item {
|
||||
.ch-scroll-btn,
|
||||
.chooser-btn,
|
||||
.clock-filter-btn,
|
||||
.compare-btn,
|
||||
.copy-link-btn,
|
||||
.alab-btn {
|
||||
min-height: 48px;
|
||||
@@ -421,7 +422,6 @@ button.ch-item {
|
||||
.ch-scroll-btn:active,
|
||||
.chooser-btn:active,
|
||||
.clock-filter-btn:active,
|
||||
.compare-btn:active,
|
||||
.copy-link-btn:active,
|
||||
.alab-btn:active {
|
||||
background: var(--row-hover);
|
||||
@@ -450,6 +450,16 @@ input[type="week"] {
|
||||
touch-action: manipulation;
|
||||
}
|
||||
|
||||
/* Theme the UA-native checkbox + radio so they pick up the page accent
|
||||
* instead of rendering as bare white squares against the dark theme.
|
||||
* Pairs with `color-scheme: dark` on the dark-theme rule so the box
|
||||
* surround also re-skins. (Tufte #1646 — bare native widgets are
|
||||
* chartjunk-by-omission: they advertise the chrome, not the data.) */
|
||||
input[type="checkbox"],
|
||||
input[type="radio"] {
|
||||
accent-color: var(--accent);
|
||||
}
|
||||
|
||||
/* Visible :active states — touch devices have no hover, so :active is the
|
||||
primary feedback channel. Use opacity + slight scale + background shift so
|
||||
the press is felt even when the user's finger covers the control. */
|
||||
@@ -3411,7 +3421,14 @@ button.region-pill-active:hover { opacity: 0.85; color: #fff; }
|
||||
* diff-strip (Tufte's "show the data") makes the asymmetry
|
||||
* legible at a glance; the textual cards are demoted to subtext.
|
||||
*/
|
||||
.compare-page { padding: var(--space-md); max-width: 1200px; margin: 0 auto; }
|
||||
.compare-page { padding: var(--space-md); max-width: 1200px; margin: 0 auto;
|
||||
/* #1646 mobile: reserve space below the floating bottom-nav at ≤768px.
|
||||
* --bottom-nav-reserve is 0px on desktop, 56px + safe-area on mobile,
|
||||
* so this rule is a no-op for desktop and adds the exact clearance
|
||||
* needed on phones — the Shared-Types pill row + tabs were getting
|
||||
* eaten by the nav. (Tufte: don't reserve space we don't need.) */
|
||||
padding-bottom: calc(var(--space-md) + var(--bottom-nav-reserve, 0px));
|
||||
}
|
||||
.compare-page .page-header {
|
||||
display: flex; align-items: center; justify-content: flex-start;
|
||||
gap: var(--space-sm); margin-bottom: var(--space-sm);
|
||||
@@ -3433,6 +3450,17 @@ button.region-pill-active:hover { opacity: 0.85; color: #fff; }
|
||||
padding: var(--space-sm) var(--space-md);
|
||||
margin-bottom: var(--space-md);
|
||||
}
|
||||
/* Once both observers are picked, the headline strip below is the
|
||||
* answer. Demote the picker: tighter padding, smaller labels.
|
||||
* Operators can still swap A/B without leaving the page. The "A"/"B"
|
||||
* referent stays visible (issue #1646 Tufte: erase labels, not
|
||||
* referents). */
|
||||
.compare-controls.is-collapsed {
|
||||
padding: 4px var(--space-md);
|
||||
}
|
||||
.compare-controls.is-collapsed .compare-select-group label { display: none; }
|
||||
.compare-controls.is-collapsed .compare-select { font-size: var(--fs-sm); padding: 4px 8px; }
|
||||
.compare-controls.is-collapsed .compare-select-id { display: inline-block; }
|
||||
.compare-selector {
|
||||
display: flex; align-items: flex-end; gap: var(--space-sm); flex-wrap: wrap;
|
||||
}
|
||||
@@ -3441,6 +3469,14 @@ button.region-pill-active:hover { opacity: 0.85; color: #fff; }
|
||||
font-size: 10px; font-weight: 700; text-transform: uppercase;
|
||||
letter-spacing: 0.6px; color: var(--text-muted);
|
||||
}
|
||||
/* The "A" / "B" badge is hidden when the labels are visible (expanded
|
||||
* state) and revealed when collapsed, so the dropdown row never loses
|
||||
* its A/B referent. */
|
||||
.compare-select-id {
|
||||
display: none;
|
||||
font-size: 10px; font-weight: 700; color: var(--text-muted);
|
||||
letter-spacing: 0.6px; padding: 2px 0;
|
||||
}
|
||||
.compare-select {
|
||||
padding: 6px 10px; border: 1px solid var(--border); border-radius: var(--radius-sm);
|
||||
background: var(--input-bg); color: var(--text); font-size: var(--fs-sm);
|
||||
@@ -3448,19 +3484,16 @@ button.region-pill-active:hover { opacity: 0.85; color: #fff; }
|
||||
}
|
||||
.compare-select:focus { border-color: var(--accent); outline: none; }
|
||||
.compare-vs {
|
||||
font-size: 12px; font-weight: 700; color: var(--text-muted);
|
||||
text-transform: uppercase; letter-spacing: 1px;
|
||||
padding: 0 4px 8px;
|
||||
font-size: 10px; font-weight: 700; color: var(--text-muted);
|
||||
text-transform: uppercase; letter-spacing: 1.2px;
|
||||
padding: 0 4px;
|
||||
align-self: center;
|
||||
opacity: 0.7;
|
||||
}
|
||||
/* Compare-now action — primary accent button, reuses .btn-primary visuals
|
||||
* but lives inside the controls strip with consistent height. */
|
||||
.compare-btn {
|
||||
padding: 6px 14px; border: 1px solid var(--accent); border-radius: var(--radius-sm);
|
||||
background: var(--accent); color: #fff; font-size: var(--fs-sm); font-weight: 700;
|
||||
cursor: pointer; transition: background 120ms, opacity 120ms;
|
||||
}
|
||||
.compare-btn:hover:not(:disabled) { background: var(--accent-hover); border-color: var(--accent-hover); }
|
||||
.compare-btn:disabled { opacity: 0.5; cursor: not-allowed; }
|
||||
/* The Compare CTA was removed in #1646 — comparison auto-runs when
|
||||
* both observers are chosen. The legacy .compare-btn touch-target
|
||||
* declaration was deleted with it; surrounding form controls already
|
||||
* carry their own 48px-min-height rule. */
|
||||
|
||||
.compare-results { margin-top: var(--space-md); }
|
||||
|
||||
@@ -3488,10 +3521,16 @@ button.region-pill-active:hover { opacity: 0.85; color: #fff; }
|
||||
font-size: var(--fs-sm); font-weight: 700; color: var(--text);
|
||||
white-space: nowrap; overflow: hidden; text-overflow: ellipsis;
|
||||
}
|
||||
.compare-strip-count {
|
||||
/* The side cells lead with the percentage, matching the middle cell
|
||||
* (one unit across the row, no design variation — issue #1646). */
|
||||
.compare-strip-side-pct {
|
||||
font-family: var(--mono);
|
||||
font-size: var(--fs-xl); font-weight: 700;
|
||||
color: var(--text); font-variant-numeric: tabular-nums;
|
||||
line-height: 1;
|
||||
}
|
||||
.compare-strip-side-pct-unit {
|
||||
font-size: 0.6em; margin-left: 1px;
|
||||
}
|
||||
.compare-strip-sub { font-size: 11px; color: var(--text-muted); }
|
||||
.compare-strip-mid {
|
||||
@@ -3502,9 +3541,22 @@ button.region-pill-active:hover { opacity: 0.85; color: #fff; }
|
||||
cursor: pointer;
|
||||
}
|
||||
.compare-strip-mid-count {
|
||||
font-family: var(--mono);
|
||||
font-size: var(--fs-sm); font-weight: 600;
|
||||
color: var(--text); font-variant-numeric: tabular-nums;
|
||||
white-space: nowrap;
|
||||
}
|
||||
/* The 79% gets the largest type — it IS the answer to "how much
|
||||
* overlap?". The raw count and the label demote underneath. (Tufte:
|
||||
* the most important number gets the most ink. Issue #1646.) */
|
||||
.compare-strip-mid-pct {
|
||||
font-family: var(--mono);
|
||||
font-size: var(--fs-xl); font-weight: 700;
|
||||
color: var(--accent); font-variant-numeric: tabular-nums;
|
||||
line-height: 1;
|
||||
}
|
||||
.compare-strip-mid-pct-unit {
|
||||
font-size: 0.6em; margin-left: 1px;
|
||||
}
|
||||
.compare-strip-mid-label {
|
||||
font-size: 10px; text-transform: uppercase; letter-spacing: 0.6px;
|
||||
@@ -3517,7 +3569,16 @@ button.region-pill-active:hover { opacity: 0.85; color: #fff; }
|
||||
display: flex; height: 10px; border-radius: 5px; overflow: hidden;
|
||||
background: var(--surface-2);
|
||||
}
|
||||
.compare-bar-seg { transition: width 240ms ease; }
|
||||
.compare-bar-seg {
|
||||
transition: width 240ms ease;
|
||||
/* #1646: Tufte/honesty — a 1–2% segment must still be visible or
|
||||
* the chart is lying about presence. This is a *presence floor only*,
|
||||
* not a perception-equivalent floor: 2px is the smallest pip the
|
||||
* eye can reliably register on a 10px-tall bar without falsely
|
||||
* equating a 1% and a 5% slice (which a 6px floor would do).
|
||||
* Above ~2% the proportional encoding remains accurate. */
|
||||
min-width: 2px;
|
||||
}
|
||||
.compare-bar-a { background: var(--accent); }
|
||||
.compare-bar-both { background: var(--status-green); }
|
||||
.compare-bar-b { background: var(--status-amber); }
|
||||
@@ -3545,8 +3606,8 @@ button.region-pill-active:hover { opacity: 0.85; color: #fff; }
|
||||
.compare-asym-line {
|
||||
background: var(--surface-1);
|
||||
border: 1px solid var(--border);
|
||||
border-left: 3px solid var(--accent);
|
||||
border-radius: 0 var(--radius-sm) var(--radius-sm) 0;
|
||||
border-left: none;
|
||||
border-radius: var(--radius-sm);
|
||||
padding: var(--space-sm) var(--space-md);
|
||||
font-size: var(--fs-sm);
|
||||
color: var(--text-muted);
|
||||
@@ -3563,8 +3624,8 @@ button.region-pill-active:hover { opacity: 0.85; color: #fff; }
|
||||
.compare-type-summary {
|
||||
background: var(--surface-1);
|
||||
border: 1px solid var(--border);
|
||||
border-left: 3px solid var(--status-green);
|
||||
border-radius: 0 var(--radius-sm) var(--radius-sm) 0;
|
||||
border-left: none;
|
||||
border-radius: var(--radius-sm);
|
||||
padding: var(--space-sm) var(--space-md);
|
||||
margin-bottom: var(--space-md);
|
||||
font-size: var(--fs-sm); color: var(--text);
|
||||
@@ -3638,6 +3699,32 @@ button.region-pill-active:hover { opacity: 0.85; color: #fff; }
|
||||
border-top: 1px dashed var(--border); border-bottom: 1px dashed var(--border);
|
||||
padding: var(--space-sm) 0; flex-direction: row; gap: 6px; align-items: baseline; }
|
||||
.compare-asym { grid-template-columns: 1fr; }
|
||||
/* #1646 mobile reflow: the asym sentence wraps mid-phrase at narrow
|
||||
* widths ("3,262 of GY889 Repeater's 3,347 packets" splitting after
|
||||
* "Repeater's"). text-wrap:pretty rebalances trailing words to
|
||||
* avoid orphan-style breaks; overflow-wrap:anywhere lets very long
|
||||
* node names yield instead of pushing the column. The percentage
|
||||
* already lives on its own line (display:block), so the body is
|
||||
* free to breathe. */
|
||||
.compare-asym-line {
|
||||
text-wrap: pretty;
|
||||
overflow-wrap: anywhere;
|
||||
word-break: normal;
|
||||
}
|
||||
/* #1646 mobile tabs: "Only N6IJ Repeater (675)" overruns the row at
|
||||
* 375px; without min-width:0 a flex item can't shrink past its
|
||||
* intrinsic content width. Allow each tab to shrink and ellipsis
|
||||
* its label so all four fit. The count parenthetical stays
|
||||
* visible because it's at the tail of the string. */
|
||||
.compare-tabs .tab-btn {
|
||||
min-width: 0;
|
||||
flex: 0 1 auto;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
max-width: 50%;
|
||||
padding: 6px 10px;
|
||||
}
|
||||
}
|
||||
|
||||
/* ── Observers page — compare-selection column + button hints ─── */
|
||||
|
||||
+21
-12
@@ -877,17 +877,21 @@ async function run() {
|
||||
assert(options.length >= 2, `Need >=2 observers, got ${options.length}`);
|
||||
await page.selectOption('#compareObsA', options[0]);
|
||||
await page.selectOption('#compareObsB', options[1]);
|
||||
await page.waitForFunction(() => {
|
||||
const btn = document.getElementById('compareBtn');
|
||||
return btn && !btn.disabled;
|
||||
}, { timeout: 3000 });
|
||||
await page.click('#compareBtn');
|
||||
// #1646 — comparison auto-runs once both observers are chosen; the
|
||||
// legacy explicit Compare button has been removed entirely. The
|
||||
// picker collapses (.is-collapsed) once the run kicks off.
|
||||
await page.waitForFunction(() => {
|
||||
const c = document.getElementById('compareContent');
|
||||
return c && c.textContent.trim().length > 20;
|
||||
}, { timeout: 15000 });
|
||||
const hasResults = await page.$eval('#compareContent', el => el.textContent.trim().length > 0);
|
||||
assert(hasResults, 'Comparison should produce results');
|
||||
// And the picker should have collapsed (.is-collapsed class).
|
||||
const collapsed = await page.$eval('#compareControls', el => el.classList.contains('is-collapsed'));
|
||||
assert(collapsed, 'Picker should collapse once both observers chosen (.is-collapsed missing)');
|
||||
// The legacy Compare button must NOT exist in the DOM (#1646).
|
||||
const btnExists = await page.$('#compareBtn');
|
||||
assert(btnExists === null, 'Legacy #compareBtn must be removed — auto-run replaces it');
|
||||
});
|
||||
|
||||
// Test: Compare results show shared/unique breakdown (#129)
|
||||
@@ -900,14 +904,19 @@ async function run() {
|
||||
assert(stripMid, 'Should have "shared" strip middle (.compare-strip-mid)');
|
||||
const sides = await page.$$('.compare-strip-side');
|
||||
assert(sides.length >= 2, `Should have >=2 side strips (A-only + B-only), got ${sides.length}`);
|
||||
// Counts: 2 side counts + 1 mid count = 3 total outcome-group counts.
|
||||
const sideCounts = await page.$$eval('.compare-strip-count', els => els.map(e => e.textContent.trim()));
|
||||
const midCount = await page.$eval('.compare-strip-mid-count', el => el.textContent.trim());
|
||||
const counts = sideCounts.concat([midCount]);
|
||||
assert(counts.length >= 3, `Expected >=3 outcome-group counts, got ${counts.length}`);
|
||||
counts.forEach((c, i) => {
|
||||
assert(/^[\d,]+$/.test(c), `Count ${i} should be a number but got "${c}"`);
|
||||
// All three cells now lead with a percentage (#1646 Tufte): two
|
||||
// .compare-strip-side-pct on the sides + one .compare-strip-mid-pct
|
||||
// in the middle. The raw shared count still hangs underneath.
|
||||
const sidePcts = await page.$$eval('.compare-strip-side-pct', els => els.map(e => e.textContent.trim()));
|
||||
const midPct = await page.$eval('.compare-strip-mid-pct', el => el.textContent.trim());
|
||||
assert(sidePcts.length >= 2, `Expected >=2 side pct cells, got ${sidePcts.length}`);
|
||||
assert(/\d+%/.test(midPct), `Mid pct should look like a percentage, got "${midPct}"`);
|
||||
sidePcts.concat([midPct]).forEach((c, i) => {
|
||||
assert(/\d+\s*%/.test(c), `Pct ${i} should contain a % value but got "${c}"`);
|
||||
});
|
||||
// The raw shared count exists and is purely numeric (no embedded label).
|
||||
const midCount = await page.$eval('.compare-strip-mid-count', el => el.textContent.trim());
|
||||
assert(/^[\d,]+$/.test(midCount), `Shared count should be a bare number, got "${midCount}"`);
|
||||
// Verify tab buttons exist for both/onlyA/onlyB
|
||||
const tabs = await page.$$eval('[data-cview]', els => els.map(e => e.getAttribute('data-cview')));
|
||||
assert(tabs.includes('both'), 'Should have "both" tab');
|
||||
|
||||
@@ -0,0 +1,307 @@
|
||||
/**
|
||||
* Issue #1646 — Polish follow-ups for the #1644/#1645 observer-comparison
|
||||
* redesign. Behavioral CSS+markup assertions only (Node-only, no Playwright).
|
||||
*
|
||||
* The aesthetic items (font weights, vertical centering, removing decorative
|
||||
* bars) are verified visually via screenshots in the PR — this file gates
|
||||
* the few items that ARE behaviorally testable so a regression future-proofs
|
||||
* the polish.
|
||||
*
|
||||
* 1) `input[type=checkbox]` has a GLOBAL `accent-color: var(--accent)`
|
||||
* rule (not only the per-page `.col-compare-select` rule that misses
|
||||
* the rest of the surface). Both light + dark must theme. AND no
|
||||
* later override drops accent-color back to a non-token value.
|
||||
* 2) BOTH dark-theme blocks (auto via prefers-color-scheme + manual via
|
||||
* [data-theme="dark"]) declare `color-scheme: dark` so UA-native
|
||||
* widgets render dark.
|
||||
* 3) `.compare-vs` font-size is smaller than `.compare-select` font-size.
|
||||
* 4) `.compare-strip-mid-pct` uses var(--fs-xl) and is the largest text
|
||||
* in the mid cell.
|
||||
* 5) `.compare-strip-mid-count` is strictly smaller than
|
||||
* `.compare-strip-mid-pct` (token-rank comparison, not "not --fs-xl").
|
||||
* 6) `.compare-asym-line` and `.compare-type-summary` explicitly declare
|
||||
* `border-left: none` AND no `border:` shorthand resolves to a left
|
||||
* edge (Kent Beck: "absence of declaration" is too permissive).
|
||||
* 7) compare.js drives the collapse via the actual call
|
||||
* `wrap.classList.toggle('is-collapsed', ready)` — grepping comments
|
||||
* for the literal string is a tautology.
|
||||
* 8) The legacy Compare button has been removed from the DOM in compare.js.
|
||||
*/
|
||||
'use strict';
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const CSS = fs.readFileSync(path.join(__dirname, 'public/style.css'), 'utf8');
|
||||
const COMPARE_JS = fs.readFileSync(path.join(__dirname, 'public/compare.js'), 'utf8');
|
||||
|
||||
// Token-rank used by font-size comparisons. Comments-only mirror of the
|
||||
// scale; if --fs-xl ever moves, only the relative order matters and that
|
||||
// is what we assert.
|
||||
const TOKEN_RANK = {
|
||||
'var(--fs-xs)': 1, 'var(--fs-sm)': 2, 'var(--fs-md)': 3,
|
||||
'var(--fs-lg)': 4, 'var(--fs-xl)': 5,
|
||||
};
|
||||
function fsRank(decl) {
|
||||
if (!decl) return null;
|
||||
const v = decl.trim();
|
||||
if (TOKEN_RANK[v] != null) return TOKEN_RANK[v];
|
||||
const num = v.match(/^(\d+(?:\.\d+)?)px$/);
|
||||
if (num) return parseFloat(num[1]) / 4; // crude px-to-rank fallback
|
||||
return null;
|
||||
}
|
||||
|
||||
let passed = 0, failed = 0;
|
||||
function test(name, fn) {
|
||||
try { fn(); passed++; console.log(' \u2705 ' + name); }
|
||||
catch (e) { failed++; console.error(' \u274c ' + name + ': ' + e.message); }
|
||||
}
|
||||
function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); }
|
||||
|
||||
function ruleBlock(css, selectorRegex) {
|
||||
// returns the {...} block body for the first matching selector list
|
||||
const re = new RegExp('(?:^|[\\s,}])(' + selectorRegex.source + ')[^{}]*\\{([^}]*)\\}', 'm');
|
||||
const m = css.match(re);
|
||||
return m ? m[2] : null;
|
||||
}
|
||||
function fsOf(block) {
|
||||
if (!block) return null;
|
||||
const m = block.match(/font-size\s*:\s*([^;]+);/);
|
||||
return m ? m[1].trim() : null;
|
||||
}
|
||||
|
||||
console.log('\n#1646 compare-polish — behavioral assertions\n');
|
||||
|
||||
// ── 1) global checkbox accent-color ───────────────────────────────────
|
||||
test('global input[type=checkbox] accent-color rule uses var(--accent) and no later rule overrides it to a non-token value', () => {
|
||||
const re = /(?:^|})\s*input\[type=["']?checkbox["']?\][^{]*\{[^}]*accent-color\s*:\s*var\(--accent\)/m;
|
||||
assert(re.test(CSS),
|
||||
'missing top-level `input[type=checkbox] { accent-color: var(--accent); }`');
|
||||
// Find every accent-color decl on a checkbox selector (single-line scan)
|
||||
// and assert each uses var(--accent ...) — no white/colored hardcodes.
|
||||
const ruleRe = /input\[type=["']?checkbox["']?\][^{]*\{([^}]*)\}/g;
|
||||
let m;
|
||||
while ((m = ruleRe.exec(CSS))) {
|
||||
const decl = m[1].match(/accent-color\s*:\s*([^;]+);/);
|
||||
if (!decl) continue;
|
||||
const val = decl[1].trim();
|
||||
assert(/^var\(--/.test(val),
|
||||
`checkbox accent-color override must use a CSS var, got "${val}"`);
|
||||
}
|
||||
});
|
||||
|
||||
// ── 2) color-scheme on BOTH dark theme blocks ─────────────────────────
|
||||
test('both dark-theme rules (prefers-color-scheme + [data-theme="dark"]) declare color-scheme: dark', () => {
|
||||
const auto = /@media[^{]*prefers-color-scheme:\s*dark[^{]*\{\s*[^{]*\{[^}]*color-scheme\s*:\s*dark/m;
|
||||
const manual = /\[data-theme=["']dark["']\][^{]*\{[^}]*color-scheme\s*:\s*dark/m;
|
||||
assert(auto.test(CSS), 'missing color-scheme: dark inside @media(prefers-color-scheme: dark)');
|
||||
assert(manual.test(CSS), 'missing color-scheme: dark inside [data-theme="dark"] block');
|
||||
});
|
||||
|
||||
// ── 3) .compare-vs smaller than .compare-select ───────────────────────
|
||||
test('.compare-vs font-size < .compare-select font-size', () => {
|
||||
const vsBlock = ruleBlock(CSS, /\.compare-vs/);
|
||||
const selBlock = ruleBlock(CSS, /\.compare-select(?![a-zA-Z-])/);
|
||||
assert(vsBlock, '.compare-vs block missing');
|
||||
assert(selBlock, '.compare-select block missing');
|
||||
function px(block) {
|
||||
const v = fsOf(block);
|
||||
if (!v) return null;
|
||||
const tokenMap = {
|
||||
'var(--fs-xs)': 11, 'var(--fs-sm)': 12, 'var(--fs-md)': 14,
|
||||
'var(--fs-lg)': 15, 'var(--fs-xl)': 18,
|
||||
};
|
||||
if (tokenMap[v] != null) return tokenMap[v];
|
||||
const num = v.match(/^(\d+(?:\.\d+)?)px$/);
|
||||
return num ? parseFloat(num[1]) : null;
|
||||
}
|
||||
const vsSize = px(vsBlock);
|
||||
const selSize = px(selBlock);
|
||||
assert(vsSize != null && selSize != null, 'could not parse font-size');
|
||||
assert(vsSize < selSize, `.compare-vs (${vsSize}) must be smaller than .compare-select (${selSize})`);
|
||||
});
|
||||
|
||||
// ── 4) middle column hierarchy: pct > count > label ───────────────────
|
||||
test('.compare-strip-mid-pct exists and is var(--fs-xl)', () => {
|
||||
const pctBlock = ruleBlock(CSS, /\.compare-strip-mid-pct/);
|
||||
assert(pctBlock, '.compare-strip-mid-pct rule missing (needed for inverted hierarchy)');
|
||||
assert(/font-size\s*:\s*var\(--fs-xl\)/.test(pctBlock),
|
||||
'.compare-strip-mid-pct must be var(--fs-xl) (the largest)');
|
||||
});
|
||||
|
||||
test('.compare-strip-mid-count is strictly smaller than .compare-strip-mid-pct (token-rank)', () => {
|
||||
const countBlock = ruleBlock(CSS, /\.compare-strip-mid-count(?!-)/);
|
||||
const pctBlock = ruleBlock(CSS, /\.compare-strip-mid-pct(?!-)/);
|
||||
assert(countBlock, '.compare-strip-mid-count rule missing');
|
||||
assert(pctBlock, '.compare-strip-mid-pct rule missing');
|
||||
const cRank = fsRank(fsOf(countBlock));
|
||||
const pRank = fsRank(fsOf(pctBlock));
|
||||
assert(cRank != null && pRank != null, `could not parse font-sizes (count=${fsOf(countBlock)}, pct=${fsOf(pctBlock)})`);
|
||||
assert(cRank < pRank,
|
||||
`.compare-strip-mid-count rank (${cRank}) must be < .compare-strip-mid-pct rank (${pRank})`);
|
||||
});
|
||||
|
||||
// ── 5) Compare CTA dead — button removed from DOM ─────────────────────
|
||||
test('legacy #compareBtn is no longer emitted by compare.js (auto-run replaces it)', () => {
|
||||
// Behavioral: the markup string for an explicit Compare button must
|
||||
// be gone from the rendered HTML. A render path that conditionally
|
||||
// hides a still-present button would let an enabled-but-hidden state
|
||||
// exist; deleting the markup makes that impossible.
|
||||
const re = /id=["']compareBtn["']/;
|
||||
assert(!re.test(COMPARE_JS),
|
||||
'compare.js still renders #compareBtn — must be removed (auto-run on selection change)');
|
||||
// And no .compare-btn-ghost / .compare-btn class lingers.
|
||||
assert(!/compare-btn-ghost/.test(COMPARE_JS),
|
||||
'dead .compare-btn-ghost class still emitted');
|
||||
});
|
||||
|
||||
// ── 6) decorative asym-line border-left explicitly removed ────────────
|
||||
test('.compare-asym-line declares border-left: none AFTER any border-shorthand (cascade-safe)', () => {
|
||||
const block = ruleBlock(CSS, /\.compare-asym-line(?!-)/);
|
||||
assert(block, '.compare-asym-line rule missing');
|
||||
// Tightened (Kent Beck must-fix #3): require explicit border-left: none,
|
||||
// not just absence of a border-left: declaration. If a border: shorthand
|
||||
// exists, border-left: none must come AFTER it so the cascade kills the
|
||||
// left edge.
|
||||
const blIdx = block.search(/border-left\s*:\s*none\b/);
|
||||
assert(blIdx >= 0,
|
||||
'.compare-asym-line must explicitly declare `border-left: none` (a future `border:` shorthand could re-add the bar)');
|
||||
const shortIdx = block.search(/(?:^|[;{\s])border\s*:\s*(?!none\b)[^;]*\b\d/);
|
||||
if (shortIdx >= 0) {
|
||||
assert(blIdx > shortIdx,
|
||||
'.compare-asym-line `border-left: none` must come AFTER `border:` shorthand or shorthand wins');
|
||||
}
|
||||
});
|
||||
|
||||
// ── 7) decorative type-summary border-left explicitly removed ─────────
|
||||
test('.compare-type-summary declares border-left: none AFTER any border-shorthand (cascade-safe)', () => {
|
||||
const block = ruleBlock(CSS, /\.compare-type-summary(?!-)/);
|
||||
assert(block, '.compare-type-summary rule missing');
|
||||
const blIdx = block.search(/border-left\s*:\s*none\b/);
|
||||
assert(blIdx >= 0,
|
||||
'.compare-type-summary must explicitly declare `border-left: none`');
|
||||
const shortIdx = block.search(/(?:^|[;{\s])border\s*:\s*(?!none\b)[^;]*\b\d/);
|
||||
if (shortIdx >= 0) {
|
||||
assert(blIdx > shortIdx,
|
||||
'.compare-type-summary `border-left: none` must come AFTER `border:` shorthand');
|
||||
}
|
||||
});
|
||||
|
||||
// ── 8) controls collapse — assert the actual DOM call, not comment text ──
|
||||
test('compare.js makes the actual classList.toggle("is-collapsed", ready) call AND CSS keys on it', () => {
|
||||
// Tightened (Kent Beck must-fix #1, Adversarial #6, #8): assert the
|
||||
// behavioral call, not just a string match against comments. The PR
|
||||
// also dropped the redundant data-collapsed setAttribute path, so any
|
||||
// re-introduction is a regression.
|
||||
const callRe = /classList\.toggle\(\s*['"]is-collapsed['"]\s*,/;
|
||||
assert(callRe.test(COMPARE_JS),
|
||||
'compare.js must call classList.toggle("is-collapsed", <bool>) on #compareControls');
|
||||
assert(!/setAttribute\(\s*['"]data-collapsed['"]/.test(COMPARE_JS),
|
||||
'compare.js must NOT also setAttribute("data-collapsed", ...) — pick one source of truth (the class)');
|
||||
// CSS rule keying on the class must exist (without a stale [data-collapsed] selector).
|
||||
assert(/\.compare-controls\.is-collapsed/.test(CSS),
|
||||
'style.css must define a rule on .compare-controls.is-collapsed');
|
||||
});
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════
|
||||
// #1646 — Mobile follow-ups (Tufte review).
|
||||
// At ≤768px the page sits above a fixed bottom-nav (56px + safe-area)
|
||||
// reserved via --bottom-nav-reserve. The headline diff bar has segments
|
||||
// that go invisible at narrow widths when their share is ~2%, and the
|
||||
// asymmetric-reach sentences wrap mid-phrase. These are behavioral
|
||||
// guards so the polish doesn't quietly regress.
|
||||
// ════════════════════════════════════════════════════════════════════
|
||||
|
||||
// helper: pull the body of the FIRST @media (max-width: <=768px) block
|
||||
function mobileBlock(css) {
|
||||
const re = /@media[^{]*\(max-width:\s*(640|768)px\)\s*\{([\s\S]*?)\n\}\s*\n/g;
|
||||
let combined = '';
|
||||
let m;
|
||||
while ((m = re.exec(css))) combined += m[2] + '\n';
|
||||
return combined;
|
||||
}
|
||||
const MOBILE_CSS = mobileBlock(CSS);
|
||||
|
||||
// ── 9) compare-page reserves room for the bottom-nav at mobile ────────
|
||||
test('mobile .compare-page reserves padding-bottom for the bottom-nav', () => {
|
||||
// Either the rule lives inside a mobile @media block, OR an
|
||||
// unconditional rule references var(--bottom-nav-reserve).
|
||||
const inMobile = /\.compare-page[^{]*\{[^}]*padding-bottom[^;]*--bottom-nav-reserve/m.test(MOBILE_CSS);
|
||||
const unconditional = /\.compare-page[^{]*\{[^}]*padding-bottom[^;]*--bottom-nav-reserve/m.test(CSS);
|
||||
assert(inMobile || unconditional,
|
||||
'.compare-page must add padding-bottom tied to var(--bottom-nav-reserve) so the last row is not eaten by the bottom-nav');
|
||||
});
|
||||
|
||||
// ── 10) diff-bar segments stay visible at narrow widths ───────────────
|
||||
test('.compare-bar-seg has a min-width so non-zero segments stay visible', () => {
|
||||
// We accept either a global min-width on .compare-bar-seg, or a
|
||||
// mobile-scoped one. Visibility is what matters.
|
||||
const reAny = /\.compare-bar-seg[^{}]*\{[^}]*min-width\s*:/m;
|
||||
assert(reAny.test(CSS),
|
||||
'.compare-bar-seg must declare min-width so a 2% segment is still readable on mobile');
|
||||
// #1646 round-2 review: the floor must be a *presence* floor only,
|
||||
// not a magnitude-flattening floor. 6px collapses 1% and 5% slices
|
||||
// to identical width, lying about magnitude. Cap at 2px — visible
|
||||
// pip without falsely equating sizes.
|
||||
const segBlockRe = /\.compare-bar-seg[^{}]*\{[^}]*min-width\s*:\s*(\d+)px/m;
|
||||
const m = CSS.match(segBlockRe);
|
||||
assert(m, '.compare-bar-seg min-width must be expressed in px so we can bound it');
|
||||
const px = parseInt(m[1], 10);
|
||||
assert(px <= 2,
|
||||
`.compare-bar-seg min-width must be <= 2px (presence floor only, not magnitude-equivalent); got ${px}px`);
|
||||
});
|
||||
|
||||
// ── 11) asym sentence reflows cleanly on mobile ───────────────────────
|
||||
test('mobile .compare-asym-line uses text-wrap balance/pretty (or overflow-wrap) to avoid mid-phrase breaks', () => {
|
||||
// Look inside any mobile @media block for the rule. We accept
|
||||
// text-wrap: balance | pretty, or word-break/overflow-wrap as
|
||||
// alternative reflow strategies.
|
||||
const ok = /\.compare-asym-line[^{}]*\{[^}]*(text-wrap\s*:\s*(balance|pretty)|overflow-wrap\s*:\s*anywhere|word-break\s*:\s*break-word)/m.test(MOBILE_CSS);
|
||||
assert(ok,
|
||||
'mobile .compare-asym-line needs a wrap rule (text-wrap: balance/pretty or overflow-wrap) so the sentence does not break mid-phrase');
|
||||
});
|
||||
|
||||
// ── 12) tabs row stays usable on narrow widths ────────────────────────
|
||||
test('mobile .compare-tabs .tab-btn shrinks/wraps so all four tabs fit', () => {
|
||||
// We need EITHER tabs to allow wrap (flex-wrap: wrap on .compare-tabs
|
||||
// — already present at desktop) AND a per-button rule that lets the
|
||||
// long "Only <name> (NNN)" labels truncate or shrink.
|
||||
// Accept: (a) a mobile rule on .tab-btn with min-width:0 + text-overflow,
|
||||
// (b) a flex: 1 1 0 / flex-shrink, or
|
||||
// (c) overflow-wrap on the tab.
|
||||
const ok = /\.compare-tabs[^{}]*\.tab-btn[^{}]*\{[^}]*(min-width\s*:\s*0|flex\s*:\s*1|flex-shrink|overflow-wrap)/m.test(MOBILE_CSS) ||
|
||||
/\.compare-tabs\s+\.tab-btn[^{}]*\{[^}]*(min-width\s*:\s*0|flex\s*:\s*1|flex-shrink|overflow-wrap)/m.test(MOBILE_CSS);
|
||||
assert(ok,
|
||||
'mobile .compare-tabs .tab-btn must declare min-width:0 / flex / overflow-wrap so all four tab labels fit at 375px');
|
||||
});
|
||||
|
||||
// ── 13) compare.js auto-run guards are consistent (#1646 round-2) ─────
|
||||
// Round-2 review found two run-comparison call sites with *different*
|
||||
// guards: loadObservers used `selA && selB` (no inequality check) while
|
||||
// onChange used `selA && selB && selA !== selB`. URL-prepopulated
|
||||
// ?a=X&b=X (same observer in both slots) would launch a comparison via
|
||||
// the loadObservers path and not the change path. Both call sites must
|
||||
// gate on the same predicate.
|
||||
test('compare.js — both runComparison() guards include selA !== selB', () => {
|
||||
// Find every line that calls runComparison() and check the guard
|
||||
// immediately preceding it on the same line is identical.
|
||||
// Only inspect guards that reference the selA/selB / ready predicate
|
||||
// (not the route-filter re-run path which conditions on comparisonResult).
|
||||
const guardLines = COMPARE_JS
|
||||
.split('\n')
|
||||
.filter(l => /\brunComparison\s*\(/.test(l) && /\bif\s*\(/.test(l))
|
||||
.filter(l => /selA|selB|[Rr]eady\b|canCompare\b/.test(l));
|
||||
assert(guardLines.length >= 2,
|
||||
`expected at least 2 selA/selB-guarded runComparison() call sites, found ${guardLines.length}`);
|
||||
// Every such line must check selA !== selB (or the equivalent
|
||||
// ready predicate factored into a helper). The forbidden pattern is
|
||||
// a guard that ONLY checks `selA && selB` without the inequality.
|
||||
guardLines.forEach((line, i) => {
|
||||
const trimmed = line.trim();
|
||||
const hasInequality = /selA\s*!==\s*selB|selB\s*!==\s*selA|[Rr]eady\b|canCompare\b/.test(trimmed);
|
||||
assert(hasInequality,
|
||||
`runComparison() guard #${i + 1} missing selA !== selB / ready predicate: ${trimmed}`);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
process.exit(failed === 0 ? 0 : 1);
|
||||
Reference in New Issue
Block a user