fix(packets): URL and modal leftovers from #167 (#180)

.github/workflows/deploy.yml

@@ -429,6 +429,7 @@ jobs:
           BASE_URL=http://localhost:13581 node test-slideover-1056-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1692-packets-init-parallel-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-147-packets-url-obs-e2e.js 2>&1 | tee -a e2e-output.txt
+          BASE_URL=http://localhost:13581 node test-issue-180-packets-url-modal-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-slideover-1168-munger-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-logo-pulse-1173-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1122-packets-filter-ux-e2e.js 2>&1 | tee -a e2e-output.txt

public/packet-path-map.js

@@ -80,11 +80,30 @@
   // only this top layer. Stopping it keeps the layers below (the packets
   // detail pane, a SlideOver), whose Escape handlers listen on document in
   // the bubble phase, open until the next Escape (#167).
+  // A layer opened over the modal afterwards (the global search via Ctrl+K,
+  // a nav menu, the More sheet, a filter popover) holds focus, and Escape is
+  // that layer's: the event is left alone then (#180).
   function onKeydown(e) {
-    if (e.key === 'Escape' && document.getElementById('packetPathModal')) {
-      e.stopPropagation();
-      close();
-    }
+    var overlay = document.getElementById('packetPathModal');
+    if (e.key !== 'Escape' || !overlay || focusInLayerAbove(overlay, e.target)) return;
+    e.stopPropagation();
+    close();
+  }
+
+  // True when el, the focused element, is in a floating layer drawn over the
+  // modal: outside it, inside a position:fixed ancestor other than the
+  // packets detail surfaces the modal opens over (SlideOver, mobile sheet),
+  // and topmost at its own centre, i.e. not under the modal's backdrop. The
+  // sticky top nav and the page are not floating layers, so Escape with focus
+  // there still closes the modal.
+  function focusInLayerAbove(overlay, el) {
+    if (!el || !el.getBoundingClientRect || overlay.contains(el)) return false;
+    var layer = el;
+    while (layer && layer !== document.body && getComputedStyle(layer).position !== 'fixed') layer = layer.parentElement;
+    if (!layer || layer === document.body || layer.matches('.slide-over-panel, .mobile-detail-sheet')) return false;
+    var r = el.getBoundingClientRect();
+    var top = document.elementFromPoint(r.left + r.width / 2, r.top + r.height / 2);
+    return !!top && layer.contains(top);
   }
 
   // A #/packets/<hash>?…&viewPath=1 URL describes this modal as open (#147).
@@ -103,6 +122,41 @@
     history.replaceState(null, '', h.slice(0, q) + (kept.length ? '?' + kept.join('&') : ''));
   }
 
+  // The modal belongs to the route it was opened on (#180): the hash path
+  // (#/packets/<hash>, without the query) at open(). A hashchange to another
+  // path -- Back/Forward, a nav link -- closes it.
+  var openPath = null;
+  // The id open() put in the history.state of the #/packets/<hash> entry it
+  // opened on, and the ids of entries whose modal was closed while another
+  // route was shown. Such an entry still carries ?viewPath=1, so Back/Forward
+  // onto it must not reopen the modal (restore()). The id is per entry, so a
+  // new link to the same URL (no state) still opens it. Bounded list.
+  var openEntry = null;
+  var closedEntries = [];
+  var MAX_CLOSED_ENTRIES = 50;
+
+  function hashPath(h) {
+    h = String(h || '');
+    var q = h.indexOf('?');
+    return q < 0 ? h : h.slice(0, q);
+  }
+
+  function tagEntry() {
+    if (typeof history === 'undefined' || !history.replaceState || openPath.indexOf('#/packets/') !== 0) return null;
+    var id = Date.now().toString(36) + Math.random().toString(36).slice(2, 8);
+    var state = {};
+    var cur = history.state;
+    if (cur && typeof cur === 'object') for (var k in cur) state[k] = cur[k];
+    state.packetPathModal = id;
+    // No URL argument: only the entry's state changes.
+    history.replaceState(state, '');
+    return id;
+  }
+
+  function onHashChange() {
+    if (hashPath(location.hash) !== openPath) close();
+  }
+
   // Removes the modal; true when one was open.
   function removeModal() {
     var overlay = document.getElementById('packetPathModal');
@@ -112,11 +166,32 @@
       activeMap = null;
     }
     document.removeEventListener('keydown', onKeydown, true);
