From 67da696a42abeef3952d50a89c44995c7c04887e Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Mon, 4 May 2026 20:56:01 -0700 Subject: [PATCH] fix(channels): hide raw psk:* in header, label share button, red delete button (#1041) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Channel UX round 2 (follow-up to #1040) Three UX issues reported after #1040 landed: ### 1. Header shows raw `psk:372a9c93` for PSK channels The selected-channel title rendered `ch.name` directly, which for user-added PSK channels is the synthetic `psk:` string. Users see opaque key fragments where they expected the friendly name they typed. **Fix:** new `channelDisplayName(ch)` helper. Returns `ch.userLabel` when set, falls back to `"Private Channel"` for any `psk:*` name, then to the original name, then to `Channel `. Used in both `selectChannel` (header) and `renderChannelRow` (sidebar). ### 2. Share button `โคด` is unrecognizable Up-arrow glyph carried no meaning โ€” users didn't know it opened the QR/key reshare modal. **Fix:** swap `โคด` for `๐Ÿ“ค Share` text label. Same hook, same handler. ### 3. โœ• delete button is a subtle span, not a destructive button Looked like decorative text, not a real action. **Fix:** `.ch-remove-btn` gets `background: var(--statusRed, #b54a4a)`, `color: white`, `border-radius: 4px`, `padding: 4px 8px`, `font-weight: bold`. Now reads as a destructive action. ### TDD - Red commit `2d05bbf`: 9 failing assertions (helper missing, โคด still present, CSS rules absent), test compiles + runs to assertion failure. - Green commit `938f3fc`: all 12 assertions pass. Existing `test-channel-ux-followup.js` still 28/28. ### Files - `public/channels.js` โ€” `channelDisplayName` helper, header + row rendering, share button label - `public/style.css` โ€” `.ch-remove-btn` destructive styling - `test-channel-ux-round2.js` โ€” new test (helper behavior + source/CSS assertions) --------- Co-authored-by: openclaw-bot Co-authored-by: corescope-bot --- public/channels.js | 37 ++++++++++++++-- public/style.css | 16 +++++-- test-channel-ux-followup.js | 7 +++ test-channel-ux-round2.js | 87 +++++++++++++++++++++++++++++++++++++ 4 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 test-channel-ux-round2.js diff --git a/public/channels.js b/public/channels.js index 51e27d6a..73769a68 100644 --- a/public/channels.js +++ b/public/channels.js @@ -1322,12 +1322,39 @@ } } + // #1041: single source of truth for the user-facing placeholder shown + // when a PSK channel has no user-supplied label. Hoisted so the helper + // and any future call sites stay in sync (i18n / branding-friendly). + const PRIVATE_CHANNEL_LABEL = 'Private Channel'; + + // Display name for a channel โ€” handles PSK channels where the raw + // "psk:" key prefix shouldn't be shown to users. Falls back to + // userLabel, then a friendly placeholder, then a caller-supplied + // fallback, then `Channel `. + // + // `fallback` lets row rendering preserve its existing "Unknown" / + // "Channel " semantics for encrypted-but-not-user-added channels + // without duplicating the psk:* check. + function channelDisplayName(ch, fallback) { + if (!ch) return ''; + const name = ch.name || ''; + if (ch.userLabel) return ch.userLabel; + if (name.indexOf('psk:') === 0) return PRIVATE_CHANNEL_LABEL; + if (name) return name; + if (fallback) return fallback; + return 'Channel ' + (typeof formatHashHex === 'function' ? formatHashHex(ch.hash) : ch.hash); + } + // #1034 PR1: render a single channel row (used by all sidebar sections). function renderChannelRow(ch) { const isEncrypted = ch.encrypted === true; const isUserAdded = ch.userAdded === true; - const baseName = isEncrypted ? (ch.name || 'Unknown') : (ch.name || `Channel ${formatHashHex(ch.hash)}`); - const name = (isUserAdded && ch.userLabel) ? ch.userLabel : baseName; + // #1041: route through channelDisplayName so the psk:* โ†’ "Private + // Channel" rule lives in one place. Pass an `encryptedFallback` so + // rows for non-user-added encrypted channels keep showing "Unknown" + // (their existing behavior) when there's no name at all. + const encryptedFallback = isEncrypted ? 'Unknown' : ''; + const name = channelDisplayName(ch, encryptedFallback); const color = isEncrypted && !isUserAdded ? 'var(--text-muted, #6b7280)' : getChannelColor(ch.hash); const time = ch.lastActivityMs ? formatSecondsAgo(Math.floor((Date.now() - ch.lastActivityMs) / 1000)) : ''; // Preview: show last sender+message when we have one. Otherwise show @@ -1372,7 +1399,7 @@ 'Remove channel and clear saved key', 'Remove', '') : ''; const shareBtn = isUserAdded - ? iconBtn('ch-share-btn', 'data-share-channel', ch.hash, name, 'โคด', + ? iconBtn('ch-share-btn', 'data-share-channel', ch.hash, name, '๐Ÿ“ค Share', 'Share channel key (QR + URL)', 'Share', ' aria-haspopup="dialog"') : ''; const userBadge = isUserAdded ? ' ๐Ÿ”‘' : ''; @@ -1457,7 +1484,9 @@ history.replaceState(null, '', `#/channels/${encodeURIComponent(hash)}`); renderChannelList(); const ch = channels.find(c => c.hash === hash); - const name = ch?.name || `Channel ${formatHashHex(hash)}`; + // #1041: never show raw "psk:" prefixes in the header โ€” use the + // user-supplied label or "Private Channel". + const name = ch ? channelDisplayName(ch) : `Channel ${formatHashHex(hash)}`; const header = document.getElementById('chHeader'); header.querySelector('.ch-header-text').textContent = `${name} โ€” ${ch?.messageCount || 0} messages`; diff --git a/public/style.css b/public/style.css index f1064060..a3e3673c 100644 --- a/public/style.css +++ b/public/style.css @@ -547,11 +547,19 @@ button.ch-item.selected { background: var(--selected-bg); } opacity: 0.55; transition: opacity 0.15s, color 0.15s; line-height: 1; user-select: none; } -.ch-remove-btn { font-size: 16px; } -.ch-share-btn { font-size: 14px; } -button.ch-item:hover .ch-icon-btn { opacity: 0.9; } +.ch-remove-btn { + font-size: 14px; + background: var(--statusRed, #b54a4a); + color: white; + border-radius: 4px; + padding: 4px 8px; + font-weight: bold; + opacity: 0.9; +} +.ch-share-btn { font-size: 13px; padding: 4px 8px; } +button.ch-item:hover .ch-icon-btn { opacity: 1; } .ch-icon-btn:hover, .ch-icon-btn:focus { opacity: 1 !important; outline: none; } -.ch-remove-btn:hover, .ch-remove-btn:focus { color: var(--danger, #dc2626); } +.ch-remove-btn:hover, .ch-remove-btn:focus { background: #8b3838; color: white; } .ch-share-btn:hover, .ch-share-btn:focus { color: var(--accent, #3b82f6); } .ch-user-badge { font-size: 12px; margin-left: 2px; opacity: 0.85; flex-shrink: 0; } .ch-item-preview { font-size: 12px; color: var(--text-muted); white-space: nowrap; overflow: hidden; text-overflow: ellipsis; } diff --git a/test-channel-ux-followup.js b/test-channel-ux-followup.js index 0a9e306e..cb058796 100644 --- a/test-channel-ux-followup.js +++ b/test-channel-ux-followup.js @@ -122,7 +122,14 @@ if (renderRowSrc) { window: {}, renderChannelRow: null, }; + // #1041 follow-up: renderChannelRow now delegates to channelDisplayName. + // Eval the REAL helper (and its module-local PRIVATE_CHANNEL_LABEL) + // into the sandbox so this test stays in sync with production behavior + // automatically โ€” no hand-rolled duplicate of the psk:* rule. + const helperSrc = extractFn(chSrc, 'function channelDisplayName(ch'); + assert(helperSrc, 'extracted channelDisplayName source for behavior sandbox'); vm.createContext(sandbox); + vm.runInContext('const PRIVATE_CHANNEL_LABEL = "Private Channel";\n' + helperSrc, sandbox); vm.runInContext(renderRowSrc, sandbox); const userRow = sandbox.renderChannelRow({ hash: 'user:Crew', diff --git a/test-channel-ux-round2.js b/test-channel-ux-round2.js new file mode 100644 index 00000000..4d223b6f --- /dev/null +++ b/test-channel-ux-round2.js @@ -0,0 +1,87 @@ +/** + * Follow-up UX round 2 to channels (post #1040): + * + * 1. Channel header (selected-channel title) must NOT display the raw + * "psk:" key prefix. Use the user-supplied label when present, + * otherwise fall back to "Private Channel". + * 2. Sidebar share button uses a recognizable label ("๐Ÿ“ค Share" or + * similar), not the bare โคด glyph. + * 3. โœ• remove button has a red background, white text, proper button + * styling โ€” looks like a destructive action. + */ +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +const chSrc = fs.readFileSync(path.join(__dirname, 'public/channels.js'), 'utf8'); +const cssSrc = fs.readFileSync(path.join(__dirname, 'public/style.css'), 'utf8'); + +let passed = 0, failed = 0; +function assert(cond, msg) { + if (cond) { passed++; console.log(' โœ“ ' + msg); } + else { failed++; console.error(' โœ— ' + msg); } +} + +console.log('\n=== Fix 1: header display name for PSK channels ==='); +// Behavior test: extract channelDisplayName helper and exercise it. +const vm = require('vm'); +function extractFn(src, header) { + const start = src.indexOf(header); + if (start < 0) return null; + let depth = 0, i = src.indexOf('{', start); + if (i < 0) return null; + for (let j = i; j < src.length; j++) { + const c = src[j]; + if (c === '{') depth++; + else if (c === '}') { depth--; if (depth === 0) return src.substring(start, j + 1); } + } + return null; +} +const helperSrc = extractFn(chSrc, 'function channelDisplayName(ch'); +assert(helperSrc, 'channelDisplayName helper exists'); +if (helperSrc) { + const sandbox = { formatHashHex: h => h, PRIVATE_CHANNEL_LABEL: 'Private Channel' }; + vm.createContext(sandbox); + vm.runInContext('const PRIVATE_CHANNEL_LABEL = "Private Channel";\n' + helperSrc, sandbox); + assert(sandbox.channelDisplayName({ name: 'psk:372a9c93', userLabel: 'My Crew' }) === 'My Crew', + 'psk:* with userLabel returns the userLabel'); + assert(sandbox.channelDisplayName({ name: 'psk:372a9c93' }) === 'Private Channel', + 'psk:* without label returns "Private Channel"'); + assert(sandbox.channelDisplayName({ name: '#meshcore' }) === '#meshcore', + 'non-PSK names pass through unchanged'); + assert(sandbox.channelDisplayName({ hash: 'abc', name: '' }) === 'Channel abc', + 'falls back to "Channel " when name missing'); + assert(sandbox.channelDisplayName({ hash: 'abc', name: '' }, 'Unknown') === 'Unknown', + 'caller-supplied fallback overrides "Channel " default'); + assert(sandbox.channelDisplayName({ name: 'psk:abc' }, 'Unknown') === 'Private Channel', + 'fallback does NOT override the psk:* โ†’ "Private Channel" rule'); +} +// Source-level: header rendering must call channelDisplayName, not raw ch.name. +assert(/channelDisplayName\(ch\)/.test(chSrc), + 'selectChannel header rendering uses channelDisplayName(ch)'); + +console.log('\n=== Fix 2: share button has recognizable label ==='); +assert(!/'โคด'/.test(chSrc) && !/"โคด"/.test(chSrc), + 'bare โคด glyph no longer used as the share button content'); +// Tighten: assert the literal '๐Ÿ“ค Share' string is the glyph argument +// passed into the iconBtn(...) call for ch-share-btn โ€” this catches the +// case where someone removes the icon from the button content but leaves +// "Share" in an aria-label or title. +assert(/iconBtn\(\s*'ch-share-btn'[^)]*'๐Ÿ“ค Share'/.test(chSrc), + "iconBtn('ch-share-btn', ...) is called with '๐Ÿ“ค Share' as the glyph"); + +console.log('\n=== Fix 3: โœ• delete button is a visibly red destructive button ==='); +const removeRule = (cssSrc.match(/\.ch-remove-btn\s*\{[^}]*\}/) || [''])[0]; +assert(/background:\s*var\(--statusRed/.test(removeRule) || /background:\s*#b54a4a/.test(removeRule), + '.ch-remove-btn has red background (var(--statusRed,...) or #b54a4a)'); +assert(/color:\s*white/.test(removeRule) || /color:\s*#fff/.test(removeRule), + '.ch-remove-btn has white text'); +assert(/border-radius:/.test(removeRule), + '.ch-remove-btn has border-radius (button shape)'); +assert(/font-weight:\s*bold|font-weight:\s*700/.test(removeRule), + '.ch-remove-btn has bold font-weight'); + +console.log('\n=== Results ==='); +console.log('Passed: ' + passed + ', Failed: ' + failed); +process.exit(failed > 0 ? 1 : 0);