From 6966304c63644c1e3ad0b642e2ac56da95a4cd32 Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Sat, 5 Sep 2026 19:11:02 -0400 Subject: [PATCH 1/5] feat(0x67):#67:copy any cell on tap, open on hold, toast for clipboard life --- pages/0x67/page.css | 44 ++++++++++++ pages/0x67/page.html | 2 + pages/0x67/page.ts | 130 ++++++++++++++++++++++++++-------- pages/tests/0x67-page.test.ts | 112 +++++++++++++++++++---------- 4 files changed, 221 insertions(+), 67 deletions(-) diff --git a/pages/0x67/page.css b/pages/0x67/page.css index 3fceb96..e3ac448 100644 --- a/pages/0x67/page.css +++ b/pages/0x67/page.css @@ -801,6 +801,33 @@ so an absolutely positioned menu would be cut off. */ cursor: pointer; } +/* A tap on a cell copies (#67), so the cell must not start a text selection or +wait on a double-tap-to-zoom before that tap registers. */ +.entry-table td { + touch-action: manipulation; + user-select: none; + -webkit-touch-callout: none; +} + +/* Present at rest, brighter on hover (#67): hover may raise emphasis, never be +what makes the affordance discoverable, since touch has none. */ +.copy-hint { + margin-left: 0.35rem; + opacity: 0.25; + font-size: 0.8em; +} + +.entry-table td:hover .copy-hint, +.entry-table td:focus-within .copy-hint { + opacity: 1; +} + +.entry-table-open { + width: 1px; + text-align: right; + white-space: nowrap; +} + .entry-table tbody tr:hover { background: var(--surface-2); } @@ -1464,3 +1491,20 @@ body.embedded footer { display: none; } } + +/* Visible for exactly as long as the clipboard holds the value (#67). */ +.toast { + position: fixed; + left: 50%; + bottom: 1.5rem; + transform: translateX(-50%); + z-index: 50; + max-width: min(90vw, 32rem); + padding: 0.5rem 0.9rem; + border: 1px solid var(--border); + border-radius: 8px; + background: var(--surface); + box-shadow: 0 8px 30px rgba(0, 0, 0, 0.2); + color: var(--text); + font-size: 0.85rem; +} diff --git a/pages/0x67/page.html b/pages/0x67/page.html index 4ac58e8..fa0d33f 100644 --- a/pages/0x67/page.html +++ b/pages/0x67/page.html @@ -360,6 +360,8 @@

Discard unsaved changes?

+ + diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index d42ca04..6b94b1d 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -117,13 +117,25 @@ logic.ts) are globals from bundle-iife's concatenation; see globals.d.ts. */ let clipboardTimer: ReturnType | null = null; -async function copyToClipboard(text: string): Promise { +/* Names the field, never the value (#67): the toast is a reminder that a secret +is on the clipboard, so it must not put that secret on screen as well. */ +function showClipboardToast(label: string): void { + const toast = byId('toast'); + toast.textContent = `${label} copied to clipboard`; + toast.hidden = false; +} + +async function copyToClipboard(text: string, label = 'Value'): Promise { try { await navigator.clipboard.writeText(text); if (clipboardTimer) clearTimeout(clipboardTimer); + showClipboardToast(label); + /* One timer drives both the wipe and the toast (#67), so the toast is gone + exactly when the clipboard is, rather than on a schedule of its own. */ clipboardTimer = setTimeout(() => { navigator.clipboard.writeText('').catch(() => {}); clipboardTimer = null; + byId('toast').hidden = true; }, app.clipboardTimeout * 1000); } catch (err) { console.error('Clipboard write failed', err); @@ -689,31 +701,79 @@ function entryColumnDisplayValue(entry: XmlElement, column: EntryColumnKey): str return raw; } -// Delays opening a row so a second click (a double-click) has time to cancel -// it via wireCopyOnDblClick, rather than racing ahead of the copy. -let entryRowOpenTimer: ReturnType | null = null; -const ENTRY_ROW_OPEN_DELAY_MS = 250; +const ENTRY_HOLD_MS = 500; +const ENTRY_HOLD_SLOP_PX = 8; -function openEntryDetailDelayed(entry: XmlElement): void { - if (entryRowOpenTimer) clearTimeout(entryRowOpenTimer); - entryRowOpenTimer = setTimeout(() => { - entryRowOpenTimer = null; - app.currentEntry = entry; - showEntryDetail(); - }, ENTRY_ROW_OPEN_DELAY_MS); +function openEntry(entry: XmlElement): void { + app.currentEntry = entry; + showEntryDetail(); } -function wireCopyOnDblClick(cell: HTMLTableCellElement, value: string): void { - cell.addEventListener('dblclick', (e) => { - e.stopPropagation(); - if (entryRowOpenTimer) { - clearTimeout(entryRowOpenTimer); - entryRowOpenTimer = null; - } - if (value) copyToClipboard(value); +/* A tap copies, a hold opens the card (#67). Hold duration separates the two at +release, so neither waits on the other the way click-versus-double-click did. +The slop check matters on touch, where scrolling the table starts on a cell. */ +function wireCellPress( + cell: HTMLTableCellElement, + entry: XmlElement, + value: string, + label: string, +): void { + let holdTimer: ReturnType | null = null; + let startX = 0; + let startY = 0; + + const cancelHold = (): void => { + if (holdTimer) clearTimeout(holdTimer); + holdTimer = null; + }; + + cell.addEventListener('pointerdown', (down) => { + startX = down.clientX; + startY = down.clientY; + holdTimer = setTimeout(() => { + holdTimer = null; + openEntry(entry); + }, ENTRY_HOLD_MS); + }); + + cell.addEventListener('pointermove', (move) => { + if (Math.hypot(move.clientX - startX, move.clientY - startY) > ENTRY_HOLD_SLOP_PX) cancelHold(); + }); + + cell.addEventListener('pointercancel', cancelHold); + + cell.addEventListener('pointerup', () => { + const tapped = holdTimer !== null; + cancelHold(); + if (tapped && value) copyToClipboard(value, label); }); } +/* Dim rather than hover-only (#67): hover may raise a control's emphasis, but a +touch user has no hover, so it must never be what makes it discoverable. */ +function copyHint(): HTMLSpanElement { + const hint = document.createElement('span'); + hint.className = 'copy-hint'; + hint.textContent = '📋'; + return hint; +} + +function buildEntryCell( + entry: XmlElement, + display: string, + value: string, + label: string, +): HTMLTableCellElement { + const td = document.createElement('td'); + td.appendChild(document.createTextNode(display)); + if (value) { + td.appendChild(copyHint()); + td.title = `Copy ${label.toLowerCase()}`; + } + wireCellPress(td, entry, value, label); + return td; +} + function buildEntryTable(rows: EntryWithGroup[]): HTMLTableElement { const table = document.createElement('table'); table.className = 'entry-table'; @@ -730,6 +790,7 @@ function buildEntryTable(rows: EntryWithGroup[]): HTMLTableElement { th.textContent = column.label; headRow.appendChild(th); } + headRow.appendChild(document.createElement('th')); thead.appendChild(headRow); table.appendChild(thead); @@ -737,21 +798,32 @@ function buildEntryTable(rows: EntryWithGroup[]): HTMLTableElement { for (const { entry } of rows) { const tr = document.createElement('tr'); - const titleTd = document.createElement('td'); + const titleTd = buildEntryCell( + entry, + `${iconEmoji(elementIconId(entry))} ${entryTitle(entry)}`, + entryTitle(entry), + 'Title', + ); titleTd.className = 'entry-table-title'; - titleTd.textContent = `${iconEmoji(elementIconId(entry))} ${entryTitle(entry)}`; - wireCopyOnDblClick(titleTd, entryTitle(entry)); tr.appendChild(titleTd); for (const column of visibleColumns) { - const td = document.createElement('td'); - td.textContent = entryColumnDisplayValue(entry, column.key); + const td = buildEntryCell( + entry, + entryColumnDisplayValue(entry, column.key), + entryColumnValue(entry, column.key), + column.label, + ); if (column.key === 'password') td.classList.add('entry-table-protected'); - wireCopyOnDblClick(td, entryColumnValue(entry, column.key)); tr.appendChild(td); } - tr.addEventListener('click', () => openEntryDetailDelayed(entry)); + // A visible way in, since a hold is not discoverable on its own (#67). + const openTd = document.createElement('td'); + openTd.className = 'entry-table-open'; + openTd.appendChild(makeIconButton('icon-btn', 'Open entry', '›', () => openEntry(entry))); + tr.appendChild(openTd); + tbody.appendChild(tr); } table.appendChild(tbody); @@ -1069,7 +1141,7 @@ function buildDetailField(key: string, value: string, isProtected: boolean): HTM } const copyBtn = makeIconButton('icon-btn', 'Copy', '📋', async () => { - await copyToClipboard(value); + await copyToClipboard(value, key); copyBtn.textContent = '✓'; setTimeout(() => { copyBtn.textContent = '📋'; @@ -1301,7 +1373,7 @@ function buildEditField( const copyBtn = makeIconButton('icon-btn', 'Copy', '📋', async () => { // Copies whatever is currently typed, not the value the field opened // with — the user may have already edited it. - await copyToClipboard(valueInput.value); + await copyToClipboard(valueInput.value, keyInput.value); copyBtn.textContent = '✓'; setTimeout(() => { copyBtn.textContent = '📋'; diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index 957c4be..496bf73 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -1250,6 +1250,9 @@ test('0x67 app', async (t) => { .then(() => Promise.resolve()) .then(() => { assert.equal(copyBtn?.textContent, '✓'); + // The toast names the field and never carries the value itself (#67). + assert.equal(byId('toast').hidden, false); + assert.equal(byId('toast').textContent, 'Password copied to clipboard'); // Second, immediate copy exercises the "clear the pending timer" // branch in copyToClipboard before advancing time at all. copyBtn?.click(); @@ -1274,6 +1277,8 @@ test('0x67 app', async (t) => { // The rejected write must not throw, and must not have "succeeded" // in clearing the (mock) clipboard either. assert.equal(clipboardText, 'still there before the timer fires'); + // One timer drives both, so the reminder goes when the clipboard does. + assert.equal(byId('toast').hidden, true); }); }, ); @@ -2129,12 +2134,15 @@ test('entry list table view: default columns, masked password, column toggling, Array.from(root().querySelectorAll('.entry-table th')).map((th) => th.textContent ?? ''); assert.deepEqual( headerText(), - ['Title', 'Username', 'Password', 'URL', 'Modified'], - 'default visible columns', + ['Title', 'Username', 'Password', 'URL', 'Modified', ''], + 'default visible columns, plus the unlabelled open-entry column', ); + // The cell's own text, without the copy hint appended beside it. const bodyCells = (): string[] => - Array.from(root().querySelectorAll('.entry-table tbody td')).map((td) => td.textContent ?? ''); + Array.from(root().querySelectorAll('.entry-table tbody td')).map( + (td) => td.firstChild?.textContent ?? '', + ); const [titleCell, usernameCell, passwordCell, urlCell] = bodyCells(); assert.ok(titleCell?.includes('GitHub')); assert.equal(usernameCell, 'octocat'); @@ -2162,29 +2170,67 @@ test('entry list table view: default columns, masked password, column toggling, assert.ok(bodyCells().includes('work account')); t.mock.timers.enable({ apis: ['setTimeout'] }); - const row = q('.entry-table tbody tr') as HTMLElement; - dispatch(row, 'click'); - assert.equal(q('#detail-title'), null, 'not yet — still within the delay window'); - t.mock.timers.tick(250); - assert.ok(q('#detail-title')?.textContent?.includes('GitHub')); - - // Back to the entry list, table view, to exercise the double-click path. - dispatch(q('[data-action="back"]'), 'click'); - dispatch(q('[data-action="view-table"]'), 'click'); + const press = (el: EventTarget, type: string, x = 0, y = 0): void => { + dispatch(el, type, { clientX: x, clientY: y, pointerId: 1 }); + }; clipboardWritesShouldFail = false; - const passwordTd = Array.from(root().querySelectorAll('.entry-table tbody td')).find( - (td) => td.textContent === '••••••••', - ) as HTMLElement; - dispatch(passwordTd, 'click'); - dispatch(passwordTd, 'dblclick'); - // The double-click's own handler cancels the pending single-click timer — - // advancing past its delay must not open the entry. - t.mock.timers.tick(250); - assert.equal(q('#detail-title'), null, 'the double-click cancelled the pending open'); + const maskedCell = (): HTMLElement => + Array.from(root().querySelectorAll('.entry-table tbody td')).find( + (td) => td.firstChild?.textContent === '••••••••', + ) as HTMLElement; + + // A tap copies the real value, names it, and does not open the card. + press(maskedCell(), 'pointerdown'); + press(maskedCell(), 'pointerup'); await Promise.resolve(); await Promise.resolve(); assert.equal(clipboardText, 'hunter2', 'the real password was copied, not the mask'); + assert.equal(byId('toast').hidden, false); + assert.equal(byId('toast').textContent, 'Password copied to clipboard'); + assert.ok(!byId('toast').textContent?.includes('hunter2'), 'never the value itself'); + assert.equal(q('#detail-title'), null, 'a tap does not open the card'); + + // A drag that begins on a cell is a scroll, not a tap. + clipboardText = ''; + press(maskedCell(), 'pointerdown', 0, 0); + press(maskedCell(), 'pointermove', 0, 40); + press(maskedCell(), 'pointerup', 0, 40); + await Promise.resolve(); + assert.equal(clipboardText, '', 'moving past the slop cancels the copy'); + + // Staying inside the slop is still a tap. + press(maskedCell(), 'pointerdown', 0, 0); + press(maskedCell(), 'pointermove', 0, 2); + press(maskedCell(), 'pointerup', 0, 2); + await Promise.resolve(); + await Promise.resolve(); + assert.equal(clipboardText, 'hunter2', 'a small wobble still copies'); + + // A cancelled press neither copies nor opens. + clipboardText = ''; + press(maskedCell(), 'pointerdown'); + press(maskedCell(), 'pointercancel'); + t.mock.timers.tick(500); + await Promise.resolve(); + assert.equal(clipboardText, '', 'a cancelled press copies nothing'); + assert.equal(q('#detail-title'), null, 'and opens nothing'); + + // A hold opens the card. + press(maskedCell(), 'pointerdown'); + t.mock.timers.tick(500); + assert.ok(q('#detail-title')?.textContent?.includes('GitHub'), 'a hold opens it'); + dispatch(q('[data-action="back"]'), 'click'); + dispatch(q('[data-action="view-table"]'), 'click'); + + // The visible way in, for anyone who never discovers the hold. + const openBtn = (q('.entry-table tbody tr') as HTMLElement).querySelector( + '.entry-table-open button', + ) as HTMLButtonElement; + dispatch(openBtn, 'click'); + assert.ok(q('#detail-title')?.textContent?.includes('GitHub'), 'the › opens it too'); + dispatch(q('[data-action="back"]'), 'click'); + dispatch(q('[data-action="view-table"]'), 'click'); // Turn on Created and Attachments too — Created exercises the other half // of entryColumnDisplayValue's date-formatting check, and Attachments (0, @@ -2208,25 +2254,15 @@ test('entry list table view: default columns, masked password, column toggling, const attachmentsTd = (q('.entry-table tbody tr') as HTMLElement).querySelectorAll('td')[ columnIndex ] as HTMLElement; - assert.equal(attachmentsTd.textContent, '', 'no attachments on this entry'); - dispatch(attachmentsTd, 'dblclick'); - await Promise.resolve(); + assert.equal(attachmentsTd.firstChild?.textContent, '', 'no attachments on this entry'); + assert.equal(attachmentsTd.querySelector('.copy-hint'), null, 'and so no copy hint'); + clipboardText = ''; + press(attachmentsTd, 'pointerdown'); + press(attachmentsTd, 'pointerup'); await Promise.resolve(); - assert.equal( - clipboardText, - 'hunter2', - 'still the last real copy — the empty cell copied nothing', - ); - - // Clicking the same row twice before the delay elapses reschedules rather - // than opening the entry twice. - const row2 = q('.entry-table tbody tr') as HTMLElement; - dispatch(row2, 'click'); - dispatch(row2, 'click'); - t.mock.timers.tick(250); - assert.ok(q('#detail-title')?.textContent?.includes('GitHub')); - dispatch(q('[data-action="back"]'), 'click'); + assert.equal(clipboardText, '', 'a cell with nothing in it copies nothing'); + t.mock.timers.reset(); dispatch(q('[data-action="view-tile"]'), 'click'); assert.equal(root().querySelectorAll('.entry-row').length, 2); assert.equal(root().querySelectorAll('.entry-table').length, 0); From b3436f996efe8054bbda344d12d185d231177794 Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Sat, 5 Sep 2026 20:04:35 -0400 Subject: [PATCH 2/5] fix(0x67):#67:only the primary button copies, and capture the press --- pages/0x67/page.ts | 8 ++++++-- pages/tests/0x67-page.test.ts | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index 6b94b1d..6def14f 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -728,6 +728,10 @@ function wireCellPress( }; cell.addEventListener('pointerdown', (down) => { + if (down.button !== 0) return; // a right- or middle-click is not a copy (#67) + /* Capture, so a release outside the cell still ends the gesture here; without + it the hold timer survives and opens the card on its own. */ + cell.setPointerCapture(down.pointerId); startX = down.clientX; startY = down.clientY; holdTimer = setTimeout(() => { @@ -742,8 +746,8 @@ function wireCellPress( cell.addEventListener('pointercancel', cancelHold); - cell.addEventListener('pointerup', () => { - const tapped = holdTimer !== null; + cell.addEventListener('pointerup', (up) => { + const tapped = holdTimer !== null && up.button === 0; cancelHold(); if (tapped && value) copyToClipboard(value, label); }); diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index 496bf73..3af9447 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -2170,8 +2170,8 @@ test('entry list table view: default columns, masked password, column toggling, assert.ok(bodyCells().includes('work account')); t.mock.timers.enable({ apis: ['setTimeout'] }); - const press = (el: EventTarget, type: string, x = 0, y = 0): void => { - dispatch(el, type, { clientX: x, clientY: y, pointerId: 1 }); + const press = (el: EventTarget, type: string, x = 0, y = 0, button = 0): void => { + dispatch(el, type, { clientX: x, clientY: y, pointerId: 1, button }); }; clipboardWritesShouldFail = false; @@ -2216,6 +2216,18 @@ test('entry list table view: default columns, masked password, column toggling, assert.equal(clipboardText, '', 'a cancelled press copies nothing'); assert.equal(q('#detail-title'), null, 'and opens nothing'); + // Only the primary button copies: a right-click must not put a password on + // the clipboard, and neither must a primary press released with another. + clipboardText = ''; + press(maskedCell(), 'pointerdown', 0, 0, 2); + press(maskedCell(), 'pointerup', 0, 0, 2); + await Promise.resolve(); + assert.equal(clipboardText, '', 'a right-click copies nothing'); + press(maskedCell(), 'pointerdown'); + press(maskedCell(), 'pointerup', 0, 0, 2); + await Promise.resolve(); + assert.equal(clipboardText, '', 'nor a primary press released with another button'); + // A hold opens the card. press(maskedCell(), 'pointerdown'); t.mock.timers.tick(500); From 03c556b50732d9ed3a97a55d1201ff0030ed1ff6 Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Sat, 5 Sep 2026 20:27:52 -0400 Subject: [PATCH 3/5] fix(0x67):#67:keyboard-reachable copy, honest toast, one label per field --- e2e/entry-copy.test.ts | 99 +++++++++++++++++++++++++++++++++++ e2e/group-rail.test.ts | 24 ++------- e2e/support/app.ts | 20 +++++++ pages/0x67/page.css | 26 ++++----- pages/0x67/page.ts | 47 ++++++++++------- pages/tests/0x67-page.test.ts | 44 ++++++++++++++-- 6 files changed, 205 insertions(+), 55 deletions(-) create mode 100644 e2e/entry-copy.test.ts create mode 100644 e2e/support/app.ts diff --git a/e2e/entry-copy.test.ts b/e2e/entry-copy.test.ts new file mode 100644 index 0000000..1b7590f --- /dev/null +++ b/e2e/entry-copy.test.ts @@ -0,0 +1,99 @@ +/** Real-browser coverage for the table's copy gesture (issue #67). Pointer + * capture is a no-op in jsdom, so a release routed back to the cell is only + * observable here — which is what keeps a tap from becoming a hold. + * + * What is copied is asserted in the jsdom tests instead: headless Chrome + * refuses `navigator.clipboard.writeText` outright ("Write permission denied") + * even with the permission overridden, so the toast never appears here. */ +import assert from 'node:assert/strict'; +import { after, before, test } from 'node:test'; +import { fileURLToPath } from 'node:url'; +import puppeteer, { type Browser, type Frame, type Page } from 'puppeteer-core'; +import { openApp } from './support/app.ts'; +import { resolveChromePath } from './support/chrome.ts'; +import { type DistServer, startDistServer } from './support/dist-server.ts'; +import { type KdbxFixture, writeKdbxFixture } from './support/fixture.ts'; +import { resolveLaunchOptions } from './support/launch-options.ts'; + +const distDir = fileURLToPath(new URL('../dist', import.meta.url)); + +let server: DistServer; +let browser: Browser; +let page: Page; +let app: Frame; +let fixture: KdbxFixture; + +before(async () => { + server = await startDistServer(distDir); + browser = await puppeteer.launch({ + executablePath: resolveChromePath(), + ...resolveLaunchOptions(), + args: ['--no-sandbox'], + }); + page = await browser.newPage(); + await page.setViewport({ width: 1280, height: 900 }); + fixture = await writeKdbxFixture(); + app = await openApp(page, server.origin, fixture); + await app.click('[data-action="view-table"]'); + await app.waitForSelector('.entry-table'); +}); + +after(async () => { + await browser.close(); + await server.close(); +}); + +/** Centre of the cell holding the fixture entry's username. */ +async function usernameCellCentre(): Promise<{ x: number; y: number }> { + const box = await app.$$eval( + '.entry-table tbody td', + (cells, name) => { + const cell = cells.find((c) => c.firstChild?.textContent === name); + if (!cell) return null; + const rect = cell.getBoundingClientRect(); + return { x: rect.x + rect.width / 2, y: rect.y + rect.height / 2 }; + }, + 'octocat', + ); + assert.ok(box, 'the username cell is on screen'); + const frameBox = await (await app.frameElement())?.boundingBox(); + assert.ok(frameBox, 'the app frame is laid out'); + return { x: frameBox.x + box.x, y: frameBox.y + box.y }; +} + +test('a tap copies without opening the entry, even past the hold delay', async () => { + const { x, y } = await usernameCellCentre(); + await page.mouse.click(x, y); + + // Past the threshold: had the release not reached the cell, the pending + // hold timer would open the card right about here. + await new Promise((resolve) => setTimeout(resolve, 900)); + assert.equal(await app.$('#detail-title'), null, 'the card stayed shut'); +}); + +test('releasing outside the cell abandons the press rather than opening the entry', async () => { + const { x, y } = await usernameCellCentre(); + await page.mouse.move(x, y); + await page.mouse.down(); + await page.mouse.move(x + 300, y + 200, { steps: 4 }); + await page.mouse.up(); + + await new Promise((resolve) => setTimeout(resolve, 900)); + assert.equal(await app.$('#detail-title'), null, 'dragging off the cell opened nothing'); +}); + +test('holding the cell opens the entry instead', async () => { + const { x, y } = await usernameCellCentre(); + await page.mouse.move(x, y); + await page.mouse.down(); + await new Promise((resolve) => setTimeout(resolve, 700)); + await page.mouse.up(); + + const title = await app.waitForSelector('#detail-title'); + assert.ok(title, 'the card opened'); + assert.match( + (await title.evaluate((el) => el.textContent)) ?? '', + /Example Entry/, + 'and it is the entry that was held', + ); +}); diff --git a/e2e/group-rail.test.ts b/e2e/group-rail.test.ts index b5fee3e..ff04303 100644 --- a/e2e/group-rail.test.ts +++ b/e2e/group-rail.test.ts @@ -6,6 +6,7 @@ import assert from 'node:assert/strict'; import { after, before, test } from 'node:test'; import { fileURLToPath } from 'node:url'; import puppeteer, { type Browser, type ElementHandle, type Frame, type Page } from 'puppeteer-core'; +import { openApp } from './support/app.ts'; import { resolveChromePath } from './support/chrome.ts'; import { type DistServer, startDistServer } from './support/dist-server.ts'; import { type KdbxFixture, writeKdbxFixture } from './support/fixture.ts'; @@ -32,28 +33,9 @@ before(async () => { await page.setViewport({ width: 1280, height: 900 }); fixture = await writeKdbxFixture(); - app = await openApp(page); + app = await openApp(page, server.origin, fixture); }); -/** Upload the fixture to local.html and unlock the app it embeds, returning - * the app's frame. */ -async function openApp(target: Page): Promise { - await target.goto(`${server.origin}/local.html`, { waitUntil: 'networkidle0' }); - const fileInput = (await target.waitForSelector( - '#file-input', - )) as ElementHandle; - await fileInput.uploadFile(fixture.path); - const frameElement = await target.waitForSelector('#app-frame'); - assert.ok(frameElement, 'the app is embedded in an iframe'); - const frame = (await frameElement.contentFrame()) as Frame; - const passwordInput = await frame.waitForSelector('#master-password'); - assert.ok(passwordInput, 'the embedded app shows its unlock screen'); - await passwordInput.type(fixture.password); - await frame.click('#unlock-btn'); - await frame.waitForSelector('#group-tree .group-btn'); - return frame; -} - after(async () => { await browser.close(); await server.close(); @@ -119,7 +101,7 @@ test('dragging the handle resizes the rail', async () => { test('at phone width the rail is a drawer: no resize handle, and ⋯ still reaches rename', async () => { const phone = await browser.newPage(); await phone.setViewport({ width: 375, height: 812 }); - const phoneApp = await openApp(phone); + const phoneApp = await openApp(phone, server.origin, fixture); assert.equal( await phoneApp.$eval('#sidebar-resize', (el) => getComputedStyle(el).display), diff --git a/e2e/support/app.ts b/e2e/support/app.ts new file mode 100644 index 0000000..dcae302 --- /dev/null +++ b/e2e/support/app.ts @@ -0,0 +1,20 @@ +/** Uploads the fixture to local.html and unlocks the app it embeds, returning + * the app's frame. Shared by every e2e that needs an open database. */ +import assert from 'node:assert/strict'; +import type { ElementHandle, Frame, Page } from 'puppeteer-core'; +import type { KdbxFixture } from './fixture.ts'; + +export async function openApp(page: Page, origin: string, fixture: KdbxFixture): Promise { + await page.goto(`${origin}/local.html`, { waitUntil: 'networkidle0' }); + const fileInput = (await page.waitForSelector('#file-input')) as ElementHandle; + await fileInput.uploadFile(fixture.path); + const frameElement = await page.waitForSelector('#app-frame'); + assert.ok(frameElement, 'the app is embedded in an iframe'); + const frame = (await frameElement.contentFrame()) as Frame; + const passwordInput = await frame.waitForSelector('#master-password'); + assert.ok(passwordInput, 'the embedded app shows its unlock screen'); + await passwordInput.type(fixture.password); + await frame.click('#unlock-btn'); + await frame.waitForSelector('#group-tree .group-btn'); + return frame; +} diff --git a/pages/0x67/page.css b/pages/0x67/page.css index e3ac448..6a712b0 100644 --- a/pages/0x67/page.css +++ b/pages/0x67/page.css @@ -791,7 +791,12 @@ so an absolutely positioned menu would be cut off. */ white-space: nowrap; } +/* A tap on a cell copies (#67), so the cell must not start a text selection or +wait on a double-tap-to-zoom before that tap registers. */ .entry-table td { + touch-action: manipulation; + user-select: none; + -webkit-touch-callout: none; padding: 0.5rem 0.75rem; border-bottom: 1px solid var(--border); max-width: 220px; @@ -801,24 +806,21 @@ so an absolutely positioned menu would be cut off. */ cursor: pointer; } -/* A tap on a cell copies (#67), so the cell must not start a text selection or -wait on a double-tap-to-zoom before that tap registers. */ -.entry-table td { - touch-action: manipulation; - user-select: none; - -webkit-touch-callout: none; -} - -/* Present at rest, brighter on hover (#67): hover may raise emphasis, never be -what makes the affordance discoverable, since touch has none. */ +/* Present at rest, brighter on hover or focus (#67): hover may raise emphasis, +never be what makes the affordance discoverable, since touch has none. */ .copy-hint { margin-left: 0.35rem; - opacity: 0.25; + padding: 0; + border: none; + background: none; + cursor: pointer; font-size: 0.8em; + line-height: 1; + opacity: 0.25; } .entry-table td:hover .copy-hint, -.entry-table td:focus-within .copy-hint { +.copy-hint:focus-visible { opacity: 1; } diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index 6def14f..00e37f4 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -117,6 +117,13 @@ logic.ts) are globals from bundle-iife's concatenation; see globals.d.ts. */ let clipboardTimer: ReturnType | null = null; +const FIELD_LABELS: Record = { UserName: 'Username' }; + +// KeePass's own field keys are not what the rest of the app calls them (#67). +function fieldLabel(key: string): string { + return FIELD_LABELS[key] ?? key; +} + /* Names the field, never the value (#67): the toast is a reminder that a secret is on the clipboard, so it must not put that secret on screen as well. */ function showClipboardToast(label: string): void { @@ -133,9 +140,13 @@ async function copyToClipboard(text: string, label = 'Value'): Promise { /* One timer drives both the wipe and the toast (#67), so the toast is gone exactly when the clipboard is, rather than on a schedule of its own. */ clipboardTimer = setTimeout(() => { - navigator.clipboard.writeText('').catch(() => {}); clipboardTimer = null; - byId('toast').hidden = true; + navigator.clipboard + .writeText('') + .then(() => { + byId('toast').hidden = true; + }) + .catch(() => {}); // the clear failed, so the value is still there and the toast stays }, app.clipboardTimeout * 1000); } catch (err) { console.error('Clipboard write failed', err); @@ -693,7 +704,7 @@ const ENTRY_COLUMNS: ReadonlyArray<{ key: EntryColumnKey; label: string }> = [ ]; /** Display text: dates formatted, password masked. entryColumnValue itself - * (unmasked) is what a double-click copies — see buildEntryTable. */ + * (unmasked) is what a tap copies — see buildEntryTable. */ function entryColumnDisplayValue(entry: XmlElement, column: EntryColumnKey): string { const raw = entryColumnValue(entry, column); if (column === 'password') return raw ? '••••••••' : ''; @@ -709,9 +720,10 @@ function openEntry(entry: XmlElement): void { showEntryDetail(); } -/* A tap copies, a hold opens the card (#67). Hold duration separates the two at -release, so neither waits on the other the way click-versus-double-click did. -The slop check matters on touch, where scrolling the table starts on a cell. */ +/* A tap copies, a hold opens the card (#67). The hold fires on its own timer +while the pointer is still down, and a release before then copies instead, so +neither gesture waits on the other the way click-versus-double-click did. The +slop check matters on touch, where scrolling the table starts on a cell. */ function wireCellPress( cell: HTMLTableCellElement, entry: XmlElement, @@ -729,6 +741,7 @@ function wireCellPress( cell.addEventListener('pointerdown', (down) => { if (down.button !== 0) return; // a right- or middle-click is not a copy (#67) + if (down.target !== cell) return; // the copy button runs its own handler /* Capture, so a release outside the cell still ends the gesture here; without it the hold timer survives and opens the card on its own. */ cell.setPointerCapture(down.pointerId); @@ -753,13 +766,12 @@ function wireCellPress( }); } -/* Dim rather than hover-only (#67): hover may raise a control's emphasis, but a -touch user has no hover, so it must never be what makes it discoverable. */ -function copyHint(): HTMLSpanElement { - const hint = document.createElement('span'); - hint.className = 'copy-hint'; - hint.textContent = '📋'; - return hint; +/* A button, not decoration (#67): dim at rest so hover only raises its emphasis, +and focusable so copying is reachable without a pointer at all. */ +function copyHint(value: string, label: string): HTMLButtonElement { + return makeIconButton('copy-hint', `Copy ${label.toLowerCase()}`, '📋', () => { + copyToClipboard(value, label); + }); } function buildEntryCell( @@ -770,10 +782,7 @@ function buildEntryCell( ): HTMLTableCellElement { const td = document.createElement('td'); td.appendChild(document.createTextNode(display)); - if (value) { - td.appendChild(copyHint()); - td.title = `Copy ${label.toLowerCase()}`; - } + if (value) td.appendChild(copyHint(value, label)); wireCellPress(td, entry, value, label); return td; } @@ -1145,7 +1154,7 @@ function buildDetailField(key: string, value: string, isProtected: boolean): HTM } const copyBtn = makeIconButton('icon-btn', 'Copy', '📋', async () => { - await copyToClipboard(value, key); + await copyToClipboard(value, fieldLabel(key)); copyBtn.textContent = '✓'; setTimeout(() => { copyBtn.textContent = '📋'; @@ -1377,7 +1386,7 @@ function buildEditField( const copyBtn = makeIconButton('icon-btn', 'Copy', '📋', async () => { // Copies whatever is currently typed, not the value the field opened // with — the user may have already edited it. - await copyToClipboard(valueInput.value, keyInput.value); + await copyToClipboard(valueInput.value, fieldLabel(keyInput.value) || 'Value'); copyBtn.textContent = '✓'; setTimeout(() => { copyBtn.textContent = '📋'; diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index 3af9447..d9fe55e 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -1237,17 +1237,29 @@ test('0x67 app', async (t) => { 'copying a field writes to the clipboard, flips the icon, then reverts on a timer', (t) => { t.mock.timers.enable({ apis: ['setTimeout'] }); + const usernameRow = Array.from(root().querySelectorAll('.detail-field')).find( + (row) => row.querySelector('.detail-label')?.textContent === 'UserName', + ) as HTMLElement; const passwordRow = Array.from(root().querySelectorAll('.detail-field')).find( (row) => row.querySelector('.detail-label')?.textContent === 'Password', ) as HTMLElement; const copyBtn = passwordRow.querySelector('[title="Copy"]'); clipboardWritesShouldFail = false; - copyBtn?.click(); + usernameRow.querySelector('[title="Copy"]')?.click(); // The write and the icon flip both happen inside an async click handler; // let its microtasks settle before advancing fake timers. return Promise.resolve() .then(() => Promise.resolve()) + .then(() => { + assert.equal( + byId('toast').textContent, + 'Username copied to clipboard', + "not KeePass's own 'UserName'", + ); + copyBtn?.click(); + return Promise.resolve().then(() => Promise.resolve()); + }) .then(() => { assert.equal(copyBtn?.textContent, '✓'); // The toast names the field and never carries the value itself (#67). @@ -1277,8 +1289,19 @@ test('0x67 app', async (t) => { // The rejected write must not throw, and must not have "succeeded" // in clearing the (mock) clipboard either. assert.equal(clipboardText, 'still there before the timer fires'); - // One timer drives both, so the reminder goes when the clipboard does. - assert.equal(byId('toast').hidden, true); + // The clear failed, so the value is still on the clipboard and the + // reminder must not retire as though it were gone. + assert.equal(byId('toast').hidden, false); + copyBtn?.click(); + return Promise.resolve().then(() => Promise.resolve()); + }) + .then(() => { + t.mock.timers.tick(30_000); + return Promise.resolve().then(() => Promise.resolve()); + }) + .then(() => { + assert.equal(clipboardText, '', 'this clear succeeded'); + assert.equal(byId('toast').hidden, true, 'so the reminder retires'); }); }, ); @@ -1300,7 +1323,10 @@ test('0x67 app', async (t) => { // value the field was opened with. valueInput.value = 'not-yet-saved-password'; clipboardWritesShouldFail = false; + const keyInput = passwordRow.querySelector('.edit-key') as HTMLInputElement; + keyInput.value = ''; copyBtn?.click(); + keyInput.value = 'Password'; return Promise.resolve() .then(() => Promise.resolve()) @@ -2228,6 +2254,18 @@ test('entry list table view: default columns, masked password, column toggling, await Promise.resolve(); assert.equal(clipboardText, '', 'nor a primary press released with another button'); + clipboardText = ''; + const hintBtn = maskedCell().querySelector('.copy-hint') as HTMLButtonElement; + assert.equal(hintBtn.title, 'Copy password', 'the button carries its own name'); + press(hintBtn, 'pointerdown'); + press(hintBtn, 'pointerup'); + await Promise.resolve(); + assert.equal(clipboardText, '', 'pressing the button does not also run the cell handler'); + dispatch(hintBtn, 'click'); + await Promise.resolve(); + await Promise.resolve(); + assert.equal(clipboardText, 'hunter2', 'clicking it copies, so a keyboard can reach it'); + // A hold opens the card. press(maskedCell(), 'pointerdown'); t.mock.timers.tick(500); From ef37c25a02fff0df37936c292fa14728bddf385a Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Mon, 7 Sep 2026 12:19:39 -0400 Subject: [PATCH 4/5] refactor(0x67):#67:drop the hold gesture, open only via the row control --- e2e/entry-copy.test.ts | 43 +++++++-------------- pages/0x67/page.ts | 66 +++++--------------------------- pages/tests/0x67-page.test.ts | 71 +++++------------------------------ 3 files changed, 33 insertions(+), 147 deletions(-) diff --git a/e2e/entry-copy.test.ts b/e2e/entry-copy.test.ts index 1b7590f..9c2cc4e 100644 --- a/e2e/entry-copy.test.ts +++ b/e2e/entry-copy.test.ts @@ -1,10 +1,11 @@ -/** Real-browser coverage for the table's copy gesture (issue #67). Pointer - * capture is a no-op in jsdom, so a release routed back to the cell is only - * observable here — which is what keeps a tap from becoming a hold. +/** Real-browser coverage for the entry table's controls (issue #67). jsdom + * dispatches events straight at an element; only a real browser hit-tests a + * coordinate, so this is what proves the cells and the open control are + * actually clickable where they render. * - * What is copied is asserted in the jsdom tests instead: headless Chrome - * refuses `navigator.clipboard.writeText` outright ("Write permission denied") - * even with the permission overridden, so the toast never appears here. */ + * What gets copied is asserted in the jsdom tests instead: headless Chrome + * refuses `navigator.clipboard.writeText` outright ("Write permission denied"), + * so no toast ever appears here. */ import assert from 'node:assert/strict'; import { after, before, test } from 'node:test'; import { fileURLToPath } from 'node:url'; @@ -61,39 +62,23 @@ async function usernameCellCentre(): Promise<{ x: number; y: number }> { return { x: frameBox.x + box.x, y: frameBox.y + box.y }; } -test('a tap copies without opening the entry, even past the hold delay', async () => { +test('clicking a value never opens the entry', async () => { const { x, y } = await usernameCellCentre(); await page.mouse.click(x, y); - - // Past the threshold: had the release not reached the cell, the pending - // hold timer would open the card right about here. - await new Promise((resolve) => setTimeout(resolve, 900)); + await new Promise((resolve) => setTimeout(resolve, 300)); assert.equal(await app.$('#detail-title'), null, 'the card stayed shut'); }); -test('releasing outside the cell abandons the press rather than opening the entry', async () => { - const { x, y } = await usernameCellCentre(); - await page.mouse.move(x, y); - await page.mouse.down(); - await page.mouse.move(x + 300, y + 200, { steps: 4 }); - await page.mouse.up(); - - await new Promise((resolve) => setTimeout(resolve, 900)); - assert.equal(await app.$('#detail-title'), null, 'dragging off the cell opened nothing'); -}); - -test('holding the cell opens the entry instead', async () => { - const { x, y } = await usernameCellCentre(); - await page.mouse.move(x, y); - await page.mouse.down(); - await new Promise((resolve) => setTimeout(resolve, 700)); - await page.mouse.up(); +test('the row control is the way into the card', async () => { + const openButton = await app.$('.entry-table-open button'); + assert.ok(openButton, 'every row carries one'); + await openButton.click(); const title = await app.waitForSelector('#detail-title'); assert.ok(title, 'the card opened'); assert.match( (await title.evaluate((el) => el.textContent)) ?? '', /Example Entry/, - 'and it is the entry that was held', + 'and it is the row that was clicked', ); }); diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index 00e37f4..9fe1433 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -712,57 +712,18 @@ function entryColumnDisplayValue(entry: XmlElement, column: EntryColumnKey): str return raw; } -const ENTRY_HOLD_MS = 500; -const ENTRY_HOLD_SLOP_PX = 8; - function openEntry(entry: XmlElement): void { app.currentEntry = entry; showEntryDetail(); } -/* A tap copies, a hold opens the card (#67). The hold fires on its own timer -while the pointer is still down, and a release before then copies instead, so -neither gesture waits on the other the way click-versus-double-click did. The -slop check matters on touch, where scrolling the table starts on a cell. */ -function wireCellPress( - cell: HTMLTableCellElement, - entry: XmlElement, - value: string, - label: string, -): void { - let holdTimer: ReturnType | null = null; - let startX = 0; - let startY = 0; - - const cancelHold = (): void => { - if (holdTimer) clearTimeout(holdTimer); - holdTimer = null; - }; - - cell.addEventListener('pointerdown', (down) => { - if (down.button !== 0) return; // a right- or middle-click is not a copy (#67) - if (down.target !== cell) return; // the copy button runs its own handler - /* Capture, so a release outside the cell still ends the gesture here; without - it the hold timer survives and opens the card on its own. */ - cell.setPointerCapture(down.pointerId); - startX = down.clientX; - startY = down.clientY; - holdTimer = setTimeout(() => { - holdTimer = null; - openEntry(entry); - }, ENTRY_HOLD_MS); - }); - - cell.addEventListener('pointermove', (move) => { - if (Math.hypot(move.clientX - startX, move.clientY - startY) > ENTRY_HOLD_SLOP_PX) cancelHold(); - }); - - cell.addEventListener('pointercancel', cancelHold); - - cell.addEventListener('pointerup', (up) => { - const tapped = holdTimer !== null && up.button === 0; - cancelHold(); - if (tapped && value) copyToClipboard(value, label); +/* A click copies (#67); opening the card is the row's own button instead. The +browser already decides what counts as a click, so a scroll that starts on a +cell copies nothing and a secondary button never reaches here at all. */ +function wireCellCopy(cell: HTMLTableCellElement, value: string, label: string): void { + cell.addEventListener('click', (event) => { + if (event.target !== cell) return; // the copy button runs its own handler + if (value) copyToClipboard(value, label); }); } @@ -774,16 +735,11 @@ function copyHint(value: string, label: string): HTMLButtonElement { }); } -function buildEntryCell( - entry: XmlElement, - display: string, - value: string, - label: string, -): HTMLTableCellElement { +function buildEntryCell(display: string, value: string, label: string): HTMLTableCellElement { const td = document.createElement('td'); td.appendChild(document.createTextNode(display)); if (value) td.appendChild(copyHint(value, label)); - wireCellPress(td, entry, value, label); + wireCellCopy(td, value, label); return td; } @@ -812,7 +768,6 @@ function buildEntryTable(rows: EntryWithGroup[]): HTMLTableElement { const tr = document.createElement('tr'); const titleTd = buildEntryCell( - entry, `${iconEmoji(elementIconId(entry))} ${entryTitle(entry)}`, entryTitle(entry), 'Title', @@ -822,7 +777,6 @@ function buildEntryTable(rows: EntryWithGroup[]): HTMLTableElement { for (const column of visibleColumns) { const td = buildEntryCell( - entry, entryColumnDisplayValue(entry, column.key), entryColumnValue(entry, column.key), column.label, @@ -831,7 +785,7 @@ function buildEntryTable(rows: EntryWithGroup[]): HTMLTableElement { tr.appendChild(td); } - // A visible way in, since a hold is not discoverable on its own (#67). + // The only way into the card, so opening to edit stays a deliberate act (#67). const openTd = document.createElement('td'); openTd.className = 'entry-table-open'; openTd.appendChild(makeIconButton('icon-btn', 'Open entry', '›', () => openEntry(entry))); diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index d9fe55e..7c0c088 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -2196,89 +2196,38 @@ test('entry list table view: default columns, masked password, column toggling, assert.ok(bodyCells().includes('work account')); t.mock.timers.enable({ apis: ['setTimeout'] }); - const press = (el: EventTarget, type: string, x = 0, y = 0, button = 0): void => { - dispatch(el, type, { clientX: x, clientY: y, pointerId: 1, button }); - }; - clipboardWritesShouldFail = false; const maskedCell = (): HTMLElement => Array.from(root().querySelectorAll('.entry-table tbody td')).find( (td) => td.firstChild?.textContent === '••••••••', ) as HTMLElement; - // A tap copies the real value, names it, and does not open the card. - press(maskedCell(), 'pointerdown'); - press(maskedCell(), 'pointerup'); + // A click copies the real value, names it, and leaves the card shut. + dispatch(maskedCell(), 'click'); await Promise.resolve(); await Promise.resolve(); assert.equal(clipboardText, 'hunter2', 'the real password was copied, not the mask'); assert.equal(byId('toast').hidden, false); assert.equal(byId('toast').textContent, 'Password copied to clipboard'); assert.ok(!byId('toast').textContent?.includes('hunter2'), 'never the value itself'); - assert.equal(q('#detail-title'), null, 'a tap does not open the card'); - - // A drag that begins on a cell is a scroll, not a tap. - clipboardText = ''; - press(maskedCell(), 'pointerdown', 0, 0); - press(maskedCell(), 'pointermove', 0, 40); - press(maskedCell(), 'pointerup', 0, 40); - await Promise.resolve(); - assert.equal(clipboardText, '', 'moving past the slop cancels the copy'); - - // Staying inside the slop is still a tap. - press(maskedCell(), 'pointerdown', 0, 0); - press(maskedCell(), 'pointermove', 0, 2); - press(maskedCell(), 'pointerup', 0, 2); - await Promise.resolve(); - await Promise.resolve(); - assert.equal(clipboardText, 'hunter2', 'a small wobble still copies'); - - // A cancelled press neither copies nor opens. - clipboardText = ''; - press(maskedCell(), 'pointerdown'); - press(maskedCell(), 'pointercancel'); - t.mock.timers.tick(500); - await Promise.resolve(); - assert.equal(clipboardText, '', 'a cancelled press copies nothing'); - assert.equal(q('#detail-title'), null, 'and opens nothing'); - - // Only the primary button copies: a right-click must not put a password on - // the clipboard, and neither must a primary press released with another. - clipboardText = ''; - press(maskedCell(), 'pointerdown', 0, 0, 2); - press(maskedCell(), 'pointerup', 0, 0, 2); - await Promise.resolve(); - assert.equal(clipboardText, '', 'a right-click copies nothing'); - press(maskedCell(), 'pointerdown'); - press(maskedCell(), 'pointerup', 0, 0, 2); - await Promise.resolve(); - assert.equal(clipboardText, '', 'nor a primary press released with another button'); + assert.equal(q('#detail-title'), null, 'copying never opens the card'); + // The 📋 is a real button, so a keyboard reaches it; its click must not also + // run the cell's own handler underneath. clipboardText = ''; const hintBtn = maskedCell().querySelector('.copy-hint') as HTMLButtonElement; assert.equal(hintBtn.title, 'Copy password', 'the button carries its own name'); - press(hintBtn, 'pointerdown'); - press(hintBtn, 'pointerup'); - await Promise.resolve(); - assert.equal(clipboardText, '', 'pressing the button does not also run the cell handler'); dispatch(hintBtn, 'click'); await Promise.resolve(); await Promise.resolve(); - assert.equal(clipboardText, 'hunter2', 'clicking it copies, so a keyboard can reach it'); - - // A hold opens the card. - press(maskedCell(), 'pointerdown'); - t.mock.timers.tick(500); - assert.ok(q('#detail-title')?.textContent?.includes('GitHub'), 'a hold opens it'); - dispatch(q('[data-action="back"]'), 'click'); - dispatch(q('[data-action="view-table"]'), 'click'); + assert.equal(clipboardText, 'hunter2', 'the copy button copies exactly once'); - // The visible way in, for anyone who never discovers the hold. + // Opening the card is its own button, never a gesture over the values. const openBtn = (q('.entry-table tbody tr') as HTMLElement).querySelector( '.entry-table-open button', ) as HTMLButtonElement; dispatch(openBtn, 'click'); - assert.ok(q('#detail-title')?.textContent?.includes('GitHub'), 'the › opens it too'); + assert.ok(q('#detail-title')?.textContent?.includes('GitHub'), 'the › opens it'); dispatch(q('[data-action="back"]'), 'click'); dispatch(q('[data-action="view-table"]'), 'click'); @@ -2307,12 +2256,10 @@ test('entry list table view: default columns, masked password, column toggling, assert.equal(attachmentsTd.firstChild?.textContent, '', 'no attachments on this entry'); assert.equal(attachmentsTd.querySelector('.copy-hint'), null, 'and so no copy hint'); clipboardText = ''; - press(attachmentsTd, 'pointerdown'); - press(attachmentsTd, 'pointerup'); + dispatch(attachmentsTd, 'click'); await Promise.resolve(); assert.equal(clipboardText, '', 'a cell with nothing in it copies nothing'); - t.mock.timers.reset(); dispatch(q('[data-action="view-tile"]'), 'click'); assert.equal(root().querySelectorAll('.entry-row').length, 2); assert.equal(root().querySelectorAll('.entry-table').length, 0); From 7e370be2651f40bcfb4d521b8af0ed63f0c606ed Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Mon, 7 Sep 2026 12:30:07 -0400 Subject: [PATCH 5/5] fix(0x67):#67:serialize the clipboard clear, label the open column --- e2e/entry-copy.test.ts | 2 +- pages/0x67/page.css | 14 +++++++++++++- pages/0x67/page.ts | 33 ++++++++++++++++++--------------- pages/tests/0x67-page.test.ts | 23 +++++++++++------------ 4 files changed, 43 insertions(+), 29 deletions(-) diff --git a/e2e/entry-copy.test.ts b/e2e/entry-copy.test.ts index 9c2cc4e..bcde9c1 100644 --- a/e2e/entry-copy.test.ts +++ b/e2e/entry-copy.test.ts @@ -65,7 +65,7 @@ async function usernameCellCentre(): Promise<{ x: number; y: number }> { test('clicking a value never opens the entry', async () => { const { x, y } = await usernameCellCentre(); await page.mouse.click(x, y); - await new Promise((resolve) => setTimeout(resolve, 300)); + // Opening is synchronous now, so there is nothing to wait out. assert.equal(await app.$('#detail-title'), null, 'the card stayed shut'); }); diff --git a/pages/0x67/page.css b/pages/0x67/page.css index 6a712b0..57cd3e3 100644 --- a/pages/0x67/page.css +++ b/pages/0x67/page.css @@ -816,7 +816,7 @@ never be what makes the affordance discoverable, since touch has none. */ cursor: pointer; font-size: 0.8em; line-height: 1; - opacity: 0.25; + opacity: 0.55; } .entry-table td:hover .copy-hint, @@ -1510,3 +1510,15 @@ body.embedded footer { color: var(--text); font-size: 0.85rem; } + +/* Announced but not drawn: gives the open column a name for table navigation. */ +.visually-hidden { + position: absolute; + width: 1px; + height: 1px; + margin: -1px; + padding: 0; + overflow: hidden; + clip-path: inset(50%); + white-space: nowrap; +} diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index 9fe1433..1d5d489 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -117,6 +117,11 @@ logic.ts) are globals from bundle-iife's concatenation; see globals.d.ts. */ let clipboardTimer: ReturnType | null = null; +/* The most recent auto-clear (#67). Every copy waits for it before writing, so +a clear can never resolve on top of a newer value and wipe it; a settled one +costs nothing to await, which is why this is never null. */ +let clipboardClear: Promise = Promise.resolve(); + const FIELD_LABELS: Record = { UserName: 'Username' }; // KeePass's own field keys are not what the rest of the app calls them (#67). @@ -134,6 +139,7 @@ function showClipboardToast(label: string): void { async function copyToClipboard(text: string, label = 'Value'): Promise { try { + await clipboardClear; await navigator.clipboard.writeText(text); if (clipboardTimer) clearTimeout(clipboardTimer); showClipboardToast(label); @@ -141,7 +147,7 @@ async function copyToClipboard(text: string, label = 'Value'): Promise { exactly when the clipboard is, rather than on a schedule of its own. */ clipboardTimer = setTimeout(() => { clipboardTimer = null; - navigator.clipboard + clipboardClear = navigator.clipboard .writeText('') .then(() => { byId('toast').hidden = true; @@ -722,7 +728,7 @@ browser already decides what counts as a click, so a scroll that starts on a cell copies nothing and a secondary button never reaches here at all. */ function wireCellCopy(cell: HTMLTableCellElement, value: string, label: string): void { cell.addEventListener('click', (event) => { - if (event.target !== cell) return; // the copy button runs its own handler + if (event.target !== event.currentTarget) return; // the copy button runs its own handler if (value) copyToClipboard(value, label); }); } @@ -759,7 +765,12 @@ function buildEntryTable(rows: EntryWithGroup[]): HTMLTableElement { th.textContent = column.label; headRow.appendChild(th); } - headRow.appendChild(document.createElement('th')); + const openTh = document.createElement('th'); + const openLabel = document.createElement('span'); + openLabel.className = 'visually-hidden'; + openLabel.textContent = 'Open'; + openTh.appendChild(openLabel); + headRow.appendChild(openTh); thead.appendChild(headRow); table.appendChild(thead); @@ -1107,12 +1118,8 @@ function buildDetailField(key: string, value: string, isProtected: boolean): HTM actions.appendChild(revealBtn); } - const copyBtn = makeIconButton('icon-btn', 'Copy', '📋', async () => { - await copyToClipboard(value, fieldLabel(key)); - copyBtn.textContent = '✓'; - setTimeout(() => { - copyBtn.textContent = '📋'; - }, 1500); + const copyBtn = makeIconButton('icon-btn', 'Copy', '📋', () => { + copyToClipboard(value, fieldLabel(key)); }); actions.appendChild(copyBtn); @@ -1337,14 +1344,10 @@ function buildEditField( row.appendChild(toggle); } - const copyBtn = makeIconButton('icon-btn', 'Copy', '📋', async () => { + const copyBtn = makeIconButton('icon-btn', 'Copy', '📋', () => { // Copies whatever is currently typed, not the value the field opened // with — the user may have already edited it. - await copyToClipboard(valueInput.value, fieldLabel(keyInput.value) || 'Value'); - copyBtn.textContent = '✓'; - setTimeout(() => { - copyBtn.textContent = '📋'; - }, 1500); + copyToClipboard(valueInput.value, fieldLabel(keyInput.value) || 'Value'); }); row.appendChild(copyBtn); diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index 7c0c088..3538cf5 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -1234,7 +1234,7 @@ test('0x67 app', async (t) => { }); await t.test( - 'copying a field writes to the clipboard, flips the icon, then reverts on a timer', + 'copying a field writes to the clipboard, announces it, and clears on a timer', (t) => { t.mock.timers.enable({ apis: ['setTimeout'] }); const usernameRow = Array.from(root().querySelectorAll('.detail-field')).find( @@ -1247,8 +1247,8 @@ test('0x67 app', async (t) => { clipboardWritesShouldFail = false; usernameRow.querySelector('[title="Copy"]')?.click(); - // The write and the icon flip both happen inside an async click handler; - // let its microtasks settle before advancing fake timers. + // The write happens inside an async handler; let its microtasks settle + // before advancing fake timers. return Promise.resolve() .then(() => Promise.resolve()) .then(() => { @@ -1261,7 +1261,6 @@ test('0x67 app', async (t) => { return Promise.resolve().then(() => Promise.resolve()); }) .then(() => { - assert.equal(copyBtn?.textContent, '✓'); // The toast names the field and never carries the value itself (#67). assert.equal(byId('toast').hidden, false); assert.equal(byId('toast').textContent, 'Password copied to clipboard'); @@ -1271,8 +1270,6 @@ test('0x67 app', async (t) => { return Promise.resolve().then(() => Promise.resolve()); }) .then(() => { - t.mock.timers.tick(1500); - assert.equal(copyBtn?.textContent, '📋'); // The clipboard-clear timer (app.clipboardTimeout, still the // default 30s here) was reset by the second copy above; advance // past it to cover the auto-clear callback itself. Force this @@ -1332,9 +1329,11 @@ test('0x67 app', async (t) => { .then(() => Promise.resolve()) .then(() => { assert.equal(clipboardText, 'not-yet-saved-password'); - assert.equal(copyBtn?.textContent, '✓'); - t.mock.timers.tick(1500); - assert.equal(copyBtn?.textContent, '📋'); + assert.equal( + byId('toast').textContent, + 'Value copied to clipboard', + 'an unnamed field still names something', + ); q('[data-action="cancel"]').dispatchEvent(new dom.window.Event('click', { bubbles: true })); }); }); @@ -2116,7 +2115,7 @@ test('entry list sorting: by title, username, or modified time, in either direct q('[data-action="close"]').dispatchEvent(new dom.window.Event('click', { bubbles: true })); }); -test('entry list table view: default columns, masked password, column toggling, and click vs double-click', async (t) => { +test('entry list table view: columns, masked password, click to copy, button to open', async (t) => { const credentials = new Credentials({ password: PASSWORD, keyFile: KEYFILE }); const kdbx = await Kdbx.create(credentials, { version: 4, @@ -2160,8 +2159,8 @@ test('entry list table view: default columns, masked password, column toggling, Array.from(root().querySelectorAll('.entry-table th')).map((th) => th.textContent ?? ''); assert.deepEqual( headerText(), - ['Title', 'Username', 'Password', 'URL', 'Modified', ''], - 'default visible columns, plus the unlabelled open-entry column', + ['Title', 'Username', 'Password', 'URL', 'Modified', 'Open'], + "default visible columns, plus the open column's screen-reader-only name", ); // The cell's own text, without the copy hint appended beside it.