+    window.removeEventListener('hashchange', onHashChange);
     return !!overlay;
   }
 
   function close() {
-    if (removeModal()) dropViewPathParam();
+    var elsewhere = hashPath(location.hash) !== openPath;
+    var entry = openEntry;
+    if (!removeModal()) return;
+    openPath = openEntry = null;
+    if (!elsewhere) {
+      dropViewPathParam();
+    } else if (entry) {
+      closedEntries.push(entry);
+      if (closedEntries.length > MAX_CLOSED_ENTRIES) closedEntries.shift();
+    }
+  }
+
+  // Reopens the modal that a #/packets/<hash>?…&viewPath=1 entry describes
+  // (packets.js init(): a shared link, a reload, Back/Forward), unless that
+  // entry's modal was closed while another route was shown (#180). Returns
+  // whether it opened; when not, packets.js drops ?viewPath=1 from the URL.
+  function restore(hash) {
+    var state = typeof history !== 'undefined' ? history.state : null;
+    if (state && state.packetPathModal && closedEntries.indexOf(state.packetPathModal) !== -1) return false;
+    open(hash);
+    return true;
   }
 
   // A short prefix marking a node's role in tooltips -- purely a label,
@@ -207,6 +282,9 @@
     var closeBtn = document.getElementById('packetPathClose');
     if (closeBtn) closeBtn.addEventListener('click', close);
     document.addEventListener('keydown', onKeydown, true);
+    openPath = hashPath(location.hash);
+    openEntry = tagEntry();
+    window.addEventListener('hashchange', onHashChange);
 
     // Shareable link: #/packets/<hash>?viewPath=1 -- packets.js's init()
     // (public/packets.js) checks for viewPath=1 and re-opens this exact
@@ -580,5 +658,5 @@
     if (statusEl) statusEl.textContent = statusParts.join(' · ');
   }
 
-  window.PacketPathMap = { open: open, close: close };
+  window.PacketPathMap = { open: open, close: close, restore: restore };
 })();

public/packets.js

@@ -816,8 +816,9 @@
   //   selection. That re-runs init(), whose cold-load call here writes the
   //   restored filters back; the previous packet's ?obs= does not apply.
   // detail (optional): { subpath: '/<hash|id>' or '', obs: id or null } when
-  // the caller changes the selection; without it the current subpath and
-  // ?obs= are kept (filter changes, Clear Filters, cold load).
+  // the caller changes the selection or closes the detail (Clear Filters,
+  // #180); without it the current subpath and ?obs= are kept (filter
+  // changes, cold load).
   function updatePacketsUrl(detail) {
     var cur = String(location.hash || '');
     // The packets list route: #/packets, #/packets/…, #/packets?…, or no
@@ -842,7 +843,9 @@
       if (pathModal && subpath === '/' + pathModal.dataset.hash) keep.push('viewPath=1');
     }
     if (keep.length) query += (query ? '&' : '?') + keep.join('&');
-    history.replaceState(null, '', '#/packets' + subpath + query);
+    // Keeps the entry's history.state: packet-path-map.js marks the entry
+    // its modal opened on there (#180).
+    history.replaceState(history.state, '', '#/packets' + subpath + query);
     updateClearFiltersVisibility();
   }
 
@@ -935,14 +938,21 @@
   }
 
   function closeDetailPanel() {
+    // The ≤640 px bottom sheet as well: Escape used to reset only the
+    // desktop pane and never closed the sheet (#180).
+    var sheet = document.getElementById('mobileDetailSheet');
+    if (sheet) sheet.classList.remove('open');
     var panel = document.getElementById('pktRight');
     if (panel) {
       panel.classList.add('empty');
       panel.innerHTML = '<div class="panel-resize-handle" id="pktResizeHandle"></div>' + PANEL_CLOSE_HTML + '<span>Select a packet to view details</span>';
       var layout = panel.closest('.split-layout');
       if (layout) layout.classList.add('detail-collapsed');
+      // Re-render only to drop a row's selection highlight (#180: Clear
+      // Filters closes the detail and reloads the rows itself).
+      var wasSelected = selectedId !== null;
       selectedId = null;
-      renderTableRows();
+      if (wasSelected) renderTableRows();
     }
   }
 
@@ -1173,11 +1183,13 @@
     // link reopens the exact same modal instead of leaving the recipient
     // on the plain packet detail page. Independent of the packets-list
     // rendering below (the modal fetches its own data), so it's safe to
