fix(packets): empty observer/type selections on Clear Filters

.github/workflows/deploy.yml

@@ -138,6 +138,8 @@ jobs:
           set -e
           node test-packet-filter.js
           node test-packet-filter-time.js
+          node test-issue-121-clear-filters-selection.js
+          node test-clear-filters.js
           node test-confidence-indicator.js
           node test-1659-analytics-warmup.js
           node test-app-api-inflight-cleanup-rejection.js

public/packets.js

@@ -791,6 +791,14 @@
   }
   window.buildPacketsQuery = buildPacketsQuery;
 
+  // Show the Clear button whenever any filter (URL-backed or not) is active.
+  function updateClearFiltersVisibility() {
+    var cb = document.getElementById('clearFiltersBtn');
+    if (!cb) return;
+    var active = !!(filters.hash || filters.node || filters.observer || filters.channel || filters.type || filters._filterExpr || filters.myNodes) || !!RegionFilter.getRegionParam() || savedTimeWindowMin !== DEFAULT_TIME_WINDOW;
+    cb.style.display = active ? '' : 'none';
+  }
+
   function updatePacketsUrl() {
     // Preserve any subpath after /packets (e.g. #/packets/<hash>).
     var cur = String(location.hash || '');
@@ -800,12 +808,7 @@
     // Don't double-encode filters.hash when it's already the path segment.
     var skipHash = !!(filters.hash && subpath === '/' + filters.hash);
     history.replaceState(null, '', '#/packets' + subpath + buildPacketsQuery(savedTimeWindowMin, RegionFilter.getRegionParam(), skipHash));
-    // Update clear-filters button visibility
-    var cb = document.getElementById('clearFiltersBtn');
-    if (cb) {
-      var active = !!(filters.hash || filters.node || filters.observer || filters.channel || filters.type || filters._filterExpr || filters.myNodes) || !!RegionFilter.getRegionParam() || savedTimeWindowMin !== DEFAULT_TIME_WINDOW;
-      cb.style.display = active ? '' : 'none';
-    }
+    updateClearFiltersVisibility();
   }
 
   let filtersBuilt = false;
@@ -1845,6 +1848,9 @@
       if (filters.type) localStorage.setItem('meshcore-type-filter', filters.type); else localStorage.removeItem('meshcore-type-filter');
       buildTypeMenu();
       updateTypeTrigger();
+      // Type is not in the URL, so leave the hash alone (updatePacketsUrl()
+      // would drop ?obs=/?viewPath=); only refresh the Clear button (#121).
+      updateClearFiltersVisibility();
       renderTableRows();
     });
 
@@ -1951,15 +1957,15 @@
       document.getElementById('fChannel').value = '';
       document.getElementById('fMyNodes').classList.remove('active');
 
-      // Reset observer multi-select
-      var obMenu = document.getElementById('observerMenu');
-      if (obMenu) obMenu.querySelectorAll('input[type=checkbox]').forEach(function(cb) { cb.checked = false; });
-      document.getElementById('observerTrigger').textContent = 'All Observers ▾';
-
-      // Reset type multi-select
-      var typeMenu = document.getElementById('typeMenu');
-      if (typeMenu) typeMenu.querySelectorAll('input[type=checkbox]').forEach(function(cb) { cb.checked = false; });
-      document.getElementById('typeTrigger').textContent = 'All Types ▾';
+      // Reset observer and type multi-selects (#121): empty the selection
+      // Sets, not only the checkboxes, or the next pick adds to the old
+      // selection; rebuilding the menus checks "All Observers"/"All Types".
+      selectedObservers.clear();
+      buildObserverMenu();
+      updateObsTrigger();
+      selectedTypes.clear();
+      buildTypeMenu();
+      updateTypeTrigger();
 
       // Reset time window to default
       savedTimeWindowMin = DEFAULT_TIME_WINDOW;

test-all.sh

@@ -11,6 +11,8 @@ echo ""
 echo "── Unit Tests ──"
 node test-packet-filter.js
 node test-packet-filter-ux.js
+node test-issue-121-clear-filters-selection.js
+node test-clear-filters.js
 node test-aging.js
 node test-issue-1065-gesture-hints-gates.js
 node test-frontend-helpers.js

test-clear-filters.js

