fix(packets): collapsed groups show a right caret, expanded a down caret (#189)

.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

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 = `<tr class="${isSingle ? '' : 'group-header'} ${isExpanded ? 'expanded' : ''}" data-hash="${p.hash}" data-action="${isSingle ? 'select-hash' : 'toggle-select'}" data-value="${p.hash}" data-entry-idx="${entryIdx}" tabindex="0" role="row"${_grpStyle ? ' style="' + _grpStyle + '"' : ''}>
-          <td class="col-expand" style="text-align:center;cursor:pointer">${isSingle ? '' : (isExpanded ? '<svg class="ph-icon" aria-hidden="true"><use href="/icons/phosphor-sprite.svg#ph-caret-down"/></svg>' : '<svg class="ph-icon" aria-hidden="true"><use href="/icons/phosphor-sprite.svg#ph-caret-up"/></svg>')}</td>
+    // #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 ? '' : '<svg class="ph-icon" aria-hidden="true"><use href="/icons/phosphor-sprite.svg#ph-caret-' + (isExpanded ? 'down' : 'right') + '"/></svg>';
+    let html = `<tr class="${isSingle ? '' : 'group-header'} ${isExpanded ? 'expanded' : ''}" data-hash="${p.hash}" data-action="${isSingle ? 'select-hash' : 'toggle-select'}" data-value="${p.hash}" data-entry-idx="${entryIdx}" tabindex="0" role="row"${isSingle ? '' : ' aria-expanded="' + isExpanded + '"'}${_grpStyle ? ' style="' + _grpStyle + '"' : ''}>
+          <td class="col-expand" style="text-align:center;cursor:pointer">${_grpCaret}</td>
           <td class="col-region">${groupRegion ? `<span class="badge-region">${groupRegion}</span>` : '—'}</td>
           <td class="col-time">${renderTimestampCell(p.latest)}</td>
           <td class="mono col-hash" data-filter-field="hash" data-filter-value="${escapeHtml(p.hash || '')}">${truncate(p.hash || '—', 8)}</td>
@@ -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); },
     };
   }
 

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; }

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); });

test-packets.js

@@ -39,6 +39,13 @@ function phIcon(name) {
   return '<svg class="ph-icon" aria-hidden="true"><use href="/icons/phosphor-sprite.svg#ph-' + name + '"/></svg>';
 }
 
+// The contents of a packet row's expand cell.
+function expandCell(rowHtml) {
+  const m = /<td class="col-expand"[^>]*>([\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) => /<tr class="group-header[^>]*>/.exec(html)[0];
+    assert(header(api.buildGroupRowHtml(collapsedGroup)).includes('aria-expanded="false"'), 'collapsed: aria-expanded=false');
+    api._setExpanded(collapsedGroup.hash, true);
+    try {
+      assert(header(api.buildGroupRowHtml(collapsedGroup)).includes('aria-expanded="true"'), 'expanded: aria-expanded=true');
+    } finally { api._setExpanded(collapsedGroup.hash, false); }
+  });
+
+  test('buildGroupRowHtml: a single-observation row has no caret and no aria-expanded', () => {
+    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', () => {