-    // fire immediately.
+    // fire immediately. restore() skips a history entry whose modal was
+    // closed on another page (#180); the cold-load updatePacketsUrl() below
+    // then drops ?viewPath=1, as the modal is not open.
     var _urlViewPath = _initUrlParams.get('viewPath');
     if (_urlViewPath === '1') {
       var _viewPathHash = directPacketHash || filters.hash;
-      if (_viewPathHash && window.PacketPathMap) window.PacketPathMap.open(_viewPathHash);
+      if (_viewPathHash && window.PacketPathMap) window.PacketPathMap.restore(_viewPathHash);
     }
     var _urlNode = _initUrlParams.get('node');
     if (_urlNode) { filters.node = _urlNode; filters.nodeName = _urlNode.slice(0, 8); }
@@ -2019,8 +2031,14 @@
       // Reset region filter
       RegionFilter.setSelected([]);
 
+      // Clear also leaves the packet detail (#180): a #/packets/<hash>
+      // subpath sets filters.hash again on load, so after Clear the URL is
+      // the list and the detail it named is closed.
+      selectedObservationId = null;
+      closeDetailPanel();
+
       // Update URL and reload
-      updatePacketsUrl();
+      updatePacketsUrl({ subpath: '', obs: null });
       loadPackets();
     });
     // Show clear button if page loaded with active filters (e.g. from URL params)

public/style.css

