mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-05-11 14:24:43 +00:00
fix(channels): hide raw psk:* in header, label share button, red delete button (#1041)
## 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:<hex8>` 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 <hash>`. 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 <bot@openclaw.dev> Co-authored-by: corescope-bot <bot@corescope.local>
This commit is contained in:
Kpa-clawbot
+33
-4
@@ -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:<hex8>" key prefix shouldn't be shown to users. Falls back to
|
||||
// userLabel, then a friendly placeholder, then a caller-supplied
|
||||
// fallback, then `Channel <hash>`.
|
||||
//
|
||||
// `fallback` lets row rendering preserve its existing "Unknown" /
|
||||
// "Channel <hash>" 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 ? ' <span class="ch-user-badge" title="You added this key" aria-label="Your key">🔑</span>' : '';
|
||||
@@ -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:<hex>" 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`;
|
||||
|
||||
|
||||
+12
-4
@@ -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; }
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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:<hex8>" 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 <hash>" when name missing');
|
||||
assert(sandbox.channelDisplayName({ hash: 'abc', name: '' }, 'Unknown') === 'Unknown',
|
||||
'caller-supplied fallback overrides "Channel <hash>" 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);
|
||||
Reference in New Issue
Block a user