diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 0d9aefb90..e43d15620 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -443,6 +443,7 @@ jobs: BASE_URL=http://localhost:13581 node test-issue-1122-packets-filter-ux-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1122-details-row-clamp-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1128-packets-layout-e2e.js 2>&1 | tee -a e2e-output.txt + BASE_URL=http://localhost:13581 node test-issue-189-group-caret-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1128-multi-viewport-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1136-live-region-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-live-multibyte-only-e2e.js 2>&1 | tee -a e2e-output.txt diff --git a/public/packets.js b/public/packets.js index 8cfa2824f..a06d8028d 100644 --- a/public/packets.js +++ b/public/packets.js @@ -2436,8 +2436,13 @@ const _grpChanStyle = window.ChannelColors ? window.ChannelColors.getRowStyle(_grpDecoded.type || groupTypeName, _grpDecoded.channel) : ''; const _grpHashStripe = _hashStripeStyle(p.hash); const _grpStyle = _grpHashStripe + _grpChanStyle; - let html = ` - ${isSingle ? '' : (isExpanded ? '' : '')} + // #189: disclosure caret, right when collapsed and down when expanded, as in + // channels.js, network-digest.js, analytics.js and route-view.js. The row that + // toggles reports its state in aria-expanded; a single-observation row cannot + // expand, so it has neither. + const _grpCaret = isSingle ? '' : ''; + let html = ` + ${_grpCaret} ${groupRegion ? `${groupRegion}` : '—'} ${renderTimestampCell(p.latest)} ${truncate(p.hash || '—', 8)} @@ -4298,6 +4303,7 @@ renderTableRows, _setPackets: function(p) { packets = p; }, _setFilter: function(k, v) { filters[k] = v; }, + _setExpanded: function(hash, on) { if (on) expandedHashes.add(hash); else expandedHashes.delete(hash); }, }; } diff --git a/public/style.css b/public/style.css index 75b358575..6c2007117 100644 --- a/public/style.css +++ b/public/style.css @@ -2614,8 +2614,6 @@ button.ch-item:hover .ch-icon-btn { opacity: 1; } /* === Grouped Packet Rows === */ .group-header { cursor: pointer; font-weight: 600; } -.group-header td:first-child::before { content: '▶ '; font-size: 10px; color: var(--link-color); transition: transform 0.15s; display: inline-block; } -.group-header.expanded td:first-child::before { content: '▼ '; } .group-header:hover { background: var(--row-hover); } .group-child { font-size: 12px; } .group-child td { padding-left: 20px; } diff --git a/test-issue-189-group-caret-e2e.js b/test-issue-189-group-caret-e2e.js new file mode 100644 index 000000000..681bdbbc6 --- /dev/null +++ b/test-issue-189-group-caret-e2e.js @@ -0,0 +1,86 @@ +/** + * E2E (#189): a packet group in the grouped packets view shows a right caret + * while collapsed and a down caret while expanded, once, and its toggle row + * reports the state in aria-expanded. + * + * Against the e2e fixture's seeded 3-observation group (see the "Seed + * grouped-packet row for #1486" step in deploy.yml): + * - collapsed: #ph-caret-right, aria-expanded="false"; + * - expanded (after a click): #ph-caret-down, aria-expanded="true"; + * - collapsed again (second click): back to #ph-caret-right and "false"; + * - in each state the expand cell holds exactly one caret and no CSS triangle + * (style.css used to add a second ▶/▼ through ::before). + * + * Usage: BASE_URL=http://localhost:13581 node test-issue-189-group-caret-e2e.js + */ +'use strict'; +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; +const SEED_HASH = 'fae0c9e6d357a814'; +const ROW = `#pktBody tr[data-hash="${SEED_HASH}"][data-action="toggle-select"]`; + +let passed = 0, failed = 0; +async function step(name, fn) { + try { await fn(); passed++; console.log(' ✓ ' + name); } + catch (e) { failed++; console.error(' ✗ ' + name + ': ' + e.message); } +} +function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } + +// What the user sees in the group row's first cell, and what the row reports. +async function rowState(page) { + return page.evaluate((sel) => { + const tr = document.querySelector(sel); + const td = tr.querySelector('td.col-expand'); + return { + expandedClass: tr.classList.contains('expanded'), + aria: tr.getAttribute('aria-expanded'), + carets: [...td.querySelectorAll('use')].map((u) => (u.getAttribute('href') || '').split('#')[1]), + before: getComputedStyle(tr.querySelector('td:first-child'), '::before').content, + }; + }, ROW); +} + +function assertState(s, caret, aria) { + assert(s.carets.length === 1 && s.carets[0] === caret, 'expand cell should hold exactly #' + caret + ', got ' + JSON.stringify(s.carets)); + assert(s.aria === aria, 'aria-expanded should be "' + aria + '", got ' + JSON.stringify(s.aria)); + assert(s.expandedClass === (aria === 'true'), 'the .expanded class should match aria-expanded'); + assert(s.before === 'none' || s.before === 'normal', 'no CSS triangle next to the caret, got ::before ' + s.before); +} + +(async () => { + 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(10000); + page.on('pageerror', (e) => console.error('[pageerror]', e.message)); + + console.log(`\n=== #189 packet group caret E2E against ${BASE} ===`); + + await step('the seeded group is listed, collapsed', async () => { + await page.goto(BASE + '/#/packets?hash=' + SEED_HASH + '&timeWindow=0', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector(ROW, { timeout: 12000 }); + await page.waitForTimeout(300); + assertState(await rowState(page), 'ph-caret-right', 'false'); + }); + + await step('clicking the group expands it: down caret, aria-expanded true', async () => { + await page.click(ROW + ' td.col-expand'); + await page.waitForFunction((sel) => document.querySelector(sel).classList.contains('expanded'), ROW, { timeout: 8000 }); + assertState(await rowState(page), 'ph-caret-down', 'true'); + }); + + await step('clicking again collapses it: right caret, aria-expanded false', async () => { + await page.click(ROW + ' td.col-expand'); + await page.waitForFunction((sel) => !document.querySelector(sel).classList.contains('expanded'), ROW, { timeout: 8000 }); + assertState(await rowState(page), 'ph-caret-right', 'false'); + }); + + await browser.close(); + console.log(`\n${passed} passed, ${failed} failed`); + process.exit(failed === 0 ? 0 : 1); +})().catch((e) => { console.error(e); process.exit(1); }); diff --git a/test-packets.js b/test-packets.js index faf999239..19cebc87e 100644 --- a/test-packets.js +++ b/test-packets.js @@ -39,6 +39,13 @@ function phIcon(name) { return ''; } +// The contents of a packet row's expand cell. +function expandCell(rowHtml) { + const m = /]*>([\s\S]*?)<\/td>/.exec(rowHtml); + assert(m, 'row has an expand cell'); + return m[1]; +} + // Build a browser-like sandbox with all deps packets.js needs function makeSandbox() { const registeredPages = {}; @@ -1153,15 +1160,43 @@ console.log('\n=== packets.js: buildGroupRowHtml ==='); assert(!result.includes(phIcon('caret-down')), 'collapsed group does not show the expanded caret'); }); - // KNOWN BUG #189. Before 30627454 (#1648 M2, emoji to Phosphor sprites) a - // collapsed group showed ▶ and an expanded one ▼. The migration mapped ▶ to - // #ph-caret-up, so a collapsed group now points up. The right-pointing sprite - // is #ph-caret-right, which is also what the other collapsed disclosures use - // (analytics.js, #ptOverviewChevron). public/packets.js is not fixed yet; when - // it is, this assertion starts to pass, knownBug() reports that, and the run - // goes red until the call below is turned into a plain test(). - knownBug('#189', 'buildGroupRowHtml shows a right-pointing caret on a collapsed group', () => { - assert(api.buildGroupRowHtml(collapsedGroup).includes(phIcon('caret-right')), 'collapsed group shows a right caret'); + // #189: before 30627454 (#1648 M2, emoji to Phosphor sprites) a collapsed + // group showed ▶ and an expanded one ▼. The migration mapped ▶ to + // #ph-caret-up, so a collapsed group pointed up. The disclosure convention in + // the front end is caret-right when collapsed and caret-down when expanded + // (channels.js, network-digest.js, analytics.js #ptOverviewChevron, + // route-view.js paths chevron). + test('buildGroupRowHtml shows a right-pointing caret on a collapsed group', () => { + const cell = expandCell(api.buildGroupRowHtml(collapsedGroup)); + assert(cell.includes(phIcon('caret-right')), 'collapsed group shows a right caret'); + assert(!cell.includes(phIcon('caret-up')), 'collapsed group does not point up'); + assert(!cell.includes(phIcon('caret-down')), 'collapsed group does not show the expanded caret'); + }); + + test('buildGroupRowHtml shows a down-pointing caret on an expanded group', () => { + api._setExpanded(collapsedGroup.hash, true); + try { + const cell = expandCell(api.buildGroupRowHtml(collapsedGroup)); + assert(cell.includes(phIcon('caret-down')), 'expanded group shows a down caret'); + assert(!cell.includes(phIcon('caret-right')), 'expanded group does not show the collapsed caret'); + assert(!cell.includes(phIcon('caret-up')), 'expanded group does not point up'); + } finally { api._setExpanded(collapsedGroup.hash, false); } + }); + + test('buildGroupRowHtml: the group toggle row reports its state in aria-expanded', () => { + const header = (html) => / { + const single = Object.assign({}, collapsedGroup, { hash: 'single1', count: 1 }); + const html = api.buildGroupRowHtml(single); + assert(!html.includes('aria-expanded'), 'a row that cannot expand does not claim a state'); + assert(!/#ph-caret-/.test(expandCell(html)), 'no caret in the expand cell'); }); test('buildGroupRowHtml shows observation count badge', () => {