From 531bc8acb398018a9cbfba9b34194f9fc7d234e2 Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Wed, 10 Jun 2026 11:43:24 -0700 Subject: [PATCH] =?UTF-8?q?feat(#1640):=20promote=20observer=20comparison?= =?UTF-8?q?=20to=20first-class=20=E2=80=94=203=20new=20entry=20points=20+?= =?UTF-8?q?=20multi-select=20(#1642)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary The observer-comparison page (`#/compare`) is a powerful side-by-side overlap tool but was reachable from exactly one place β€” an icon-only πŸ” button in the observers page header. Most operators never found it. This PR promotes it to an IA citizen with **three new entry points** plus breadcrumbs back from the compare page to each observer's detail page. Red commit: `f937d29658e25973786f88a9ddeaaa33768f269e` (test asserts all three new affordances are present + navigate correctly; would have caught the original undiscoverability). Green commit: `5ceb34b66d780a971d3a43de06a0744445bdbecf`. ## Design rationale Three orthogonal user paths reach the same goal: - **Operator who lands on `/observers`** sees a labeled button β€” no more icon-guessing β€” and a row-selection workflow for direct manipulation ("pick two, compare"). - **Operator who lands on a specific observer's page** sees an in-context "Compare with…" picker β€” the comparison is parameterised with the current observer, removing the cognitive jump back to the list. - **Operator who already has two observer IDs** can still hit `#/compare?a=…&b=…` directly β€” legacy deep-links regression-guarded by the E2E. Plus: every compare-page view now shows `Observers β€Ί ⇆ ` breadcrumbs that link back to each observer's detail page, so users can navigate sideways instead of bouncing through the list. ## Entry points added | # | Surface | Affordance | File:line | |---|---|---|---| | A | `/observers` header | ` + `; }).join('')} `; @@ -365,6 +400,34 @@ window.ObserversSummary = (function () { registerPage('observers', { init, destroy }); + // #1640 β€” multi-select compare wiring. + function collectSelectedIds() { + var boxes = document.querySelectorAll('#obsTable tbody input[data-compare-select]:checked'); + var ids = []; + for (var i = 0; i < boxes.length; i++) ids.push(boxes[i].value); + return ids; + } + function updateCompareSelectedState() { + var btn = document.querySelector('[data-action="compare-selected"]'); + if (!btn) return; + var ids = collectSelectedIds(); + var countEl = btn.querySelector('[data-role="compare-count"]'); + if (countEl) countEl.textContent = String(ids.length); + var enabled = ids.length === 2; + btn.disabled = !enabled; + btn.setAttribute('aria-disabled', enabled ? 'false' : 'true'); + btn.title = enabled + ? 'Compare the two selected observers' + : 'Select exactly two observers to enable (currently ' + ids.length + ')'; + } + // Wire change events globally so the state updates even when checkbox + // toggles by keyboard (space) rather than mouse click. + document.addEventListener('change', function (e) { + if (e.target && e.target.matches && e.target.matches('input[data-compare-select]')) { + updateCompareSelectedState(); + } + }); + // #1056 AC#4: row-detail slide-over (narrow viewports). Renders a compact // summary from the in-memory observer + a link to the full page. function openObserverSlideOver(hashHref) { diff --git a/test-issue-1640-compare-discovery-e2e.js b/test-issue-1640-compare-discovery-e2e.js new file mode 100644 index 00000000..c67b301a --- /dev/null +++ b/test-issue-1640-compare-discovery-e2e.js @@ -0,0 +1,171 @@ +/** + * E2E test (#1640): Observer comparison must be a first-class IA citizen. + * + * Asserts THREE new entry points to `#/compare`, beyond the pre-existing + * πŸ” button on the observers page header: + * + * (A) Observers page header β€” a labeled button reading "Compare observers" + * (text + icon, NOT a bare emoji). + * (B) Observer-detail page β€” a "Compare with…" affordance that opens + * #/compare?a=&b= pre-populated. + * (D) Multi-select on observers table β€” checkbox-per-row, enabling a + * "Compare selected" button once exactly two observers are checked. + * + * Also asserts: + * - The compare page renders breadcrumb links back to BOTH observer + * detail pages. + * - The legacy deep-link `#/compare?a=...&b=...` continues to work. + * + * Usage: BASE_URL=http://localhost:13581 node test-issue-1640-compare-discovery-e2e.js + */ +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:3000'; + +let passed = 0, failed = 0; +async function step(name, fn) { + try { await 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'); } + +async function pickTwoObserverIds(page) { + await page.goto(BASE + '/#/observers', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#obsTable tbody tr', { timeout: 15000 }); + const ids = await page.$$eval('#obsTable tbody tr[data-value]', rows => + rows.slice(0, 2).map(r => decodeURIComponent( + (r.getAttribute('data-value') || '').replace('#/observers/', '') + )) + ); + assert(ids.length === 2, 'need at least 2 observers in fixture, got ' + ids.length); + return ids; +} + +async function run() { + const browser = await chromium.launch({ + headless: true, + executablePath: process.env.CHROMIUM_PATH || undefined, + args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'] + }); + const ctx = await browser.newContext({ viewport: { width: 1400, height: 900 } }); + const page = await ctx.newPage(); + page.setDefaultTimeout(15000); + page.on('pageerror', e => console.error(' pageerror:', e.message)); + + console.log('\nRunning #1640 compare-discovery E2E tests against ' + BASE + '\n'); + + const [idA, idB] = await pickTwoObserverIds(page); + + // ── Entry point A: labeled "Compare observers" on observers page ── + await step('(A) Observers page header has labeled "Compare observers" button', async () => { + await page.goto(BASE + '/#/observers', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#obsTable tbody tr', { timeout: 15000 }); + const btn = await page.$('[data-action="compare-observers"]'); + assert(btn, 'expected element with data-action="compare-observers" in observers page header'); + const text = (await btn.textContent() || '').trim(); + assert(/compare/i.test(text), + 'compare button must have visible text mentioning "Compare", got "' + text + '"'); + }); + + await step('(A) Clicking "Compare observers" navigates to #/compare', async () => { + await page.goto(BASE + '/#/observers', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('[data-action="compare-observers"]', { timeout: 10000 }); + await page.click('[data-action="compare-observers"]'); + await page.waitForFunction(() => location.hash.startsWith('#/compare'), null, { timeout: 5000 }); + assert(/^#\/compare/.test(await page.evaluate(() => location.hash)), + 'expected hash to become #/compare'); + }); + + // ── Entry point B: observer-detail "Compare with…" picker ── + await step('(B) Observer detail page exposes a "Compare with…" affordance', async () => { + await page.goto(BASE + '/#/observers/' + idA, { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#obsTitle', { timeout: 15000 }); + // Give detail page a moment to finish rendering. + await page.waitForSelector('[data-action="compare-with-picker"]', { timeout: 10000 }); + const picker = await page.$('[data-action="compare-with-picker"]'); + assert(picker, 'expected [data-action="compare-with-picker"] (select) on observer-detail'); + const options = await picker.$$('option'); + assert(options.length >= 2, 'compare-with picker should be populated with other observers'); + }); + + await step('(B) Picking another observer + Compare navigates to #/compare?a=&b=', async () => { + await page.goto(BASE + '/#/observers/' + idA, { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('[data-action="compare-with-picker"]', { timeout: 10000 }); + await page.selectOption('[data-action="compare-with-picker"]', idB); + await page.click('[data-action="compare-with-go"]'); + await page.waitForFunction((idA) => + location.hash.indexOf('#/compare') === 0 && + location.hash.indexOf('a=' + idA) >= 0, idA, { timeout: 5000 }); + const h = await page.evaluate(() => location.hash); + assert(h.indexOf('b=' + idB) >= 0, 'expected deep-link to carry b=, got: ' + h); + }); + + // ── Entry point D: multi-select on observers table ── + await step('(D) Observers table renders one checkbox per row + "Compare selected" button', async () => { + await page.goto(BASE + '/#/observers', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#obsTable tbody tr', { timeout: 15000 }); + const boxes = await page.$$('#obsTable tbody input[type="checkbox"][data-compare-select]'); + assert(boxes.length >= 2, + 'expected per-row checkboxes ([data-compare-select]); got ' + boxes.length); + const btn = await page.$('[data-action="compare-selected"]'); + assert(btn, 'expected [data-action="compare-selected"] button'); + const disabled = await btn.evaluate(el => el.disabled || el.getAttribute('aria-disabled') === 'true'); + assert(disabled, '"Compare selected" must be disabled when 0 rows are selected'); + }); + + await step('(D) Selecting exactly two rows enables "Compare selected" and navigates correctly', async () => { + await page.goto(BASE + '/#/observers', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#obsTable tbody tr', { timeout: 15000 }); + const boxes = await page.$$('#obsTable tbody input[type="checkbox"][data-compare-select]'); + await boxes[0].check(); + await boxes[1].check(); + const btn = await page.$('[data-action="compare-selected"]'); + const stillDisabled = await btn.evaluate(el => el.disabled || el.getAttribute('aria-disabled') === 'true'); + assert(!stillDisabled, '"Compare selected" must be enabled when exactly 2 rows are checked'); + await btn.click(); + await page.waitForFunction(() => location.hash.indexOf('#/compare?') === 0, null, { timeout: 5000 }); + const h = await page.evaluate(() => location.hash); + assert(/a=[^&]+&b=[^&]+/.test(h), + 'expected hash to carry both ?a=&b= deep-link params, got: ' + h); + }); + + await step('(D) Selecting a third row disables "Compare selected" again', async () => { + await page.goto(BASE + '/#/observers', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#obsTable tbody tr', { timeout: 15000 }); + const boxes = await page.$$('#obsTable tbody input[type="checkbox"][data-compare-select]'); + if (boxes.length < 3) return; // fixture might only have 2; skip silently + await boxes[0].check(); + await boxes[1].check(); + await boxes[2].check(); + const btn = await page.$('[data-action="compare-selected"]'); + const disabled = await btn.evaluate(el => el.disabled || el.getAttribute('aria-disabled') === 'true'); + assert(disabled, '"Compare selected" must re-disable when count !== 2'); + }); + + // ── Compare page breadcrumbs to both observer detail pages ── + await step('Compare page renders breadcrumb links back to both observer detail pages', async () => { + await page.goto(BASE + '/#/compare?a=' + idA + '&b=' + idB, { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('.compare-page', { timeout: 15000 }); + await page.waitForSelector('[data-role="compare-breadcrumbs"]', { timeout: 10000 }); + const linkA = await page.$('[data-role="compare-breadcrumbs"] a[href="#/observers/' + idA + '"]'); + const linkB = await page.$('[data-role="compare-breadcrumbs"] a[href="#/observers/' + idB + '"]'); + assert(linkA, 'expected breadcrumb anchor β†’ #/observers/'); + assert(linkB, 'expected breadcrumb anchor β†’ #/observers/'); + }); + + // ── Legacy deep-link regression guard ── + await step('Legacy deep-link #/compare?a=...&b=... still pre-populates both selects', async () => { + await page.goto(BASE + '/#/compare?a=' + idA + '&b=' + idB, { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#compareObsA', { timeout: 15000 }); + const valA = await page.$eval('#compareObsA', el => el.value); + const valB = await page.$eval('#compareObsB', el => el.value); + assert(valA === idA, 'compareObsA should be pre-selected to a=, got ' + valA); + assert(valB === idB, 'compareObsB should be pre-selected to b=, got ' + valB); + }); + + await browser.close(); + console.log('\n' + passed + ' passed, ' + failed + ' failed'); + if (failed) process.exit(1); +} + +run().catch(e => { console.error(e); process.exit(1); });