fix(#1281): hide empty Location row + theme map link via --accent (#1284)

## Summary Minimal fix for #1281 — two surgical changes to the packet detail pane: 1. **Hide the `Location` row when transmitter GPS is unavailable.** Only ADVERT packets carry unencrypted GPS in their payload, so ~90% of packet types (TXT_MSG, GRP_TXT, ACK, REQ, MULTIPART, …) were rendering `<dt>Location</dt><dd>—</dd>` for nothing. We now skip the `<dt>/<dd>` pair entirely when `locationHtml` is empty. ADVERT rendering is unchanged. 2. **Fix the `📍map` link contrast in dark mode.** The trailing link had only `style="font-size:0.85em"` and inherited the UA-default `<a>` blue (`rgb(0,0,238)`) → unreadable against `--card-bg` in dark theme. Replaced inline style with `class="loc-map-link"` and added a small CSS rule that pulls color from `var(--accent)`. ### Out of scope (per operator direction) The original issue also proposed adding an `Rx:` observer-GPS line and distance-from-observer. **Not in this PR** — operator decided the existing observer IATA pill already conveys that, so adding more rows here is unnecessary. Bullets 1–2 of the issue's "Acceptance" list are covered; the multi-line `Tx:`/`Rx:` reformat is intentionally not done. ## TDD - **Red** `d465cf84` — `test-issue-1281-location-row-e2e.js` asserting: - Non-ADVERT detail must NOT contain `<dt>Location</dt>` - ADVERT detail STILL contains `<dt>Location</dt>` with GPS coords - `.loc-map-link` computed `color` equals `var(--accent)` (not UA blue) Verified to fail on master (`1 passed, 2 failed`) — see commit body. - **Green** `8c9bd8cb` — implementation. All three assertions pass. - **CI wiring** `9571b4f4` — added the test to `deploy.yml`'s E2E block. ## Files changed - `public/packets.js` — empty-string default for `locationHtml`, conditional `<dt>/<dd>` render, three sites swap inline style → class. - `public/style.css` — new `.loc-map-link { color: var(--accent); … }` rule next to `.detail-meta dd`. - `test-issue-1281-location-row-e2e.js` — new Playwright E2E. - `.github/workflows/deploy.yml` — one-line CI hook. ## Acceptance verification (against fixture DB) ``` === #1281 Location row + map link contrast E2E against http://localhost:13581 === ✓ Non-ADVERT packet detail does NOT render <dt>Location</dt> ✓ ADVERT packet detail STILL renders <dt>Location</dt> with GPS coords link.color=rgb(74, 158, 255) --accent→rgb(74, 158, 255) ✓ 📍map link uses class="loc-map-link" with color = var(--accent) 3 passed, 0 failed ``` Fixes #1281 --------- Co-authored-by: bot <bot@local>
2026-08-28 20:38:29 +00:00 · 2026-05-18 23:37:04 -07:00
parent d667dc0a74
commit e2d320449b
4 changed files with 169 additions and 6 deletions
@@ -265,6 +265,7 @@ jobs:
          BASE_URL=http://localhost:13581 node test-issue-1224-channels-mobile-ux-e2e.js 2>&1 | tee -a e2e-output.txt
          BASE_URL=http://localhost:13581 node test-issue-1236-map-mobile-e2e.js 2>&1 | tee -a e2e-output.txt
          BASE_URL=http://localhost:13581 node test-issue-1273-qr-overlay-height-e2e.js 2>&1 | tee -a e2e-output.txt
+          BASE_URL=http://localhost:13581 node test-issue-1281-location-row-e2e.js 2>&1 | tee -a e2e-output.txt
          CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1206-resize-observer-leak-e2e.js 2>&1 | tee -a e2e-output.txt
          CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-drawer-1064-e2e.js 2>&1 | tee -a e2e-output.txt

@@ -2841,15 +2841,18 @@
      }
    }