@@ -130,18 +130,34 @@ function extractClearHandler() {
 /**
  * Extract updatePacketsUrl function body
  */
-function extractUpdatePacketsUrl() {
-  const src = fs.readFileSync(__dirname + '/public/packets.js', 'utf-8');
-  const marker = 'function updatePacketsUrl()';
+function extractFunctionSource(src, marker) {
   const idx = src.indexOf(marker);
-  assert(idx !== -1, 'updatePacketsUrl not found');
+  assert(idx !== -1, marker + ' not found');
   const fnStart = src.indexOf('{', idx);
   let depth = 0, fnEnd = -1;
   for (let i = fnStart; i < src.length; i++) {
     if (src[i] === '{') depth++;
     else if (src[i] === '}') { depth--; if (depth === 0) { fnEnd = i; break; } }
   }
-  return src.substring(fnStart + 1, fnEnd);
+  return src.substring(idx, fnEnd + 1);
+}
+function extractUpdatePacketsUrl() {
+  const src = fs.readFileSync(__dirname + '/public/packets.js', 'utf-8');
+  const body = extractFunctionSource(src, 'function updatePacketsUrl()');
+  // updatePacketsUrl() delegates button visibility to updateClearFiltersVisibility()
+  const vis = extractFunctionSource(src, 'function updateClearFiltersVisibility()');
+  return vis + '\n' + body.substring(body.indexOf('{') + 1, body.length - 1);
+}
+
+// Since #121 the handler empties the closure selection Sets and rebuilds
+// both menus through the multi-select helpers in the same scope.
+const MENU_PARAMS = ['selectedObservers', 'buildObserverMenu', 'updateObsTrigger',
+  'selectedTypes', 'buildTypeMenu', 'updateTypeTrigger'];
+function menuStubs() {
+  const s = { selectedObservers: new Set(['obs1']), selectedTypes: new Set(['4']), rebuilt: [] };
+  s.args = [s.selectedObservers, () => s.rebuilt.push('observerMenu'), () => s.rebuilt.push('observerTrigger'),
+    s.selectedTypes, () => s.rebuilt.push('typeMenu'), () => s.rebuilt.push('typeTrigger')];
+  return s;
 }
 
 const clearBody = extractClearHandler();
@@ -163,14 +179,15 @@ test('clear handler resets all filter keys to undefined/null/false', () => {
   // Build a function with the handler body and needed locals in scope
   const fn = new Function(
     'filters', 'savedTimeWindowMin', 'DEFAULT_TIME_WINDOW', '_observerFilterSet',
-    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets',
+    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets', ...MENU_PARAMS,
     `${clearBody}; return { savedTimeWindowMin, _observerFilterSet };`
   );
 
   const result = fn(
     filters, savedTimeWindowMin, DEFAULT_TIME_WINDOW, _observerFilterSet,
     ctx.localStorage, ctx.document, ctx.RegionFilter,
-    () => {}, () => {} // stubs for updatePacketsUrl and loadPackets
+    () => {}, () => {}, // stubs for updatePacketsUrl and loadPackets
+    ...menuStubs().args
   );
 
   assert.strictEqual(filters.hash, undefined, 'hash not cleared');
@@ -194,13 +211,13 @@ test('clear handler resets savedTimeWindowMin to DEFAULT_TIME_WINDOW', () => {
   // The handler assigns to savedTimeWindowMin — we need to check the returned value
   const fn = new Function(
     'filters', 'savedTimeWindowMin', 'DEFAULT_TIME_WINDOW', '_observerFilterSet',
-    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets',
+    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets', ...MENU_PARAMS,
     `${clearBody}; return { savedTimeWindowMin };`
   );
   const result = fn(
     filters, 120, DEFAULT_TIME_WINDOW, _observerFilterSet,
     ctx.localStorage, ctx.document, ctx.RegionFilter,
-    () => {}, () => {}
+    () => {}, () => {}, ...menuStubs().args
   );
 
   assert.strictEqual(result.savedTimeWindowMin, 15, 'savedTimeWindowMin not reset to default');
@@ -213,10 +230,10 @@ test('clear handler resets fTimeWindow dropdown value', () => {
   const filters = { myNodes: false };
   const fn = new Function(
     'filters', 'savedTimeWindowMin', 'DEFAULT_TIME_WINDOW', '_observerFilterSet',
-    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets',
+    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets', ...MENU_PARAMS,
     `${clearBody}; return { savedTimeWindowMin };`
   );
-  fn(filters, 120, 15, null, ctx.localStorage, ctx.document, ctx.RegionFilter, () => {}, () => {});
+  fn(filters, 120, 15, null, ctx.localStorage, ctx.document, ctx.RegionFilter, () => {}, () => {}, ...menuStubs().args);
   assert.strictEqual(elements['fTimeWindow'].value, '15', 'fTimeWindow DOM not reset');
 });
 
@@ -227,25 +244,27 @@ test('clear handler clears observer and type localStorage', () => {
   const filters = { myNodes: false };
   const fn = new Function(
     'filters', 'savedTimeWindowMin', 'DEFAULT_TIME_WINDOW', '_observerFilterSet',
-    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets',
+    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets', ...MENU_PARAMS,
     `${clearBody};`
   );
-  fn(filters, 15, 15, null, ctx.localStorage, ctx.document, ctx.RegionFilter, () => {}, () => {});
+  fn(filters, 15, 15, null, ctx.localStorage, ctx.document, ctx.RegionFilter, () => {}, () => {}, ...menuStubs().args);
   assert.strictEqual(ctx.localStorage.getItem('meshcore-observer-filter'), null);
   assert.strictEqual(ctx.localStorage.getItem('meshcore-type-filter'), null);
 });
 
-test('clear handler unchecks observer/type multi-select checkboxes', () => {
-  const { ctx, checkboxes } = makeSandbox();
+test('clear handler empties the observer/type selections and rebuilds both menus', () => {
+  const { ctx } = makeSandbox();
   const filters = { myNodes: false };
   const fn = new Function(
     'filters', 'savedTimeWindowMin', 'DEFAULT_TIME_WINDOW', '_observerFilterSet',
-    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets',
+    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets', ...MENU_PARAMS,
     `${clearBody};`
   );
-  fn(filters, 15, 15, null, ctx.localStorage, ctx.document, ctx.RegionFilter, () => {}, () => {});
-  for (const cb of checkboxes['observerMenu']) assert.strictEqual(cb.checked, false, 'observer checkbox still checked');
-  for (const cb of checkboxes['typeMenu']) assert.strictEqual(cb.checked, false, 'type checkbox still checked');
+  const m = menuStubs();
+  fn(filters, 15, 15, null, ctx.localStorage, ctx.document, ctx.RegionFilter, () => {}, () => {}, ...m.args);
+  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']);
 });
 
 test('clear handler resets RegionFilter', () => {
@@ -254,10 +273,10 @@ test('clear handler resets RegionFilter', () => {
   const filters = { myNodes: false };
   const fn = new Function(
     'filters', 'savedTimeWindowMin', 'DEFAULT_TIME_WINDOW', '_observerFilterSet',
-    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets',
+    'localStorage', 'document', 'RegionFilter', 'updatePacketsUrl', 'loadPackets', ...MENU_PARAMS,
     `${clearBody};`
   );
-  fn(filters, 15, 15, null, ctx.localStorage, ctx.document, ctx.RegionFilter, () => {}, () => {});
+  fn(filters, 15, 15, null, ctx.localStorage, ctx.document, ctx.RegionFilter, () => {}, () => {}, ...menuStubs().args);
   assert.deepStrictEqual(regionState.selected, [], 'RegionFilter not cleared');
 });
 
@@ -269,11 +288,11 @@ test('updatePacketsUrl shows clear button when time window != default', () => {
   const DEFAULT_TIME_WINDOW = 15;
   const fn = new Function(
     'filters', 'savedTimeWindowMin', 'DEFAULT_TIME_WINDOW',
-    'document', 'history', 'RegionFilter', 'buildPacketsQuery',
+    'document', 'history', 'RegionFilter', 'buildPacketsQuery', 'location',
     updateUrlBody
   );
   fn(filters, savedTimeWindowMin, DEFAULT_TIME_WINDOW,
-    ctx.document, ctx.history, ctx.RegionFilter, () => '');
+    ctx.document, ctx.history, ctx.RegionFilter, () => '', ctx.location);
   assert.strictEqual(elements['clearFiltersBtn'].style.display, '', 'clear button should be visible when time window != default');
 });
 
@@ -283,10 +302,10 @@ test('updatePacketsUrl hides clear button when all filters default', () => {
   const filters = {};
   const fn = new Function(
     'filters', 'savedTimeWindowMin', 'DEFAULT_TIME_WINDOW',
-    'document', 'history', 'RegionFilter', 'buildPacketsQuery',
+    'document', 'history', 'RegionFilter', 'buildPacketsQuery', 'location',
     updateUrlBody
   );
-  fn(filters, 15, 15, ctx.document, ctx.history, ctx.RegionFilter, () => '');
+  fn(filters, 15, 15, ctx.document, ctx.history, ctx.RegionFilter, () => '', ctx.location);
   assert.strictEqual(elements['clearFiltersBtn'].style.display, 'none', 'clear button should be hidden');
 });