From 4cd51a41e7729926c2835e60b81c8e55f4fef8cb Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Tue, 5 May 2026 11:19:10 -0700 Subject: [PATCH] =?UTF-8?q?fix(channels):=20strip=20Share=20modal=20?= =?UTF-8?q?=E2=80=94=20remove=20redundant=20URL=20copy=20+=20duplicated=20?= =?UTF-8?q?key=20field=20(#1101)=20(#1103)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Strips the Share Channel modal (shipped in #1090) down to its essentials. Removes redundant affordances that the QR already provides. ## What changed **Removed from the Share modal:** - The URL text printed inside the QR box (the QR encodes the URL) - The inline Copy Key button inside the QR box (overlapped the image) - The `meshcore://` URL input field below the QR - The Copy URL button next to the URL field **Result — the modal now contains exactly:** - Title `Share: ` - QR code (just the QR ``, nothing else in that box) - Hex Key field with a single Copy button BELOW the QR - Privacy warning - ✕ close button (top right) ## Implementation - `public/channels.js` — drop the `meshcore://` URL field-group from share modal markup; `openShareModal()` no longer looks up `#chShareUrl` or builds a URL into a field; pass `{ qrOnly: true }` when calling `ChannelQR.generate` so the QR box renders ONLY the QR image. - `public/channel-qr.js` — `generate(name, secret, target, opts)` now accepts `opts.qrOnly` which short-circuits before appending the inline URL line + Copy Key button. Default behaviour (no opts) unchanged, so the Add-Channel "Generate & Show QR" flow is untouched. ## Tests (TDD: red → green) - New: `test-channel-issue-1101.js` (static grep) — asserts the URL field is gone from markup, `openShareModal` no longer references it, and `ChannelQR.generate` honours `qrOnly`. - Updated: `test-channel-issue-1087.js` and `test-channel-issue-1087-e2e.js` — those previously asserted the URL field's presence (which is exactly what #1101 removes); they now assert ONLY the hex key field exists, AND that `#chShareQr` contains exactly one `` and no `.channel-qr-url` / `.channel-qr-copy` children. - Wired into `.github/workflows/deploy.yml` `node-test` job. Commit history shows red (test commit `c0c254a`) → green (fix commit `6315a19`) per AGENTS.md TDD requirement. E2E assertion added: test-channel-issue-1087-e2e.js:184 ## Acceptance criteria - [x] Share modal contains only: QR, "Copy Key" button, privacy warning - [x] No "Copy URL" affordance anywhere in the modal - [x] No duplicated hex key field below - [x] E2E test asserts the absence of the removed elements Fixes #1101 --------- Co-authored-by: meshcore-bot Co-authored-by: clawbot --- .github/workflows/deploy.yml | 1 + public/channel-qr.js | 10 +++- public/channels.js | 25 ++++----- test-channel-issue-1087-e2e.js | 27 ++++++++-- test-channel-issue-1087.js | 4 +- test-channel-issue-1101.js | 94 ++++++++++++++++++++++++++++++++++ 6 files changed, 139 insertions(+), 22 deletions(-) create mode 100644 test-channel-issue-1101.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index e995147e..a9df34eb 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -90,6 +90,7 @@ jobs: node test-channel-qr-wiring.js node test-channel-modal-ux.js node test-channel-issue-1087.js + node test-channel-issue-1101.js node test-pull-to-reconnect-1091.js node test-channel-fluid-layout.js diff --git a/public/channel-qr.js b/public/channel-qr.js index 671f176c..b4da84be 100644 --- a/public/channel-qr.js +++ b/public/channel-qr.js @@ -75,9 +75,11 @@ * every Generate click fall through to "[QR library not loaded]". * (Issue #1087 bug 1.) */ - function generate(name, secretHex, target) { + function generate(name, secretHex, target, opts) { if (!_hasDom() || !target) return; target.innerHTML = ''; + opts = opts || {}; + var qrOnly = !!opts.qrOnly; const url = buildUrl(name, secretHex); @@ -113,6 +115,12 @@ qrBox.textContent = '[QR library not loaded]'; } + // #1101: in qrOnly mode (Share modal), the host renders the hex + // key field + Copy button BELOW the QR. Skip the inline URL line + // and inline Copy Key button here so the QR box contains JUST the + // QR image — no overlap, no redundant affordances. + if (qrOnly) return; + const urlLine = document.createElement('div'); urlLine.className = 'channel-qr-url'; urlLine.style.cssText = 'font-family:monospace;font-size:11px;word-break:break-all;margin-top:6px;'; diff --git a/public/channels.js b/public/channels.js index d98cd7cc..e5b7fd7b 100644 --- a/public/channels.js +++ b/public/channels.js @@ -759,13 +759,6 @@ -
- -
- - -
-
⚠ Privacy: only share with trusted people. Anyone with this key can read all messages on this channel.
@@ -882,19 +875,19 @@ if (title) title.textContent = 'Share: ' + safeName; var qrHolder = document.getElementById('chShareQr'); var keyField = document.getElementById('chShareKey'); - var urlField = document.getElementById('chShareUrl'); var fieldsWrap = shareModalEl.querySelectorAll('.ch-share-field-group'); for (var i = 0; i < fieldsWrap.length; i++) fieldsWrap[i].hidden = false; - var url = 'meshcore://channel/add?name=' + encodeURIComponent(safeName) + - '&secret=' + keyHex; if (keyField) keyField.value = keyHex; - if (urlField) urlField.value = url; if (qrHolder) { qrHolder.innerHTML = ''; if (window.ChannelQR && typeof window.ChannelQR.generate === 'function') { // #1087 Bug 2: pass the user-facing displayName, NOT the // internal `psk:` channelName lookup key. - window.ChannelQR.generate(safeName, keyHex, qrHolder); + // #1101: qrOnly=true — render JUST the QR image. The Share + // modal has its own dedicated hex key field + Copy button + // BELOW the QR; an inline URL line + Copy Key button inside + // the QR box was redundant and visually overlapping. + window.ChannelQR.generate(safeName, keyHex, qrHolder, { qrOnly: true }); } } shareModalEl.classList.remove('hidden'); @@ -946,10 +939,10 @@ var copyBtn = e.target.closest && e.target.closest('[data-share-copy]'); if (copyBtn) { e.preventDefault(); - var which = copyBtn.getAttribute('data-share-copy'); - var src = which === 'url' - ? document.getElementById('chShareUrl') - : document.getElementById('chShareKey'); + // #1101: only the hex key is copyable from the share modal; + // the URL field was removed, so the data-share-copy attribute + // is informational only — the source is always #chShareKey. + var src = document.getElementById('chShareKey'); if (src) { try { src.select(); } catch (e2) {} var doneCopy = function () { diff --git a/test-channel-issue-1087-e2e.js b/test-channel-issue-1087-e2e.js index 4d7ba759..69601c42 100644 --- a/test-channel-issue-1087-e2e.js +++ b/test-channel-issue-1087-e2e.js @@ -160,15 +160,36 @@ function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } assert(/share/i.test(shareTitle), 'Bug 4: share modal title must contain "Share", got: ' + shareTitle); - // Hex key + URL fields must be present and copyable. + // Hex key field must be present and copyable. (#1101: URL field + // removed — QR already encodes the URL, a separate Copy URL button + // was redundant.) const hasFields = await page.evaluate(() => { const m = document.getElementById('chShareModal'); if (!m) return false; const k = m.querySelector('#chShareKey, [data-share-field="key"]'); const u = m.querySelector('#chShareUrl, [data-share-field="url"]'); - return !!(k && u); + return !!k && !u; }); - assert(hasFields, 'Bug 4: share modal must expose hex key + URL fields'); + assert(hasFields, 'Bug 4 / #1101: share modal exposes ONLY the hex key field (no URL field)'); + + // #1101: the QR box must contain ONLY the QR — no URL text + // line, no inline Copy Key button overlapping the image. + const qrBoxOnlyHasQr = await page.evaluate(() => { + const qr = document.getElementById('chShareQr'); + if (!qr) return { ok: false, reason: 'no #chShareQr' }; + const imgs = qr.querySelectorAll('img'); + const urlLine = qr.querySelector('.channel-qr-url'); + const copyBtn = qr.querySelector('.channel-qr-copy, button'); + return { + ok: imgs.length === 1 && !urlLine && !copyBtn, + imgCount: imgs.length, + hasUrlLine: !!urlLine, + hasCopyBtn: !!copyBtn, + }; + }); + assert(qrBoxOnlyHasQr.ok, + '#1101: #chShareQr contains ONLY the QR image (got ' + + JSON.stringify(qrBoxOnlyHasQr) + ')'); }); console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); diff --git a/test-channel-issue-1087.js b/test-channel-issue-1087.js index 3e233a2c..ca3ace34 100644 --- a/test-channel-issue-1087.js +++ b/test-channel-issue-1087.js @@ -120,8 +120,8 @@ assert(/id="chShareModalTitle"|class="ch-share-modal-title"|>Share[^<]*")'); assert(/id="chShareKey"|data-share-field="key"/.test(shareModalBlock), 'share modal exposes the hex key field with a copy affordance'); -assert(/id="chShareUrl"|data-share-field="url"/.test(shareModalBlock), - 'share modal exposes the meshcore:// URL field with a copy affordance'); +// #1101: meshcore:// URL field intentionally REMOVED — QR already +// encodes the URL, separate field/button was redundant. assert(/trusted|privacy|do not share|only share/i.test(shareModalBlock), 'share modal includes a privacy warning'); diff --git a/test-channel-issue-1101.js b/test-channel-issue-1101.js new file mode 100644 index 00000000..d6c178ea --- /dev/null +++ b/test-channel-issue-1101.js @@ -0,0 +1,94 @@ +/** + * #1101 — Strip Share modal: remove redundant URL copy + duplicated key field. + * + * Acceptance criteria: + * - Share modal contains only: QR (just the QR image, nothing else + * in that box), Hex Key field with single Copy button BELOW the QR, + * privacy warning, Close ✕ button. + * - No "Copy URL" affordance ANYWHERE in the modal. + * - No duplicated meshcore:// URL field below the QR. + * - The QR box (#chShareQr) must contain ONLY the QR image — no URL + * text, no Copy Key button overlapping it. + */ +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +let passed = 0, failed = 0; +function assert(cond, msg) { + if (cond) { passed++; console.log(' ✓ ' + msg); } + else { failed++; console.error(' ✗ ' + msg); } +} + +const channelsSrc = fs.readFileSync(path.join(__dirname, 'public', 'channels.js'), 'utf8'); +const qrSrc = fs.readFileSync(path.join(__dirname, 'public', 'channel-qr.js'), 'utf8'); + +console.log('\n=== #1101: Share modal markup ==='); + +// Locate the share modal markup block. +const shareModalIdx = channelsSrc.indexOf('id="chShareModal"'); +assert(shareModalIdx > 0, 'share modal markup located'); +// Tighten block isolation: scan forward for the share modal's own +// closing tag (the outer overlay div is indented 6 spaces, so its +// matching close is the first "\n " we hit after the +// opener). Falls back to the old ch-main heuristic if that pattern +// disappears for any reason. +let shareEnd = channelsSrc.indexOf('\n ', shareModalIdx); +if (shareEnd < 0) { + shareEnd = channelsSrc.indexOf('
0 && shareModalBlock.length < 4000, + 'share modal block isolated'); + +// Hex key field MUST still be present (single source of truth). +assert(/id="chShareKey"/.test(shareModalBlock), + 'share modal still exposes the hex key field with a Copy button'); + +// meshcore:// URL field MUST be removed. +assert(!/id="chShareUrl"/.test(shareModalBlock), + 'share modal does NOT render a #chShareUrl input field'); +assert(!/data-share-field="url"/.test(shareModalBlock), + 'share modal does NOT render any [data-share-field="url"] element'); +assert(!/data-share-copy="url"/.test(shareModalBlock), + 'share modal does NOT render any [data-share-copy="url"] button'); +assert(!/meshcore:\/\/ URL/.test(shareModalBlock), + 'share modal does NOT show a "meshcore:// URL" label'); + +// Privacy warning + close button still required. +assert(/ch-modal-warn/.test(shareModalBlock), + 'share modal still includes the privacy warning'); +assert(/id="chShareModalClose"/.test(shareModalBlock), + 'share modal still has the ✕ close button'); + +console.log('\n=== #1101: openShareModal() body ==='); + +// openShareModal must no longer reference chShareUrl or build URL into a field. +const openIdx = channelsSrc.indexOf('function openShareModal('); +assert(openIdx > 0, 'openShareModal located'); +const openEnd = channelsSrc.indexOf('function ', openIdx + 30); +const openBlock = channelsSrc.substring(openIdx, openEnd); +assert(!/getElementById\('chShareUrl'\)/.test(openBlock), + 'openShareModal does NOT look up #chShareUrl'); +assert(!/urlField\.value\s*=/.test(openBlock), + 'openShareModal does NOT assign to urlField.value'); + +console.log('\n=== #1101: ChannelQR.generate() supports qrOnly ==='); + +// ChannelQR.generate must accept an opts.qrOnly flag so the Share +// modal's QR box can render JUST the QR image — no URL line, no +// inline Copy Key button. (The Share modal has its own dedicated +// hex key field + Copy button BELOW the QR.) +assert(/function generate\([^)]*opts[^)]*\)/.test(qrSrc), + 'ChannelQR.generate accepts an opts argument'); +assert(/qrOnly/.test(qrSrc), + 'ChannelQR.generate honours opts.qrOnly'); + +// Share modal call site must pass qrOnly:true. +assert(/ChannelQR\.generate\([^)]*qrOnly[^)]*\)/.test(channelsSrc) || + /ChannelQR\.generate\([\s\S]{0,200}qrOnly\s*:\s*true/.test(channelsSrc), + 'openShareModal passes { qrOnly: true } to ChannelQR.generate'); + +console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); +process.exit(failed > 0 ? 1 : 0);