diff --git a/AGENTS.md b/AGENTS.md index 5b83534..c2803ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,6 +12,8 @@ The concrete implication for an agent: don't reach for a framework, a general-pu When a design decision has more than one reasonable answer, resolve it in this order: correct operation, minimal surface area (to-the-point comments, efficient algorithms, no excess features), readable (plain language, clear names), explicit (the user does something deliberate to kick off a behavior — nothing fires as a side effect), convenient, performant. Higher wins. Don't trade a higher priority for a lower one to make a later item nicer — for example, don't add a persisted session to make something more convenient at the cost of making it less explicit, and don't reach for a shared abstraction at the cost of a larger, harder-to-audit surface area. +Effort scales with reversibility. The action a user takes most often gets the cheapest gesture, and a less reversible one always costs more — a different gesture, a separate control, or a confirmation — never the same gesture as the reversible neighbor it sits beside. Deleting already works this way: trashing a single entry is reversible and so happens silently, trashing a group asks first because it carries every entry beneath it, and emptying the bin is permanent and so is confirmed. The rule generalizes that, so a new control's weight is decided by its consequence rather than by whatever fits the layout. + ## Approach Every internal dependency is owned, not borrowed. `packages/argon2`, `packages/chacha20`, and `packages/kdbx` are consumed by relative import to each other's compiled output in `build/packages/` — never through a `dependencies` entry in any `package.json`, and never published. `grep -r '"dependencies"' --include=package.json .` should always come back empty for internal code; if a change makes it not empty, that change is wrong. diff --git a/e2e/group-rail.test.ts b/e2e/group-rail.test.ts new file mode 100644 index 0000000..b5fee3e --- /dev/null +++ b/e2e/group-rail.test.ts @@ -0,0 +1,188 @@ +/** Real-browser coverage for the group rail (issue #63). Both assertions here + * need a layout engine, which jsdom does not have: that the rail's default + * width really does show 25 characters of a sub-group name with no + * intervention, and that dragging the handle really does resize it. */ +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 { 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(); + // Comfortably wider than the 700px drawer breakpoint, so the rail is the + // resizable side rail rather than the mobile drawer. + await page.setViewport({ width: 1280, height: 900 }); + fixture = await writeKdbxFixture(); + + app = await openApp(page); +}); + +/** 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(); +}); + +test('the rail shows 25 characters of a sub-group name without any intervention', async (t) => { + const measured = await app.$$eval( + '#group-tree .group-btn', + (buttons, name) => { + const button = buttons.find((b) => b.textContent?.endsWith(name)); + if (!button) return null; + // scrollWidth is clamped to clientWidth, so it can only ever report + // "overflowing" or "not" — never by how much. Measuring the text itself + // against the content box gives a margin that can be watched over time. + const text = document.createRange(); + text.selectNodeContents(button); + const style = getComputedStyle(button); + const padding = Number.parseFloat(style.paddingLeft) + Number.parseFloat(style.paddingRight); + return { + needed: text.getBoundingClientRect().width, + available: button.clientWidth - padding, + }; + }, + fixture.groupName, + ); + + assert.ok(measured, `the rail lists "${fixture.groupName}"`); + // Reported on every run: the monospace fallback differs between developer + // machines and CI, so a shrinking margin here is the early warning that the + // default width is drifting towards truncation. + t.diagnostic( + `25-character group name needs ${measured.needed.toFixed(1)}px of the ${measured.available.toFixed(1)}px content box (${(measured.available - measured.needed).toFixed(1)}px spare)`, + ); + assert.ok( + measured.needed <= measured.available, + `"${fixture.groupName}" does not fit the default rail: needs ${measured.needed.toFixed(1)}px, has ${measured.available.toFixed(1)}px`, + ); +}); + +test('dragging the handle resizes the rail', async () => { + const railWidth = (): Promise => + app.$eval('#sidebar', (el) => el.getBoundingClientRect().width); + + const handle = await app.$('#sidebar-resize'); + assert.ok(handle, 'the rail has a resize handle'); + const box = await handle.boundingBox(); + assert.ok(box, 'the handle is laid out'); + + const startWidth = await railWidth(); + const y = box.y + 20; + await page.mouse.move(box.x + box.width / 2, y); + await page.mouse.down(); + await page.mouse.move(box.x + box.width / 2 + 80, y, { steps: 8 }); + await page.mouse.up(); + + const endWidth = await railWidth(); + assert.ok( + endWidth > startWidth, + `dragging right widened the rail (${startWidth}px -> ${endWidth}px)`, + ); +}); + +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); + + assert.equal( + await phoneApp.$eval('#sidebar-resize', (el) => getComputedStyle(el).display), + 'none', + 'the drawer has no edge to drag, so the handle is not rendered', + ); + + await phoneApp.click('[data-action="toggle-sidebar"]'); + await phoneApp.waitForSelector('#sidebar.sidebar-open'); + // The drawer slides in over 0.2s; clicking mid-flight misses the button. + await phoneApp.waitForFunction(() => { + const drawer = document.querySelector('#sidebar'); + return drawer !== null && getComputedStyle(drawer).transform === 'matrix(1, 0, 0, 1, 0, 0)'; + }); + + // Every drawer row exposes its ⋯, because tapping a group to make it active + // would close the drawer and cost a second visit. + const menuButton = await phoneApp.evaluateHandle((name) => { + const rows = Array.from(document.querySelectorAll('#group-tree .group-row')); + const row = rows.find((r) => r.querySelector('.group-btn')?.textContent?.endsWith(name)); + return row?.querySelector('.group-menu-btn') ?? null; + }, fixture.groupName); + const menuElement = menuButton.asElement() as ElementHandle | null; + assert.ok(menuElement, 'the sub-group row has a ⋯ button in the drawer'); + assert.notEqual( + await menuElement.evaluate((el) => getComputedStyle(el).visibility), + 'hidden', + '⋯ is visible without first selecting the row', + ); + + await menuElement.click(); + const labels = await phoneApp.$$eval('.group-menu-item', (items) => + items.map((i) => i.textContent), + ); + assert.deepEqual(labels, ['Rename', 'Move'], 'the menu opens with rename and move'); + + assert.ok(await phoneApp.$('#sidebar.sidebar-open'), 'opening the menu left the drawer open'); + await phone.close(); +}); + +test('a rail widened on desktop does not follow the user into the phone drawer', async () => { + const handle = await app.$('#sidebar-resize'); + assert.ok(handle, 'the rail has a resize handle'); + const box = await handle.boundingBox(); + assert.ok(box, 'the handle is laid out'); + + const y = box.y + 20; + await page.mouse.move(box.x + box.width / 2, y); + await page.mouse.down(); + await page.mouse.move(box.x + box.width / 2 + 400, y, { steps: 8 }); + await page.mouse.up(); + + const railWidth = (): Promise => + app.$eval('#sidebar', (el) => el.getBoundingClientRect().width); + const wide = await railWidth(); + assert.ok(wide > 400, `the rail is dragged wide first (${wide}px)`); + + await page.setViewport({ width: 375, height: 812 }); + const drawer = await railWidth(); + assert.ok( + drawer <= 375, + `the drawer keeps its own width at phone size (${drawer}px inside a 375px viewport)`, + ); + + await page.setViewport({ width: 1280, height: 900 }); +}); diff --git a/e2e/support/fixture.ts b/e2e/support/fixture.ts index 9e4060d..9ae5136 100644 --- a/e2e/support/fixture.ts +++ b/e2e/support/fixture.ts @@ -3,7 +3,13 @@ import { writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { appendChild, Credentials, createEntry, Kdbx } from '../../packages/kdbx/src/index.ts'; +import { + appendChild, + Credentials, + createEntry, + createGroup, + Kdbx, +} from '../../packages/kdbx/src/index.ts'; // Fast KDF settings (matches pages/tests/*.test.ts) — a throwaway fixture, // no reason to pay real Argon2id cost. @@ -13,11 +19,14 @@ export interface KdbxFixture { path: string; password: string; entryTitle: string; + groupName: string; } export async function writeKdbxFixture(): Promise { const password = 'e2e-test-password'; const entryTitle = 'Example Entry'; + // Exactly 25 characters, the floor issue #63 sets for the group rail. + const groupName = 'Financial Institutions XY'; const credentials = new Credentials({ password }); const kdbx = await Kdbx.create(credentials, { @@ -32,6 +41,7 @@ export async function writeKdbxFixture(): Promise { kdbx.getRootGroup(), createEntry({ title: entryTitle, username: 'octocat', password: 'hunter2' }), ); + appendChild(kdbx.getRootGroup(), createGroup(groupName)); const bytes = await kdbx.save(); const path = join( @@ -40,5 +50,5 @@ export async function writeKdbxFixture(): Promise { ); await writeFile(path, bytes); - return { path, password, entryTitle }; + return { path, password, entryTitle, groupName }; } diff --git a/pages/0x67/page.css b/pages/0x67/page.css index e967b43..3fceb96 100644 --- a/pages/0x67/page.css +++ b/pages/0x67/page.css @@ -36,7 +36,8 @@ --warning-bg: #fbf0e7; --warning-border: #ead7c2; --success: #0e7c5a; - --sidebar-width: 210px; + /* 25 chars of a first-level sub-group (#63) at .group-btn's size, plus icon, padding, nesting, and the ⋯ slot. */ + --sidebar-width: 280px; } body { @@ -470,6 +471,21 @@ through to notice unsaved edits, unlike every other dirty-aware prompt. */ overflow: hidden; } +/* A flex sibling of the rail, not an overlay on it (#63): the tree scrolls, and +an overlaid handle would sit on top of its scrollbar. */ +.sidebar-resize { + flex: 0 0 6px; + margin: 0; + border: none; + cursor: col-resize; + touch-action: none; /* a drag here resizes (#63); scrolling must not claim it */ +} + +.sidebar-resize:hover, +.sidebar-resize:focus-visible { + background: var(--accent-dim); +} + .sidebar-header { display: flex; align-items: center; @@ -479,6 +495,11 @@ through to notice unsaved edits, unlike every other dirty-aware prompt. */ flex-shrink: 0; } +.sidebar-header-actions { + display: flex; + gap: 0.15rem; +} + .sidebar-label { font-size: 0.75rem; font-weight: 600; @@ -508,14 +529,41 @@ through to notice unsaved edits, unlike every other dirty-aware prompt. */ gap: 0.1rem; } -.group-actions { - display: flex; +/* Reserved on every row, shown only on the active one (#63), so selecting a +group doesn't reflow its name. */ +.group-menu-btn { flex-shrink: 0; -} - -.group-action-btn { padding: 0.2rem 0.3rem; font-size: 0.8rem; + visibility: hidden; +} + +.group-row-active .group-menu-btn { + visibility: visible; +} + +/* Inline, not a floating popup (#63): the rail and the tree both clip overflow, +so an absolutely positioned menu would be cut off. */ +.group-menu { + display: flex; + gap: 0.25rem; + padding: 0.15rem 0 0.35rem 1rem; +} + +.group-menu-item { + background: none; + border: 1px solid var(--border); + border-radius: 4px; + color: var(--text); + cursor: pointer; + font-family: inherit; + font-size: 0.75rem; + padding: 0.2rem 0.5rem; + white-space: nowrap; +} + +.group-menu-item:hover { + background: var(--surface-2); } .group-btn { @@ -823,6 +871,16 @@ through to notice unsaved edits, unlike every other dirty-aware prompt. */ transition: transform 0.2s ease; } + .sidebar-resize { + display: none; + } + + /* Tapping a group closes the drawer (#63), so gating ⋯ on the active row + would put rename and move two drawer visits away; show it on every row. */ + .group-menu-btn { + visibility: visible; + } + .sidebar.sidebar-open { transform: translateX(0); } diff --git a/pages/0x67/page.html b/pages/0x67/page.html index e7a8052..4ac58e8 100644 --- a/pages/0x67/page.html +++ b/pages/0x67/page.html @@ -116,10 +116,14 @@

New database

+
@@ -143,7 +147,7 @@

New database

- +
diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index 1839162..d42ca04 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -320,31 +320,108 @@ function setSidebarOpen(open: boolean): void { qs('#sidebar-backdrop').hidden = !open; } +const SIDEBAR_WIDTH_MIN = 180; +const SIDEBAR_WIDTH_MAX = 520; +const SIDEBAR_WIDTH_STEP = 16; + +/* Null until the user adjusts the handle, so the rail opens at the CSS default +that fits 25 characters (#63); the width stays in memory because one silently +restored forever would be implicit state, yet it must survive a re-render. */ +let sidebarWidth: number | null = null; + +/* Sets the custom property rather than an inline width (#63); an inline width +outranks the drawer's own rule, so a rail widened on a desktop stayed that wide +once the viewport narrowed. */ +function setSidebarWidth(px: number): void { + sidebarWidth = Math.min(SIDEBAR_WIDTH_MAX, Math.max(SIDEBAR_WIDTH_MIN, Math.round(px))); + document.documentElement.style.setProperty('--sidebar-width', `${sidebarWidth}px`); + qs('#sidebar-resize').setAttribute('aria-valuenow', String(sidebarWidth)); +} + +// The rail as rendered until the user picks a width; their choice after that (#63). +function currentSidebarWidth(): number { + return sidebarWidth ?? qs('#sidebar').getBoundingClientRect().width; +} + +/* Pointer events rather than mouse events (#63): one path covers mouse, touch +and pen, so the rail is resizable wherever it is visible. */ +function wireSidebarResize(): void { + const handle = qs('#sidebar-resize'); + handle.setAttribute('aria-valuemin', String(SIDEBAR_WIDTH_MIN)); + handle.setAttribute('aria-valuemax', String(SIDEBAR_WIDTH_MAX)); + if (sidebarWidth !== null) setSidebarWidth(sidebarWidth); // resync the fresh handle's aria + + handle.addEventListener('pointerdown', (down) => { + down.preventDefault(); + handle.setPointerCapture(down.pointerId); + const startX = down.clientX; + const startWidth = currentSidebarWidth(); + + const onMove = (move: PointerEvent) => setSidebarWidth(startWidth + move.clientX - startX); + const onDone = () => { + handle.releasePointerCapture(down.pointerId); + handle.removeEventListener('pointermove', onMove); + handle.removeEventListener('pointerup', onDone); + handle.removeEventListener('pointercancel', onDone); + }; + + handle.addEventListener('pointermove', onMove); + handle.addEventListener('pointerup', onDone); + handle.addEventListener('pointercancel', onDone); + }); + + handle.addEventListener('keydown', (key) => { + if (key.key !== 'ArrowLeft' && key.key !== 'ArrowRight') return; + key.preventDefault(); + const step = key.key === 'ArrowLeft' ? -SIDEBAR_WIDTH_STEP : SIDEBAR_WIDTH_STEP; + setSidebarWidth(currentSidebarWidth() + step); + }); +} + +/* The group whose ⋯ menu is open, not a flag (#63): the drawer shows ⋯ on every +row, so the menu belongs to a row rather than to the selection. */ +let groupMenuFor: XmlElement | null = null; + function renderGroupTree(): void { const container = qs('#group-tree'); container.innerHTML = ''; + const rootGroup = must(app.db).getRootGroup(); const ul = document.createElement('ul'); ul.className = 'group-list'; - ul.appendChild(buildGroupNode(must(app.db).getRootGroup(), true)); + ul.appendChild(buildGroupNode(rootGroup, true)); container.appendChild(ul); + // Deleting acts on the selection (#63); the root group is the database itself. + const selected = must(app.currentGroup); + qs('#delete-group-btn').disabled = + selected === rootGroup || isRecycleBinGroup(selected); +} + +/* Opening ⋯ selects the row too (#63); the drawer shows ⋯ on every row, so +without this the header's delete could act on a different group than the menu +the user is looking at. */ +function selectGroup(group: XmlElement): void { + app.currentGroup = group; + app.searchQuery = ''; + const searchInput = document.querySelector('#search-input'); + if (searchInput) searchInput.value = ''; + renderGroupTree(); + renderEntryPanel(); } function buildGroupNode(group: XmlElement, isRoot: boolean): HTMLLIElement { const li = document.createElement('li'); const row = document.createElement('div'); - row.className = 'group-row'; + const isActive = group === app.currentGroup; + row.className = `group-row${isActive ? ' group-row-active' : ''}`; const btn = document.createElement('button'); btn.type = 'button'; - btn.className = `group-btn${group === app.currentGroup ? ' active' : ''}`; + btn.className = `group-btn${isActive ? ' active' : ''}`; btn.textContent = `${iconEmoji(elementIconId(group))} ${groupName(group)}`; + btn.title = groupName(group); // the rail truncates; hover still gives the whole name (#63) btn.addEventListener('click', () => { - app.currentGroup = group; - app.searchQuery = ''; - const searchInput = document.querySelector('#search-input'); - if (searchInput) searchInput.value = ''; - renderGroupTree(); - renderEntryPanel(); + groupMenuFor = null; + selectGroup(group); setSidebarOpen(false); }); row.appendChild(btn); @@ -353,11 +430,20 @@ function buildGroupNode(group: XmlElement, isRoot: boolean): HTMLLIElement { // settings (its master password/name), not as an ordinary group, and // neither movable nor deletable. if (!isRoot) { - row.appendChild(buildGroupActions(group)); + row.appendChild( + makeIconButton('icon-btn group-menu-btn', 'Group actions', '⋯', () => { + groupMenuFor = groupMenuFor === group ? null : group; + selectGroup(group); + }), + ); } li.appendChild(row); + if (!isRoot && groupMenuFor === group) { + li.appendChild(buildGroupMenu(group)); + } + const subgroups = getChildren(group, 'Group'); if (subgroups.length > 0) { const ul = document.createElement('ul'); @@ -370,33 +456,69 @@ function buildGroupNode(group: XmlElement, isRoot: boolean): HTMLLIElement { return li; } -function buildGroupActions(group: XmlElement): HTMLDivElement { - const actions = document.createElement('div'); - actions.className = 'group-actions'; - - const renameBtn = makeIconButton('icon-btn group-action-btn', 'Rename group', '✏️', () => { - openGroupDialog({ type: 'rename', group }, () => { - renderGroupTree(); - renderEntryPanel(); - }); +function makeMenuItem(label: string, onClick: () => void): HTMLButtonElement { + const btn = document.createElement('button'); + btn.type = 'button'; + btn.className = 'group-menu-item'; + btn.textContent = label; + // Closing here, not in each action, also covers a dialog the user cancels (#63). + btn.addEventListener('click', () => { + groupMenuFor = null; + renderGroupTree(); + onClick(); }); - actions.appendChild(renameBtn); + return btn; +} - const moveBtn = makeIconButton('icon-btn group-action-btn', 'Move group', '📂', () => { - openMoveToDialog( - 'Move group to…', - (candidate) => !isDescendantGroup(group, candidate), - (destination) => moveGroupTo(group, destination), - ); - }); - actions.appendChild(moveBtn); +function buildGroupMenu(group: XmlElement): HTMLDivElement { + const menu = document.createElement('div'); + menu.className = 'group-menu'; - const deleteBtn = makeIconButton('icon-btn group-action-btn', 'Delete group', '🗑', () => { - deleteGroupAction(group); - }); - actions.appendChild(deleteBtn); + menu.appendChild( + makeMenuItem('Rename', () => { + openGroupDialog({ type: 'rename', group }, () => { + renderGroupTree(); + renderEntryPanel(); + }); + }), + ); + + menu.appendChild( + makeMenuItem('Move', () => { + openMoveToDialog( + 'Move group to…', + (candidate) => !isDescendantGroup(group, candidate), + (destination) => moveGroupTo(group, destination), + ); + }), + ); + + if (isTrashedGroup(group)) { + menu.appendChild(makeMenuItem('Undelete', () => undeleteGroup(group))); + } + + return menu; +} - return actions; +/* The bin reports itself as inside itself (#63), so deleting it would take the +permanent branch and destroy every trashed item; it is never an ordinary +delete target. */ +function isRecycleBinGroup(group: XmlElement): boolean { + return isInRecycleBin(must(app.db).root, group) && !isTrashedGroup(group); +} + +/* Tests the parent, not the group (#63): isInRecycleBin counts the bin as +containing itself, which would offer the bin an Undelete of its own. */ +function isTrashedGroup(group: XmlElement): boolean { + const db = must(app.db); + const parent = findGroupParent(db.getRootGroup(), group); + return parent !== null && isInRecycleBin(db.root, parent); +} + +/* Restores to the root group (#63); KDBX records no previous location, so this +matches where a trashed entry is restored to. */ +function undeleteGroup(group: XmlElement): void { + moveGroupTo(group, must(app.db).getRootGroup()); } /** Deselect (back to root) if the current selection is `group` or nested @@ -422,6 +544,10 @@ function moveGroupTo(group: XmlElement, destination: XmlElement): void { } function deleteGroupAction(group: XmlElement): void { + // The row is about to move or vanish; its menu should not travel with it (#63). + groupMenuFor = null; + renderGroupTree(); + const db = must(app.db); const rootGroup = db.getRootGroup(); @@ -446,11 +572,20 @@ function deleteGroupAction(group: XmlElement): void { return; } - // Outside the bin, deleting a group is "Trash" — reversible, so (like - // trashing an entry) it needs no confirmation. - const bin = findOrCreateRecycleBin(db.root); - resetSelectionIfAffected(rootGroup, group); - moveGroupTo(group, bin); + /* Trashing is reversible, but a group carries its whole subtree with it, so + it asks first (#63) where a single entry does not. */ + openConfirmDelete( + 'Move to Recycle Bin?', + `"${groupName(group)}" and everything in it can be restored from the bin.`, + () => { + // Created inside the callback so cancelling leaves no empty bin behind. + const bin = findOrCreateRecycleBin(db.root); + resetSelectionIfAffected(rootGroup, group); + moveGroupTo(group, bin); + }, + 'Move to Bin', + false, + ); } function renderEntryPanel(): void { @@ -739,6 +874,12 @@ function wireEntryListEvents(): void { openGroupDialog({ type: 'create', parent: must(app.currentGroup) }, () => showEntryList()); }); + qs('[data-action="delete-group"]').addEventListener('click', () => { + deleteGroupAction(must(app.currentGroup)); + }); + + wireSidebarResize(); + qs('[data-action="view-tile"]').addEventListener('click', () => { app.entryView = 'tile'; updateViewToggleUI(); @@ -1522,12 +1663,22 @@ function confirmUnsavedChanges(prompt: UnsavedChangesPrompt, proceed: () => void // Dialog: Confirm Delete // ============================================================ -function openConfirmDelete(title: string, message: string, callback: () => void): void { +function openConfirmDelete( + title: string, + message: string, + callback: () => void, + confirmLabel = 'Delete', + danger = true, +): void { const dlg = byId('dlg-confirm-delete'); byId('confirm-delete-title').textContent = title; byId('confirm-delete-message').textContent = message; - must(dlg.querySelector('[data-action="confirm-delete"]')).onclick = () => { + const confirmBtn = must(dlg.querySelector('[data-action="confirm-delete"]')); + confirmBtn.textContent = confirmLabel; + confirmBtn.className = danger ? 'btn btn-danger' : 'btn btn-primary'; + + confirmBtn.onclick = () => { dlg.close(); callback(); }; diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index 950ad37..957c4be 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -11,13 +11,15 @@ * exercise it exclusively through the real page.html markup and dispatched * events, not by importing its internals directly. * - * Two real, upstream gaps this file works around rather than pretends don't - * exist: jsdom does not implement HTMLDialogElement's showModal()/close() - * (tracked upstream: https://github.com/jsdom/jsdom/issues/3294, still open - * as of the jsdom version pinned here) or the Clipboard API at all. Both are - * given minimal, behavior-only polyfills below — open/close state and a - * writable clipboard buffer, nothing about real focus-trapping or OS - * clipboard access, which page.ts doesn't rely on anyway. + * Three real, upstream gaps this file works around rather than pretends + * don't exist: jsdom does not implement HTMLDialogElement's + * showModal()/close() (tracked upstream: + * https://github.com/jsdom/jsdom/issues/3294, still open as of the jsdom + * version pinned here), the Clipboard API, or pointer capture. All three are + * given minimal, behavior-only polyfills below — open/close state, a + * writable clipboard buffer, and no-op capture — nothing about real + * focus-trapping, OS clipboard access or pointer routing, none of which + * page.ts relies on. */ import assert from 'node:assert/strict'; @@ -132,6 +134,10 @@ let clipboardWritesShouldFail = false; }, }; +// --- Pointer capture polyfill (see file header) --- +dom.window.HTMLElement.prototype.setPointerCapture = () => {}; +dom.window.HTMLElement.prototype.releasePointerCapture = () => {}; + // --- Hoist the kdbx library and this page's own pure logic onto globalThis, // --- exactly like bundle.js does in the real browser build (see // --- bundle-iife.json's "exports" list, which this mirrors exactly). --- @@ -591,10 +597,10 @@ test('0x67 app', async (t) => { await t.test( 'group tree: rename, move (with own-subtree blocked), and delete/permanently-delete', () => { - // Root has no rename/move/delete actions of its own. + // Root has no actions of its own, so it gets no ⋯ at all. const rootBtn = (): HTMLButtonElement => q('#group-tree').querySelector('.group-btn') as HTMLButtonElement; - assert.equal(rootBtn().closest('.group-row')?.querySelector('.group-actions'), null); + assert.equal(rootBtn().closest('.group-row')?.querySelector('.group-menu-btn'), null); // Group rows are icon-prefixed ("🌐 Personal"), and the root's own // label ("📁 Personal Vault") would falsely match a plain substring @@ -605,21 +611,40 @@ test('0x67 app', async (t) => { ) as HTMLButtonElement; const rowFor = (name: string): HTMLElement => groupBtnFor(name).closest('.group-row') as HTMLElement; - const actionBtn = (name: string, title: string): HTMLButtonElement => - rowFor(name).querySelector(`[title="${title}"]`) as HTMLButtonElement; + const liFor = (name: string): HTMLElement => rowFor(name).closest('li') as HTMLElement; // The tree is always fully expanded, so a trashed/moved group's button // never disappears from #group-tree — it just relocates. Check its new // position (is it inside this parent's own subtree?) rather than mere // presence/absence. const isInSubtreeOf = (parentName: string, childName: string): boolean => Array.from( - rowFor(parentName).nextElementSibling?.querySelectorAll( - '.group-btn', - ) ?? [], + liFor(parentName) + .querySelector(':scope > ul') + ?.querySelectorAll('.group-btn') ?? [], ).some((b) => b.textContent?.endsWith(childName)); const click = (el: EventTarget): void => { el.dispatchEvent(new dom.window.Event('click', { bubbles: true })); }; + const menuItem = (name: string, label: string): HTMLButtonElement => { + click(groupBtnFor(name)); // only the active row reveals its ⋯ + click(rowFor(name).querySelector('.group-menu-btn') as HTMLButtonElement); + return Array.from( + liFor(name).querySelectorAll(':scope > .group-menu .group-menu-item'), + ).find((b) => b.textContent === label) as HTMLButtonElement; + }; + const clickHeaderDelete = (name: string): void => { + click(groupBtnFor(name)); + click(q('[data-action="delete-group"]')); + }; + const confirmDelete = (): void => + click(dq('#dlg-confirm-delete [data-action="confirm-delete"]')); + const menuLabels = (name: string): string[] => { + click(groupBtnFor(name)); + click(rowFor(name).querySelector('.group-menu-btn') as HTMLButtonElement); + return Array.from( + liFor(name).querySelectorAll(':scope > .group-menu .group-menu-item'), + ).map((b) => b.textContent ?? ''); + }; const addRootGroup = (name: string): void => { click(rootBtn()); click(q('[data-action="add-group"]')); @@ -633,7 +658,7 @@ test('0x67 app', async (t) => { addRootGroup('Move Target'); // --- rename: reuses dlg-new-group, retitled and prefilled --- - click(actionBtn('Rename Target', 'Rename group')); + click(menuItem('Rename Target', 'Rename')); const groupDlg = byId('dlg-new-group'); assert.equal(byId('new-group-title').textContent, 'Rename group'); assert.equal(byId('new-group-name').value, 'Rename Target'); @@ -642,9 +667,33 @@ test('0x67 app', async (t) => { assert.equal(groupDlg.open, false); assert.ok(groupBtnFor('Renamed Group')); assert.equal(groupBtnFor('Rename Target'), undefined); + assert.equal( + groupBtnFor('Renamed Group').title, + 'Renamed Group', + 'the full name is on hover, since the rail truncates', + ); + + // --- ⋯ toggles: pressing it again on the same row puts the menu away --- + const openMenu = (name: string): Element | null => + liFor(name).querySelector(':scope > .group-menu'); + const menuBtnFor = (name: string): HTMLButtonElement => + rowFor(name).querySelector('.group-menu-btn') as HTMLButtonElement; + click(groupBtnFor('Renamed Group')); + click(menuBtnFor('Renamed Group')); + assert.ok(openMenu('Renamed Group'), 'first press opens the menu'); + click(menuBtnFor('Renamed Group')); + assert.equal(openMenu('Renamed Group'), null, 'second press closes it'); + + // --- ⋯ claims the selection, so header delete cannot target another row --- + click(groupBtnFor('Personal')); + click(menuBtnFor('Work')); + assert.ok( + groupBtnFor('Work').classList.contains('active'), + 'opening ⋯ on a row makes that row the selection', + ); // --- move: a group cannot be moved into itself --- - click(actionBtn('Move Target', 'Move group')); + click(menuItem('Move Target', 'Move')); const moveDlg = byId('dlg-move-to'); assert.equal(moveDlg.open, true); assert.equal(byId('move-to-title').textContent, 'Move group to…'); @@ -670,19 +719,47 @@ test('0x67 app', async (t) => { click(q('[data-action="back"]')); // --- move: a descendant is also an invalid target, and cancel/close both leave it in place --- - click(actionBtn('Renamed Group', 'Move group')); + click(menuItem('Renamed Group', 'Move')); assert.equal(destBtn('Renamed Group').disabled, true, 'self is not a valid target'); assert.equal(destBtn('Move Target').disabled, true, 'own descendant is not a valid target'); click(dq('#dlg-move-to [data-action="close"]')); assert.equal(moveDlg.open, false); - click(actionBtn('Renamed Group', 'Move group')); + click(menuItem('Renamed Group', 'Move')); click(dq('#dlg-move-to [data-action="cancel-move"]')); assert.equal(moveDlg.open, false); + assert.equal( + liFor('Renamed Group').querySelector(':scope > .group-menu'), + null, + 'picking a menu item closes the menu even when the dialog is cancelled', + ); + + // --- a row's menu does not survive that row being deleted --- + click(menuBtnFor('Renamed Group')); + assert.ok(openMenu('Renamed Group'), 'menu is open before deleting'); + click(q('[data-action="delete-group"]')); + assert.equal(openMenu('Renamed Group'), null, 'deleting closes the row menu'); + click(dq('#dlg-confirm-delete [data-action="cancel-delete"]')); - // --- delete outside the bin: no confirmation, moves into Recycle Bin --- - click(actionBtn('Renamed Group', 'Delete group')); - assert.equal(byId('dlg-confirm-delete').open, false); + // --- delete outside the bin: confirmed, then moves into Recycle Bin --- + clickHeaderDelete('Renamed Group'); + const confirmDlg = byId('dlg-confirm-delete'); + assert.equal(confirmDlg.open, true, 'trashing a group asks first'); + assert.equal( + byId('confirm-delete-title').textContent, + 'Move to Recycle Bin?', + 'the reversible path does not read like the permanent one', + ); + assert.match(byId('confirm-delete-message').textContent ?? '', /restored/); + const confirmBtn = (): HTMLButtonElement => + dq('#dlg-confirm-delete [data-action="confirm-delete"]'); + assert.equal(confirmBtn().textContent, 'Move to Bin', 'not a red Delete on a reversible act'); + assert.equal(confirmBtn().classList.contains('btn-danger'), false); + click(dq('#dlg-confirm-delete [data-action="cancel-delete"]')); + assert.equal(groupBtnFor('Recycle Bin'), undefined, 'cancelling creates no empty bin'); + + clickHeaderDelete('Renamed Group'); + confirmDelete(); assert.ok(groupBtnFor('Recycle Bin'), 'the recycle bin group is created on first trash'); assert.ok( isInSubtreeOf('Recycle Bin', 'Renamed Group'), @@ -693,15 +770,36 @@ test('0x67 app', async (t) => { 'its own subtree moved along with it', ); + // --- undelete: offered only for a trashed group, and restores to root --- + assert.deepEqual(menuLabels('Personal'), ['Rename', 'Move'], 'a live group has no undelete'); + assert.deepEqual(menuLabels('Renamed Group'), ['Rename', 'Move', 'Undelete']); + assert.deepEqual(menuLabels('Recycle Bin'), ['Rename', 'Move'], 'the bin is not in itself'); + assert.equal( + q('[data-action="delete-group"]').disabled, + true, + 'the bin cannot be deleted: that would take the permanent path and empty it', + ); + click(menuItem('Renamed Group', 'Undelete')); + assert.equal( + isInSubtreeOf('Recycle Bin', 'Renamed Group'), + false, + 'undelete pulls the group back out of the bin', + ); + + // Back into the bin, so the permanent-delete case below has its subject. + clickHeaderDelete('Renamed Group'); + confirmDelete(); + // --- delete inside the bin: confirmed, permanent, whole subtree --- - click(actionBtn('Renamed Group', 'Delete group')); - const confirmDlg = byId('dlg-confirm-delete'); + clickHeaderDelete('Renamed Group'); assert.equal(confirmDlg.open, true); assert.equal(byId('confirm-delete-title').textContent, 'Delete group?'); + assert.equal(confirmBtn().textContent, 'Delete', 'the permanent path keeps its red Delete'); + assert.equal(confirmBtn().classList.contains('btn-danger'), true); // The 1 entry saved into "Move Target" earlier exercises the singular // wording; the Recycle Bin test elsewhere covers the plural case. assert.match(byId('confirm-delete-message').textContent ?? '', /\b1 entry\b/); - click(dq('#dlg-confirm-delete [data-action="confirm-delete"]')); + confirmDelete(); assert.equal(confirmDlg.open, false); assert.equal(groupBtnFor('Renamed Group'), undefined, 'permanently gone'); assert.equal(groupBtnFor('Move Target'), undefined, 'its subtree went with it'); @@ -713,6 +811,48 @@ test('0x67 app', async (t) => { }, ); + await t.test('rail resize: pointer drag and arrow keys, clamped, re-applied on render', () => { + const handle = (): HTMLElement => byId('sidebar-resize'); + const railWidth = (): string => + dom.window.document.documentElement.style.getPropertyValue('--sidebar-width'); + + // jsdom has no layout engine, so getBoundingClientRect() is all zeros and + // every drag here starts from a 0-width rail. That still exercises the + // arithmetic and the clamps; the real geometry — that the default width + // actually fits 25 characters — is asserted by the e2e suite, in a browser + // that has layout. + dispatch(handle(), 'pointerdown', { clientX: 100, pointerId: 1 }); + dispatch(handle(), 'pointermove', { clientX: 400, pointerId: 1 }); + assert.equal(railWidth(), '300px'); + assert.equal(handle().getAttribute('aria-valuenow'), '300'); + + dispatch(handle(), 'pointermove', { clientX: 0, pointerId: 1 }); + assert.equal(railWidth(), '180px', 'clamped at the narrow end'); + dispatch(handle(), 'pointermove', { clientX: 9000, pointerId: 1 }); + assert.equal(railWidth(), '520px', 'clamped at the wide end'); + + dispatch(handle(), 'pointerup', { clientX: 9000, pointerId: 1 }); + dispatch(handle(), 'pointermove', { clientX: 250, pointerId: 1 }); + assert.equal(railWidth(), '520px', 'releasing stops tracking the pointer'); + + // Arrows step from the width already chosen rather than from the layout + // engine, so direction is provable here even without one. + dispatch(handle(), 'keydown', { key: 'ArrowLeft' }); + assert.equal(railWidth(), '504px', 'one step narrower than the 520 just dragged to'); + dispatch(handle(), 'keydown', { key: 'a' }); + assert.equal(railWidth(), '504px', 'unchanged by a non-arrow key'); + dispatch(handle(), 'keydown', { key: 'ArrowRight' }); + assert.equal(railWidth(), '520px', 'one step wider, back at the ceiling'); + + // The custom property outlives a re-render, but the handle carrying the + // aria value does not, so the fresh one has to be resynced from memory. + dispatch(q('[data-action="add-group"]'), 'click'); + byId('new-group-name').value = 'Width Survivor'; + dispatch(dq('#dlg-new-group [data-action="create-group"]'), 'click'); + assert.equal(railWidth(), '520px'); + assert.equal(handle().getAttribute('aria-valuenow'), '520', 'the new handle is resynced'); + }); + await t.test('adding a new entry opens the edit screen, prefilled with standard fields', () => { q('[data-action="add-entry"]').dispatchEvent(new dom.window.Event('click', { bubbles: true })); assert.equal(q('#edit-title').textContent, 'New Entry');