-    // Location: from ADVERT lat/lon, or from known node via pubkey/sender name
-    let locationHtml = '—';
+    // Location: from ADVERT lat/lon, or from known node via pubkey/sender name.
+    // Issue #1281: only render the row when we actually have transmitter GPS.
+    // Non-ADVERT packets don't carry GPS in the unencrypted payload, so the row
+    // would otherwise render as "—" and waste a slot on ~90% of packet types.
+    let locationHtml = '';
    let locationNodeKey = null;
    if (decoded.lat != null && decoded.lon != null && !(decoded.lat === 0 && decoded.lon === 0)) {
      locationNodeKey = decoded.pubKey || decoded.srcPubKey || '';
      const nodeName = decoded.name || '';
      locationHtml = `${decoded.lat.toFixed(5)}, ${decoded.lon.toFixed(5)}`;
      if (nodeName) locationHtml = `${escapeHtml(nodeName)} — ${locationHtml}`;
-      if (locationNodeKey) locationHtml += ` <a href="#/map?node=${encodeURIComponent(locationNodeKey)}" style="font-size:0.85em">📍map</a>`;
+      if (locationNodeKey) locationHtml += ` <a href="#/map?node=${encodeURIComponent(locationNodeKey)}" class="loc-map-link">📍map</a>`;
    } else {
      // Try to resolve sender node location from nodes list
      const senderKey = decoded.pubKey || decoded.srcPubKey;
@@ -2861,7 +2864,7 @@
            locationNodeKey = nodeData.node.public_key;
            locationHtml = `${nodeData.node.lat.toFixed(5)}, ${nodeData.node.lon.toFixed(5)}`;
            if (nodeData.node.name) locationHtml = `${escapeHtml(nodeData.node.name)} — ${locationHtml}`;
-            locationHtml += ` <a href="#/map?node=${encodeURIComponent(locationNodeKey)}" style="font-size:0.85em">📍map</a>`;
+            locationHtml += ` <a href="#/map?node=${encodeURIComponent(locationNodeKey)}" class="loc-map-link">📍map</a>`;
          } else if (senderName && !senderKey) {
            // Search by name
            const searchData = await api(`/nodes/search?q=${encodeURIComponent(senderName)}`, { ttl: 30000 }).catch(() => null);
@@ -2870,7 +2873,7 @@
              locationNodeKey = match.public_key;
              locationHtml = `${match.lat.toFixed(5)}, ${match.lon.toFixed(5)}`;
              locationHtml = `${escapeHtml(match.name)} — ${locationHtml}`;
-              locationHtml += ` <a href="#/map?node=${encodeURIComponent(locationNodeKey)}" style="font-size:0.85em">📍map</a>`;
+              locationHtml += ` <a href="#/map?node=${encodeURIComponent(locationNodeKey)}" class="loc-map-link">📍map</a>`;
            }
          }
        } catch {}
@@ -2896,7 +2899,7 @@
      ${messageHtml}
      <dl class="detail-meta">
        <dt>Observer</dt><dd>${obsNameOnly(effectivePkt.observer_id)}${obsIataBadge(effectivePkt)}</dd>
-        <dt>Location</dt><dd>${locationHtml}</dd>
+        ${locationHtml ? `<dt>Location</dt><dd>${locationHtml}</dd>` : ''}
        <dt>SNR / RSSI</dt><dd>${snr != null ? snr + ' dB' : '—'} / ${rssi != null ? rssi + ' dBm' : '—'}</dd>
        <dt>Route Type</dt><dd>${routeTypeName(pkt.route_type)}</dd>
        <dt>Payload Type</dt><dd><span class="badge badge-${payloadTypeColor(pkt.payload_type)}">${typeName}</span></dd>
@@ -917,6 +917,10 @@ body.scroll-locked { overflow: hidden; }
 }
 .detail-meta dt { color: var(--text-muted); font-size: 11px; text-transform: uppercase; letter-spacing: .3px; }
 .detail-meta dd { font-weight: 500; margin-bottom: 4px; }
+/* #1281: 📍map link inside detail-meta — UA-default blue is unreadable on
+ * dark backgrounds. Force theme-aware color via --accent. */
+.loc-map-link { color: var(--accent); font-size: 0.85em; text-decoration: none; }
+.loc-map-link:hover { text-decoration: underline; }
 .observation-current { background: var(--accent-bg, rgba(0,122,255,0.1)); font-weight: 600; }
 .detail-obs-row:hover { background: var(--hover-bg, rgba(255,255,255,0.05)); }
 .detail-obs-table th { font-size: 0.8em; text-transform: uppercase; color: var(--text-muted); }
