From e2d320449b6baa91316aacaf617fe9f07f73fa5b Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Mon, 18 May 2026 23:37:04 -0700 Subject: [PATCH] fix(#1281): hide empty Location row + theme map link via --accent (#1284) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 `
Location
β€”
` for nothing. We now skip the `
/
` 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 `` 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 `
Location
` - ADVERT detail STILL contains `
Location
` 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 `
/
` 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
Location
βœ“ ADVERT packet detail STILL renders
Location
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 --- .github/workflows/deploy.yml | 1 + public/packets.js | 15 +-- public/style.css | 4 + test-issue-1281-location-row-e2e.js | 155 ++++++++++++++++++++++++++++ 4 files changed, 169 insertions(+), 6 deletions(-) create mode 100644 test-issue-1281-location-row-e2e.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 7e672815..b82491b3 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -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 diff --git a/public/packets.js b/public/packets.js index b222e290..5ad0c636 100644 --- a/public/packets.js +++ b/public/packets.js @@ -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 += ` πŸ“map`; + if (locationNodeKey) locationHtml += ` πŸ“map`; } 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 += ` πŸ“map`; + locationHtml += ` πŸ“map`; } 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 += ` πŸ“map`; + locationHtml += ` πŸ“map`; } } } catch {} @@ -2896,7 +2899,7 @@ ${messageHtml}
Observer
${obsNameOnly(effectivePkt.observer_id)}${obsIataBadge(effectivePkt)}
-
Location
${locationHtml}
+ ${locationHtml ? `
Location
${locationHtml}
` : ''}
SNR / RSSI
${snr != null ? snr + ' dB' : 'β€”'} / ${rssi != null ? rssi + ' dBm' : 'β€”'}
Route Type
${routeTypeName(pkt.route_type)}
Payload Type
${typeName}
diff --git a/public/style.css b/public/style.css index 7b356b23..8bc92257 100644 --- a/public/style.css +++ b/public/style.css @@ -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); } diff --git a/test-issue-1281-location-row-e2e.js b/test-issue-1281-location-row-e2e.js new file mode 100644 index 00000000..006d13aa --- /dev/null +++ b/test-issue-1281-location-row-e2e.js @@ -0,0 +1,155 @@ +/** + * #1281 β€” Packet detail Location row + πŸ“map link contrast. + * + * Bug: + * A)
Location
β€”
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 + * blue β†’ unreadable in dark mode. + * + * Asserts: + * 1. Some non-ADVERT packet detail does NOT contain
Location
. + * 2. Some ADVERT packet detail DOES contain
Location
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
Location
', 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
Location
for type "${meta.typeName}", but found one with text "${meta.locationText}"`); + }); + + await step('ADVERT packet detail STILL renders
Location
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, + ' 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); });