fix(#1486): collapse chevron no longer reopens closed detail panel (#1492)

## Summary

Fixes #1486 — clicking the collapse chevron on a grouped packet row in
the packets table no longer reopens the detail panel that the operator
just closed.

## Root cause

In the `#pktBody` row click handler the `toggle-select` action ran
**both** `pktToggleGroup(value)` and `pktSelectHash(value)` on every
chevron click. `pktToggleGroup()` already opens the detail panel itself
(via `selectPacket()`) when it expands a row, so the trailing
`pktSelectHash()` was:

  - redundant on **expand** (the panel was already opening), and
  - harmful on **collapse** — after the operator closed the detail panel
    via the ✕ in `#pktRight`, clicking the same chevron a second time
    to collapse the tree re-fetched `/packets/<hash>` and re-populated
    the panel with the same packet, exactly the behavior the issue
    describes.

## Fix

Drop the unconditional `pktSelectHash(value)` call inside the
`toggle-select` branch. `pktToggleGroup()` already handles the
expand-side panel open; the collapse branch does no panel work, so a
closed panel stays closed.

```js
else if (action === 'toggle-select') {
  // #1486: pktToggleGroup() already opens the detail panel on EXPAND
  // (via selectPacket()), and must NOT open it on COLLAPSE.
  pktToggleGroup(value);
}
```

## Tests

- New Playwright E2E `test-issue-1486-collapse-reopens-detail-e2e.js`
  walks the operator-visible repro: expand → assert panel open →
  click ✕ → assert panel empty → click chevron again → assert row
  collapsed AND panel STILL empty.
- Committed red-first: the test was added in its own commit and FAILS
  on the unpatched code (3 passed / 1 failed), then GREEN on the fix
  commit (4 passed / 0 failed).
- CI workflow seeds two extra observations onto the newest fixture
  transmission so a grouped (`toggle-select`) row exists; without this
  the fixture renders only flat rows and the chevron can't be
  exercised.

## Reproduction (manual, against staging or local)

1. Open `/#/packets` on desktop.
2. Click a grouped row's `▶` chevron — the tree expands and the detail
   panel opens on the right.
3. Click the `✕` in the top-right of the detail panel — panel goes back
   to "Select a packet to view details".
4. Click the same chevron (now `▼`) again — **before:** detail panel
   reopens with the same packet. **After:** the row collapses and the
   panel stays empty.

---------

Co-authored-by: mc-bot <bot@meshcore.local>
This commit is contained in:
Kpa-clawbot
2026-05-29 08:17:16 -07:00
committed by GitHub
parent 268751ff56
commit 7fcb226cd8
3 changed files with 187 additions and 1 deletions
+49
View File
@@ -241,6 +241,54 @@ jobs:
- name: Freshen fixture timestamps
run: bash tools/freshen-fixture.sh test-fixtures/e2e-fixture.db
- name: Seed grouped-packet row for #1486 collapse test
# The committed fixture has 499 packets, each with exactly ONE
# observation, so the packets-page renders only flat
# (select-hash) rows. The #1486 repro needs at least one grouped
# (toggle-select) row. Insert a NEW transmission with 3
# observations.
#
# The server's async hash-migrate (cmd/server/hash_migrate.go)
# recomputes `transmissions.hash` from `raw_hex` via
# ComputeContentHash(), so the inserted hash MUST equal that
# function's output for the chosen raw_hex — otherwise the row
# gets relabelled and the E2E can't find it.
#
# raw_hex 15000102030405060708090a0b0c0d0e0f
# → header=0x15 (route_type=1, payload_type=5)
# → ComputeContentHash(...) = fae0c9e6d357a814
#
# The first_seen / observation timestamps are pinned to a date
# within retentionHours but outside the default 15-min UI
# window so the row is hidden in the default view (keeping
# test-e2e-playwright's first-10-rows hex-pane test
# unaffected) and reachable via the explicit ?timeWindow=0
# deep-link the #1486 test uses.
run: |
sqlite3 test-fixtures/e2e-fixture.db <<'SQL'
-- Sort the seeded row LAST in BOTH default packets views:
-- • flat view sorts by transmissions.id DESC → id=0 puts it last
-- • grouped view (#default for the packets page) sorts by
-- MAX(observations.timestamp) DESC → we must keep our obs
-- timestamps OLDER than every other fixture observation.
-- Fixture (after freshen) has obs timestamps spanning
-- 2026-05-17 16:01:39Z .. 2026-05-28 00:00:00Z (max).
-- Note: freshen only shifts transmissions.first_seen forward
-- to ~now; observation.timestamp is left alone except for
-- the timestamp=0 case.
-- Use 2026-05-15 (~2 days older than the oldest fixture obs)
-- so our row sorts LAST in the grouped view too, keeping
-- test-e2e-playwright's first-10-rows hex-pane test
-- unaffected. The #1486 test still reaches the row via the
-- explicit hash + ?timeWindow=0 deep-link.
INSERT INTO transmissions(id,raw_hex,hash,first_seen,route_type,payload_type,payload_version,decoded_json,channel_hash,from_pubkey)
VALUES (0,'15000102030405060708090a0b0c0d0e0f','fae0c9e6d357a814','2026-05-15T00:00:00Z',1,5,0,'{"type":"CHAN","channel":"#test","text":"#1486 fixture"}',NULL,NULL);
INSERT INTO observations(transmission_id,observer_idx,direction,snr,rssi,score,path_json,timestamp,resolved_path) VALUES
(0,1,'rx',5.0,-95,0,'["AA"]',CAST(strftime('%s','2026-05-15T00:00:00Z') AS INTEGER),'["aa00000000000000000000000000000000000000000000000000000000000000"]'),
(0,2,'rx',5.5,-92,0,'["BB"]',CAST(strftime('%s','2026-05-15T00:00:00Z') AS INTEGER),'["bb00000000000000000000000000000000000000000000000000000000000000"]'),
(0,3,'rx',6.0,-90,0,'["CC"]',CAST(strftime('%s','2026-05-15T00:00:00Z') AS INTEGER),'["cc00000000000000000000000000000000000000000000000000000000000000"]');
SQL
- name: Migrate fixture DB to current schema (#1287)
# Server now ASSERTs schema is migrated and refuses to start
# otherwise (cmd/server/main.go: dbschema.AssertReady). In prod
@@ -302,6 +350,7 @@ jobs:
BASE_URL=http://localhost:13581 node test-issue-1146-path-link-contrast-e2e.js 2>&1 | tee -a e2e-output.txt
BASE_URL=http://localhost:13581 node test-issue-1147-section-order-e2e.js 2>&1 | tee -a e2e-output.txt
BASE_URL=http://localhost:13581 node test-issue-1151-orphan-separators-e2e.js 2>&1 | tee -a e2e-output.txt
BASE_URL=http://localhost:13581 node test-issue-1486-collapse-reopens-detail-e2e.js 2>&1 | tee -a e2e-output.txt
CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-logo-rebrand-e2e.js 2>&1 | tee -a e2e-output.txt
CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-logo-theme-e2e.js 2>&1 | tee -a e2e-output.txt
CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-logo-default-sage-teal-e2e.js 2>&1 | tee -a e2e-output.txt
+8 -1
View File
@@ -1939,7 +1939,14 @@
}
}
else if (action === 'select-hash') pktSelectHash(value);
else if (action === 'toggle-select') { pktToggleGroup(value); pktSelectHash(value); }
else if (action === 'toggle-select') {
// #1486: pktToggleGroup() already opens the detail panel on EXPAND
// (via selectPacket()), and must NOT open it on COLLAPSE. The
// previously-unconditional pktSelectHash() trailing call was both
// redundant on expand AND reopened the panel the operator had just
// closed when they clicked the chevron to collapse — drop it.
pktToggleGroup(value);
}
};
pktBody.addEventListener('click', handler);
pktBody.addEventListener('keydown', handler);
@@ -0,0 +1,130 @@
/**
* E2E (#1486): Packets-page collapse chevron must not reopen the detail
* panel that the operator just closed.
*
* Repro:
* 1. Open /#/packets on desktop.
* 2. Click a group-header row's expand chevron → tree expands, detail
* panel opens on the right.
* 3. Close the detail panel via the ✕ button in its top-right.
* 4. Click the SAME chevron again, intending to collapse the tree.
*
* Bug: the second click reopens the detail panel (because the
* `toggle-select` action handler unconditionally calls both
* `pktToggleGroup(value)` AND `pktSelectHash(value)` — the latter
* re-fires selectPacket even when the row was being collapsed).
*
* Fix expectation: after the second chevron click the tree row must be
* collapsed AND the #pktRight panel must remain in its "empty" /
* collapsed state.
*
* The CI workflow's "Seed grouped-packet row for #1486" step inserts a
* transmission with hash SEED_HASH that carries 3 observations so the
* page renders a grouped (toggle-select) row. When running locally,
* seed the same row (see .github/workflows/deploy.yml for the SQL).
*
* Usage: BASE_URL=http://localhost:13581 node test-issue-1486-collapse-reopens-detail-e2e.js
*/
'use strict';
const { chromium } = require('playwright');
const BASE = process.env.BASE_URL || 'http://localhost:13581';
const SEED_HASH = 'fae0c9e6d357a814';
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'); }
(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=== #1486 packets collapse-reopens-detail E2E against ${BASE} ===`);
await step('navigate to /packets filtered to seeded grouped row', async () => {
// Deep-link with the seeded hash filter + unbounded time window so
// the seeded row is the only thing in the table and is visible.
await page.goto(BASE + '/#/packets?hash=' + SEED_HASH + '&timeWindow=0', { waitUntil: 'domcontentloaded' });
await page.waitForSelector('#pktBody', { timeout: 8000 });
await page.waitForFunction(
(h) => !!document.querySelector(`#pktBody tr[data-hash="${h}"][data-action="toggle-select"]`),
SEED_HASH,
{ timeout: 12000 }
);
});
await step('1st chevron click expands row AND opens detail panel', async () => {
await page.evaluate((h) => {
const row = document.querySelector(`#pktBody tr[data-hash="${h}"][data-action="toggle-select"]`);
row.click();
}, SEED_HASH);
await page.waitForFunction(
() => {
const p = document.getElementById('pktRight');
return p && !p.classList.contains('empty');
},
{ timeout: 8000 }
);
const state = await page.evaluate((h) => {
const row = document.querySelector(`#pktBody tr[data-hash="${h}"][data-action="toggle-select"]`);
const panel = document.getElementById('pktRight');
return {
expanded: !!row && row.classList.contains('expanded'),
panelEmpty: !!panel && panel.classList.contains('empty'),
};
}, SEED_HASH);
assert(state.expanded, 'row should be expanded after 1st click');
assert(!state.panelEmpty, 'detail panel should be open after 1st click');
});
await step('Close detail panel via ✕ button — panel goes back to empty', async () => {
await page.evaluate(() => {
const btn = document.querySelector('#pktRight .panel-close-btn');
if (btn) btn.click();
});
await page.waitForFunction(
() => {
const p = document.getElementById('pktRight');
return p && p.classList.contains('empty');
},
{ timeout: 5000 }
);
});
await step('2nd chevron click COLLAPSES the row WITHOUT reopening the detail panel', async () => {
await page.evaluate((h) => {
const row = document.querySelector(`#pktBody tr[data-hash="${h}"][data-action="toggle-select"]`);
row.click();
}, SEED_HASH);
// Give any (incorrect) async pktSelectHash time to re-populate.
await page.waitForTimeout(1000);
const state = await page.evaluate((h) => {
const row = document.querySelector(`#pktBody tr[data-hash="${h}"][data-action="toggle-select"]`);
const panel = document.getElementById('pktRight');
return {
expanded: !!row && row.classList.contains('expanded'),
panelEmpty: !!panel && panel.classList.contains('empty'),
};
}, SEED_HASH);
assert(!state.expanded, 'row must be collapsed after 2nd chevron click');
assert(state.panelEmpty, 'detail panel must NOT reopen after collapse');
});
await browser.close();
console.log(`\n${passed} passed, ${failed} failed`);
process.exit(failed === 0 ? 0 : 1);
})().catch(e => { console.error(e); process.exit(1); });