@@ -1355,16 +1355,20 @@ img.brand-logo {
  * body.style.overflow capture-and-restore string approach which
  * corrupted overflow under last-writer-wins races. */
 body.scroll-locked { overflow: hidden; }
+/* #180: the SlideOver is a modal dialog (role=dialog, aria-modal, focus
+ * trap), so it stacks on the modal tokens, above the sticky .top-nav (1100).
+ * At 1000/1001 the panel's header and close button sat under the nav at
+ * 641–1023 px, and a click on × landed on the nav. */
 .slide-over-backdrop {
-  position: fixed; inset: 0; z-index: 1000;
+  position: fixed; inset: 0; z-index: var(--z-modal-backdrop);
   background: rgba(0, 0, 0, 0.5);
   cursor: pointer;
 }
 .slide-over-backdrop[hidden] { display: none; }
 .slide-over-panel {
   position: fixed; top: 0; right: 0; bottom: 0;
   width: min(480px, 90vw);
-  z-index: 1001;
+  z-index: var(--z-modal);
   background: var(--detail-bg, var(--card-bg, #1f2937));
   color: var(--text, inherit);
   border-left: 1px solid var(--border);
@@ -1392,7 +1396,7 @@ body.scroll-locked { overflow: hidden; }
   color: var(--text-muted, #94a3b8);
   padding: 4px 8px; border-radius: 4px;
   /* WCAG 2.5.5 / Apple HIG — 48px tap target on touch devices. */
-  min-width: 44px; min-height: 44px;
+  min-width: 48px; min-height: 48px;
   display: inline-flex; align-items: center; justify-content: center;
 }
 .slide-over-close:hover { color: var(--text, inherit); background: var(--row-hover); }

test-all.sh

@@ -33,6 +33,7 @@ run test-packet-filter-ux.js
 run test-issue-121-clear-filters-selection.js
 run test-clear-filters.js
 run test-issue-147-packets-url-detail-params.js
+run test-issue-180-packets-detail-close.js
 run test-aging.js
 run test-issue-1065-gesture-hints-gates.js
 run test-frontend-helpers.js

test-clear-filters.js

@@ -152,13 +152,15 @@ function extractUpdatePacketsUrl() {
 }
 
 // Since #121 the handler empties the closure selection Sets and rebuilds
-// both menus through the multi-select helpers in the same scope.
+// both menus through the multi-select helpers in the same scope. Since #180
+// it also closes the packet detail (closeDetailPanel, selectedObservationId).
 const MENU_PARAMS = ['selectedObservers', 'buildObserverMenu', 'updateObsTrigger',
-  'selectedTypes', 'buildTypeMenu', 'updateTypeTrigger'];
+  'selectedTypes', 'buildTypeMenu', 'updateTypeTrigger', 'closeDetailPanel', 'selectedObservationId'];
 function menuStubs() {
-  const s = { selectedObservers: new Set(['obs1']), selectedTypes: new Set(['4']), rebuilt: [] };
+  const s = { selectedObservers: new Set(['obs1']), selectedTypes: new Set(['4']), rebuilt: [], detailClosed: 0 };
   s.args = [s.selectedObservers, () => s.rebuilt.push('observerMenu'), () => s.rebuilt.push('observerTrigger'),
-    s.selectedTypes, () => s.rebuilt.push('typeMenu'), () => s.rebuilt.push('typeTrigger')];
+    s.selectedTypes, () => s.rebuilt.push('typeMenu'), () => s.rebuilt.push('typeTrigger'),
+    () => { s.detailClosed++; }, '123'];
   return s;
 }
 
@@ -267,6 +269,7 @@ test('clear handler empties the observer/type selections and rebuilds both menus
   assert.strictEqual(m.selectedObservers.size, 0, 'observer selection not emptied');
   assert.strictEqual(m.selectedTypes.size, 0, 'type selection not emptied');
   assert.deepStrictEqual(m.rebuilt, ['observerMenu', 'observerTrigger', 'typeMenu', 'typeTrigger']);
+  assert.strictEqual(m.detailClosed, 1, 'packet detail not closed (#180)');
 });
 
 test('clear handler resets RegionFilter', () => {

test-issue-121-clear-filters-selection.js

@@ -103,7 +103,7 @@ function mount(state, filters) {
     getElementById: (id) => elements[id] || (elements[id] = makeEl(id)),
     createElement: (tag) => makeEl('_' + tag),
   };
-  const calls = { loadPackets: 0, renderTableRows: 0 };
+  const calls = { loadPackets: 0, renderTableRows: 0, closeDetailPanel: 0 };
   const sandbox = {
     document,
     location: state.location,
@@ -124,6 +124,9 @@ function mount(state, filters) {
     debounce: (fn) => fn,
     loadPackets: () => { calls.loadPackets++; },
     renderTableRows: () => { calls.renderTableRows++; },
+    // #180: Clear Filters also closes the packet detail.
+    closeDetailPanel: () => { calls.closeDetailPanel++; },
+    selectedObservationId: null,
     filters,
     observers: [{ id: 'obsA', name: 'Alpha' }, { id: 'obsB', name: 'Bravo' }, { id: 'obsC', name: 'Charlie' }],
     SHORT_BY_ID: { 0: 'REQ', 4: 'ADVERT', 5: 'GRP_TXT', 9: 'TRACE' },

test-issue-147-packets-url-detail-params.js

@@ -3,8 +3,9 @@
  * updatePacketsUrl() rebuilds #/packets/<hash>?… from buildPacketsQuery(),
  * which only knows filter params. The packet-detail params it does not own,
  * ?obs= (selected observation) and ?viewPath=1 (View Path modal), must
- * survive: on cold load (init() calls it to show the Clear button), on every
- * filter change and on Clear Filters.
+ * survive: on cold load (init() calls it to show the Clear button) and on
+ * every filter change. Clear Filters leaves the detail, so it drops them with
+ * the subpath (#180).
  *
  * Decision under test: ?obs= travels with the detail subpath; ?viewPath=1
  * describes the open View Path modal, so it is in the URL exactly while that
@@ -81,7 +82,7 @@ function makeSandbox(startHash, opts) {
     console, URLSearchParams, encodeURIComponent, decodeURIComponent,
     String, Number, Array, Object, RegExp, JSON, Math, Set, Map, Error,
     location: { hash: startHash },
-    history: { replaceState(_s, _t, url) { urls.push(url); ctx.location.hash = url; } },
+    history: { state: opts.state === undefined ? null : opts.state, replaceState(st, _t, url) { urls.push(url); ctx.history.state = st; ctx.location.hash = url; } },
     document: { getElementById: (id) => els[id] || null },
     localStorage: {
       getItem: (k) => (k in store ? store[k] : null),
@@ -105,6 +106,8 @@ function makeSandbox(startHash, opts) {
     'var _observerFilterSet = null; var selectedObservers = new Set(); var selectedTypes = new Set();\n' +
     'function buildObserverMenu() {} function updateObsTrigger() {} function buildTypeMenu() {}\n' +
     'function updateTypeTrigger() {} function loadPackets() {}\n' +
+    // #180: Clear Filters closes the detail; record that it did.
+    'var selectedObservationId = null; var __detailClosed = 0; function closeDetailPanel() { __detailClosed++; }\n' +
     URL_FUNCS + '\nfunction __clearFilters() {' + CLEAR_BODY + '}',
     ctx, { filename: 'packets.js (#147 extract)' });
   return {
@@ -231,19 +234,33 @@ test('list view (#/packets, no detail) gets no ?obs=', () => {
   assert.strictEqual(s.update(), '#/packets?observer=OBS1');
 });
 
-// ---- Clear Filters (#121/#132): filter params go, detail subpath + obs stay ----
+// ---- Clear Filters (#121/#132): filter params go ----
+// #180 decision: Clear also leaves the detail. A #/packets/<hash> subpath
+// sets filters.hash again on load, so a Clear that kept it was undone by a
+// reload. Clear closes the detail and writes the list URL #/packets?….
 
-test('Clear Filters on a detail URL removes filter params, keeps subpath and ?obs=', () => {
+test('Clear Filters on a detail URL removes filter params and the detail (subpath, ?obs=) (#180)', () => {
   const s = makeSandbox(DETAIL + '?timeWindow=60&region=EU&observer=OBS1&obs=123');
-  s.set("filters.hash = 'abc123'; filters.observer = 'OBS1'; savedTimeWindowMin = 60; __region = ['EU']");
-  assert.strictEqual(s.clear(), DETAIL + '?obs=123');
+  s.set("filters.hash = 'abc123'; filters.observer = 'OBS1'; savedTimeWindowMin = 60; __region = ['EU']; selectedObservationId = '123'");
+  assert.strictEqual(s.clear(), '#/packets');
   assert.strictEqual(s.els.clearFiltersBtn.style.display, 'none', 'Clear button still visible after clearing');
+  assert.strictEqual(s.ctx.__detailClosed, 1, 'Clear did not close the detail');
+  assert.strictEqual(s.ctx.selectedObservationId, null, 'selected observation kept');
 });
 
-test('Clear Filters on a detail URL drops viewPath=1 when the modal is closed', () => {
+test('Clear Filters on a detail URL also drops viewPath=1 (#180)', () => {
   const s = makeSandbox(DETAIL + '?observer=OBS1&obs=123&viewPath=1', { modalOpen: false });
   s.set("filters.observer = 'OBS1'");
-  assert.strictEqual(s.clear(), DETAIL + '?obs=123');
+  assert.strictEqual(s.clear(), '#/packets');
+});
+
+test('Clear Filters keeps the default-route form: no hash filter left for a reload to re-apply (#180)', () => {
+  // The reload of what Clear wrote must not filter: no subpath, no ?hash=.
+  const s = makeSandbox(DETAIL + '?obs=123');
+  s.set("filters.hash = 'abc123'");
+  const h = s.clear();
+  assert(!/^#\/packets\//.test(h), 'detail subpath kept: ' + h);
+  assert.strictEqual(params(h).get('hash'), null, 'hash filter in the URL: ' + h);
 });
 
 test('Clear Filters on the list view still yields bare #/packets', () => {
@@ -309,6 +326,23 @@ test('closing the detail drops subpath and detail params, keeps the filters', ()
   assert.strictEqual(s.update({ subpath: '', obs: null }), '#/packets?timeWindow=60&observer=OBS1');
 });
 
+// ---- history.state survives every list-URL write (#180) ----
+// packet-path-map.js marks the #/packets/<hash> entry its modal opened on
+// in history.state, so Back/Forward onto it after the modal was closed on
+// another page does not reopen it. The writes here must not wipe the mark.
+
+test('updatePacketsUrl() keeps the entry\'s history.state (filter change, selection, Clear) (#180)', () => {
+  const mark = { packetPathModal: 'e1' };
+  const s = makeSandbox(DETAIL + '?obs=123&viewPath=1', { modalOpen: true, state: mark });
+  s.set("filters.observer = 'OBS1'");
+  s.update();
+  assert.strictEqual(s.ctx.history.state, mark, 'filter change dropped the state');
+  s.update({ subpath: '/abc123', obs: '7' });
+  assert.strictEqual(s.ctx.history.state, mark, 'selection dropped the state');
+  s.clear();
+  assert.strictEqual(s.ctx.history.state, mark, 'Clear dropped the state');
+});
+
 // ---- Route guard (#167 r2): the list URL is written only on #/packets ----
 // On the standalone #/packet/<id> page an observation click used to rewrite
 // the address bar to #/packets/<hash>?<list filters>&obs=… while the