@@ -0,0 +1,155 @@
+/**
+ * #1281 — Packet detail Location row + 📍map link contrast.
+ *
+ * Bug:
+ *   A) <dt>Location</dt><dd>—</dd> renders unconditionally on every packet,
+ *      wasting a row on ~90% of packet types (only ADVERT carries unencrypted
+ *      transmitter GPS).
+ *   B) The trailing `📍map` link has no class/color → inherits UA-default <a>
+ *      blue → unreadable in dark mode.
+ *
+ * Asserts:
+ *   1. Some non-ADVERT packet detail does NOT contain <dt>Location</dt>.
+ *   2. Some ADVERT packet detail DOES contain <dt>Location</dt> with coords.
+ *   3. The 📍map link uses class="loc-map-link" with color = --accent
+ *      (NOT the default UA blue rgb(0,0,238)).
+ *
+ * Usage: BASE_URL=http://localhost:13581 node test-issue-1281-location-row-e2e.js
+ */
+'use strict';
+const { chromium } = require('playwright');
+
+const BASE = process.env.BASE_URL || 'http://localhost:13581';
+
+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'); }
+
+function normRgb(s) {
+  const m = s && s.match(/rgba?\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)/);
+  if (!m) return null;
+  return `rgb(${m[1]}, ${m[2]}, ${m[3]})`;
+}
+
+async function gotoPackets(page) {
+  await page.goto(`${BASE}/#/packets`, { waitUntil: 'domcontentloaded' });
+  await page.evaluate(() => {
+    localStorage.removeItem('meshcore-groupbyhash');
+    localStorage.setItem('meshcore-time-window', '525600');
+  });
+  await page.reload({ waitUntil: 'load' });
+  await page.waitForSelector('table tbody tr[data-hash]', { timeout: 15000 });
+}
+
+// Click rows until detail pane's Payload Type matches `wantType` (e.g. "Advert"
+// or any non-"Advert"). Returns true on hit, false if exhausted.
+async function findPacketDetailByType(page, predicate, maxRows = 40) {
+  await page.waitForTimeout(400);
+  const rows = await page.$$('table tbody tr[data-hash][data-action]');
+  for (let i = 0; i < Math.min(rows.length, maxRows); i++) {
+    await rows[i].click({ timeout: 3000 }).catch(() => null);
+    await page.waitForTimeout(350);
+    const meta = await page.evaluate(() => {
+      const dts = document.querySelectorAll('dl.detail-meta dt');
+      let typeName = null;
+      let hasLocation = false;
+      let locationText = '';
+      for (const dt of dts) {
+        const label = dt.textContent.trim();
+        const dd = dt.nextElementSibling;
+        if (label === 'Payload Type') typeName = dd ? dd.textContent.trim() : null;
+        if (label === 'Location') { hasLocation = true; locationText = dd ? dd.textContent.trim() : ''; }
+      }
+      return { typeName, hasLocation, locationText };
+    });
+    if (predicate(meta)) return meta;
+  }
+  return null;
+}
+
+(async () => {
+  const browser = await chromium.launch({
+    headless: true,
+    executablePath: process.env.CHROMIUM_PATH || '/usr/bin/chromium',
+    args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'],
+  });
+  const ctx = await browser.newContext({ viewport: { width: 1280, height: 900 } });
+  const page = await ctx.newPage();
+  page.setDefaultTimeout(15000);
+  page.on('pageerror', (e) => console.error('[pageerror]', e.message));
+
+  console.log(`\n=== #1281 Location row + map link contrast E2E against ${BASE} ===`);
+
+  await step('Non-ADVERT packet detail does NOT render <dt>Location</dt>', async () => {
+    await gotoPackets(page);
+    // Filter to a non-ADVERT type to make the search efficient.
+    const meta = await findPacketDetailByType(
+      page,
+      (m) => m.typeName && m.typeName !== 'Advert',
+      40
+    );
+    assert(meta, 'No non-ADVERT packet found in first 40 rows');
+    assert(!meta.hasLocation,
+      `Expected NO <dt>Location</dt> for type "${meta.typeName}", but found one with text "${meta.locationText}"`);
+  });
+
+  await step('ADVERT packet detail STILL renders <dt>Location</dt> with GPS coords', async () => {
+    await gotoPackets(page);
+    // Filter UI to ADVERTs to guarantee we find one.
+    const fInput = await page.$('#packetFilterInput');
+    if (fInput) {
+      await fInput.fill('type == ADVERT');
+      await page.keyboard.press('Enter');
+      await page.waitForTimeout(600);
+    }
+    const meta = await findPacketDetailByType(
+      page,
+      (m) => m.typeName === 'Advert' && m.hasLocation,
+      40
+    );
+    assert(meta, 'No ADVERT packet with Location row found in first 40 ADVERT rows');
+    assert(/-?\d+\.\d+\s*,\s*-?\d+\.\d+/.test(meta.locationText),
+      `ADVERT Location should contain GPS coords, got: "${meta.locationText}"`);
+  });
+
+  await step('📍map link uses class="loc-map-link" with color = var(--accent)', async () => {
+    // Reuse the ADVERT detail pane left open from the previous step.
+    const result = await page.evaluate(() => {
+      const link = document.querySelector('dl.detail-meta a.loc-map-link');
+      if (!link) return { missing: true };
+      const cs = getComputedStyle(link);
+      const accentRaw = getComputedStyle(document.documentElement).getPropertyValue('--accent').trim();
+      // Resolve --accent value to its computed rgb() via a probe element.
+      const probe = document.createElement('span');
+      probe.style.color = `var(--accent)`;
+      document.body.appendChild(probe);
+      const accentRgb = getComputedStyle(probe).color;
+      probe.remove();
+      return {
+        linkColor: cs.color,
+        accentRgb,
+        accentRaw,
+        href: link.getAttribute('href'),
+        text: link.textContent.trim(),
+      };
+    });
+    assert(!result.missing,
+      '<a class="loc-map-link"> not found in detail pane — implementation must apply the class');
+    const link = normRgb(result.linkColor);
+    const accent = normRgb(result.accentRgb);
+    console.log(`    link.color=${result.linkColor}  --accent→${result.accentRgb} (raw "${result.accentRaw}")`);
+    assert(link === accent,
+      `📍map link color ${result.linkColor} must equal --accent (${result.accentRgb}); ` +
+      `default UA blue (rgb(0, 0, 238)) is not acceptable`);
+    assert(link !== 'rgb(0, 0, 238)',
+      'Link color is UA-default blue — class is missing or CSS rule does not match');
+  });
+
+  await browser.close();
+
+  console.log(`\n${passed} passed, ${failed} failed`);
+  process.exit(failed === 0 ? 0 : 1);
+})().catch((e) => { console.error(e); process.exit(1); });