From b8da63a3658493e6be2a885dfa217b5d50ac4e5d Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Sat, 5 Sep 2026 14:54:50 -0400 Subject: [PATCH 1/7] feat:group navigation improvements Closes #63. The rail showed roughly 11 characters of a first-level sub-group: three inline action buttons ate the width, and the rail was a fixed 210px. - Default rail is 280px, sized from the 25-character floor the issue sets. Since page.css is monospace, that width is arithmetic, not taste; the e2e test measures the rendered text against the content box and reports the margin on every run, so a font change that erodes it shows up before it truncates. - Rename and move move behind a single per-row menu, revealed on the active row. The menu is inline rather than a floating popup because the rail and the tree both clip overflow. In the drawer layout every row shows its menu button, since tapping a group closes the drawer and would otherwise put rename two visits away. - Delete leaves the row for the rail header, acting on the selection, so the one irreversible action here is not a gesture away from rename. - The rail is resizable by a keyboard-operable separator, driven by pointer events so mouse, touch and pen share one path. The chosen width lives in memory for the session and is deliberately not persisted. AGENTS.md gains the principle these follow: effort scales with reversibility. It describes what the code already did for trash vs. permanent delete; writing it down makes it bind future changes. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 2 + e2e/group-rail.test.ts | 161 ++++++++++++++++++++++++++++++++++ e2e/support/fixture.ts | 14 ++- pages/0x67/page.css | 72 +++++++++++++-- pages/0x67/page.html | 6 +- pages/0x67/page.ts | 136 ++++++++++++++++++++++------ pages/tests/0x67-page.test.ts | 111 ++++++++++++++++++----- 7 files changed, 446 insertions(+), 56 deletions(-) create mode 100644 e2e/group-rail.test.ts diff --git a/AGENTS.md b/AGENTS.md index 5b83534..c81c727 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 neighbour it sits beside. Deleting already works this way: trashing is reversible and so happens silently, while 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..e25c214 --- /dev/null +++ b/e2e/group-rail.test.ts @@ -0,0 +1,161 @@ +/** 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(); +}); 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..a8eeb01 100644 --- a/pages/0x67/page.css +++ b/pages/0x67/page.css @@ -36,7 +36,10 @@ --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; + --sidebar-width-min: 180px; + --sidebar-width-max: 520px; } body { @@ -470,6 +473,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: 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; it must not be claimed by scrolling */ +} + +.sidebar-resize:hover, +.sidebar-resize:focus-visible { + background: var(--accent-dim); +} + .sidebar-header { display: flex; align-items: center; @@ -479,6 +497,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 +531,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, 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: 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 +873,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, so gating ⋯ on the active row would + put rename and move two drawer visits away — show it on every row here. */ + .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..296c506 100644 --- a/pages/0x67/page.html +++ b/pages/0x67/page.html @@ -116,10 +116,14 @@

New database

+
diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index 1839162..f901767 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -320,27 +320,86 @@ 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 drags or arrows the handle, so the rail opens at the CSS +default — wide enough for 25 characters (#63). Kept in memory only: a width +chosen once and silently restored forever is exactly the implicit state the +project avoids, but it does have to survive re-rendering the screen. */ +let sidebarWidth: number | null = null; + +function setSidebarWidth(px: number): void { + sidebarWidth = Math.min(SIDEBAR_WIDTH_MAX, Math.max(SIDEBAR_WIDTH_MIN, Math.round(px))); + qs('#sidebar').style.width = `${sidebarWidth}px`; + qs('#sidebar-resize').setAttribute('aria-valuenow', String(sidebarWidth)); +} + +/* Pointer events rather than mouse events: one path covers mouse, touch and +pen, so the rail is resizable wherever it is visible. */ +function wireSidebarResize(): void { + const handle = qs('#sidebar-resize'); + if (sidebarWidth !== null) setSidebarWidth(sidebarWidth); + + handle.addEventListener('pointerdown', (down) => { + down.preventDefault(); + handle.setPointerCapture(down.pointerId); + const startX = down.clientX; + const startWidth = qs('#sidebar').getBoundingClientRect().width; + + 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(qs('#sidebar').getBoundingClientRect().width + step); + }); +} + +/* The group whose ⋯ menu is open, not a flag: the drawer layout shows ⋯ on +every row, so the menu has to belong to a row rather than to the selection. +Reset whenever the selection moves. */ +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, and the root group is the database itself. + qs('#delete-group-btn').disabled = app.currentGroup === rootGroup; } 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.addEventListener('click', () => { app.currentGroup = group; app.searchQuery = ''; + groupMenuFor = null; const searchInput = document.querySelector('#search-input'); if (searchInput) searchInput.value = ''; renderGroupTree(); @@ -353,11 +412,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; + renderGroupTree(); + }), + ); } 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 +438,41 @@ function buildGroupNode(group: XmlElement, isRoot: boolean): HTMLLIElement { return li; } -function buildGroupActions(group: XmlElement): HTMLDivElement { - const actions = document.createElement('div'); - actions.className = 'group-actions'; +function makeMenuItem(label: string, onClick: () => void): HTMLButtonElement { + const btn = document.createElement('button'); + btn.type = 'button'; + btn.className = 'group-menu-item'; + btn.textContent = label; + btn.addEventListener('click', onClick); + return btn; +} - const renameBtn = makeIconButton('icon-btn group-action-btn', 'Rename group', '✏️', () => { - openGroupDialog({ type: 'rename', group }, () => { - renderGroupTree(); - renderEntryPanel(); - }); - }); - actions.appendChild(renameBtn); +/* Deleting is deliberately absent: it is the one irreversible action here, so +it lives in the rail header rather than a gesture away from rename. */ +function buildGroupMenu(group: XmlElement): HTMLDivElement { + const menu = document.createElement('div'); + menu.className = 'group-menu'; - 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); + menu.appendChild( + makeMenuItem('Rename', () => { + openGroupDialog({ type: 'rename', group }, () => { + renderGroupTree(); + renderEntryPanel(); + }); + }), + ); - const deleteBtn = makeIconButton('icon-btn group-action-btn', 'Delete group', '🗑', () => { - deleteGroupAction(group); - }); - actions.appendChild(deleteBtn); + menu.appendChild( + makeMenuItem('Move', () => { + openMoveToDialog( + 'Move group to…', + (candidate) => !isDescendantGroup(group, candidate), + (destination) => moveGroupTo(group, destination), + ); + }), + ); - return actions; + return menu; } /** Deselect (back to root) if the current selection is `group` or nested @@ -739,6 +815,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(); diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index 950ad37..4bd7c24 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,31 @@ 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 deleteSelected = (name: string): void => { + click(groupBtnFor(name)); + click(q('[data-action="delete-group"]')); + }; const addRootGroup = (name: string): void => { click(rootBtn()); click(q('[data-action="add-group"]')); @@ -633,7 +649,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'); @@ -643,8 +659,19 @@ test('0x67 app', async (t) => { assert.ok(groupBtnFor('Renamed Group')); assert.equal(groupBtnFor('Rename Target'), undefined); + // --- ⋯ 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'); + // --- 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,18 +697,18 @@ 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); // --- delete outside the bin: no confirmation, moves into Recycle Bin --- - click(actionBtn('Renamed Group', 'Delete group')); + deleteSelected('Renamed Group'); assert.equal(byId('dlg-confirm-delete').open, false); assert.ok(groupBtnFor('Recycle Bin'), 'the recycle bin group is created on first trash'); assert.ok( @@ -694,7 +721,7 @@ test('0x67 app', async (t) => { ); // --- delete inside the bin: confirmed, permanent, whole subtree --- - click(actionBtn('Renamed Group', 'Delete group')); + deleteSelected('Renamed Group'); const confirmDlg = byId('dlg-confirm-delete'); assert.equal(confirmDlg.open, true); assert.equal(byId('confirm-delete-title').textContent, 'Delete group?'); @@ -713,6 +740,50 @@ 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 => byId('sidebar').style.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'); + + // Arrow keys step the width; every other key is left to the page. + dispatch(handle(), 'keydown', { key: 'ArrowRight' }); + assert.equal(railWidth(), '180px', '0 + one step, clamped up to the floor'); + dispatch(handle(), 'keydown', { key: 'a' }); + assert.equal(railWidth(), '180px', 'unchanged by a non-arrow key'); + dispatch(handle(), 'keydown', { key: 'ArrowLeft' }); + assert.equal(railWidth(), '180px'); + + // Re-rendering the screen replaces the rail element, so a chosen width has + // to be re-applied from memory — it is deliberately stored nowhere else. + dispatch(handle(), 'pointerdown', { clientX: 0, pointerId: 1 }); + dispatch(handle(), 'pointermove', { clientX: 300, pointerId: 1 }); + dispatch(handle(), 'pointerup', { clientX: 300, pointerId: 1 }); + assert.equal(railWidth(), '300px'); + + 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(), '300px', 're-applied after the screen re-rendered'); + }); + 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'); From 748f2ac1572af079e44336cc675205d7dbcb2c96 Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Sat, 5 Sep 2026 15:12:04 -0400 Subject: [PATCH 2/7] style:#63:use american spelling in agents guidance --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index c81c727..7850457 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,7 +12,7 @@ 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 neighbour it sits beside. Deleting already works this way: trashing is reversible and so happens silently, while 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. +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 is reversible and so happens silently, while 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 From 3e0dca38f0cce73e411e9e2a57420c04be7eaaa5 Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Sat, 5 Sep 2026 15:12:09 -0400 Subject: [PATCH 3/7] feat(0x67):#63:group tooltip, undelete, delete confirm, emoji add icons --- pages/0x67/page.css | 18 +++++------ pages/0x67/page.html | 4 +-- pages/0x67/page.ts | 57 ++++++++++++++++++++++++----------- pages/tests/0x67-page.test.ts | 49 +++++++++++++++++++++++++----- 4 files changed, 93 insertions(+), 35 deletions(-) diff --git a/pages/0x67/page.css b/pages/0x67/page.css index a8eeb01..7449c64 100644 --- a/pages/0x67/page.css +++ b/pages/0x67/page.css @@ -473,14 +473,14 @@ through to notice unsaved edits, unlike every other dirty-aware prompt. */ overflow: hidden; } -/* A flex sibling of the rail, not an overlay on it: the tree scrolls, and an -overlaid handle would sit on top of its scrollbar. */ +/* 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; it must not be claimed by scrolling */ + touch-action: none; /* a drag here resizes (#63); scrolling must not claim it */ } .sidebar-resize:hover, @@ -531,8 +531,8 @@ overlaid handle would sit on top of its scrollbar. */ gap: 0.1rem; } -/* Reserved on every row, shown only on the active one, so selecting a group -doesn't reflow its name. */ +/* 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; padding: 0.2rem 0.3rem; @@ -544,8 +544,8 @@ doesn't reflow its name. */ visibility: visible; } -/* Inline, not a floating popup: the rail and the tree both clip overflow, so -an absolutely positioned menu would be cut off. */ +/* 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; @@ -877,8 +877,8 @@ an absolutely positioned menu would be cut off. */ display: none; } - /* Tapping a group closes the drawer, so gating ⋯ on the active row would - put rename and move two drawer visits away — show it on every row here. */ + /* 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; } diff --git a/pages/0x67/page.html b/pages/0x67/page.html index 296c506..88978bc 100644 --- a/pages/0x67/page.html +++ b/pages/0x67/page.html @@ -117,7 +117,7 @@

New database

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

New database

- +
diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index f901767..47315e2 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -324,10 +324,9 @@ const SIDEBAR_WIDTH_MIN = 180; const SIDEBAR_WIDTH_MAX = 520; const SIDEBAR_WIDTH_STEP = 16; -/* Null until the user drags or arrows the handle, so the rail opens at the CSS -default — wide enough for 25 characters (#63). Kept in memory only: a width -chosen once and silently restored forever is exactly the implicit state the -project avoids, but it does have to survive re-rendering the screen. */ +/* 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; function setSidebarWidth(px: number): void { @@ -336,8 +335,8 @@ function setSidebarWidth(px: number): void { qs('#sidebar-resize').setAttribute('aria-valuenow', String(sidebarWidth)); } -/* Pointer events rather than mouse events: one path covers mouse, touch and -pen, so the rail is resizable wherever it is visible. */ +/* 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'); if (sidebarWidth !== null) setSidebarWidth(sidebarWidth); @@ -369,9 +368,8 @@ function wireSidebarResize(): void { }); } -/* The group whose ⋯ menu is open, not a flag: the drawer layout shows ⋯ on -every row, so the menu has to belong to a row rather than to the selection. -Reset whenever the selection moves. */ +/* 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 { @@ -382,7 +380,7 @@ function renderGroupTree(): void { ul.className = 'group-list'; ul.appendChild(buildGroupNode(rootGroup, true)); container.appendChild(ul); - // Deleting acts on the selection, and the root group is the database itself. + // Deleting acts on the selection (#63); the root group is the database itself. qs('#delete-group-btn').disabled = app.currentGroup === rootGroup; } @@ -396,6 +394,7 @@ function buildGroupNode(group: XmlElement, isRoot: boolean): HTMLLIElement { btn.type = 'button'; 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 = ''; @@ -447,8 +446,6 @@ function makeMenuItem(label: string, onClick: () => void): HTMLButtonElement { return btn; } -/* Deleting is deliberately absent: it is the one irreversible action here, so -it lives in the rail header rather than a gesture away from rename. */ function buildGroupMenu(group: XmlElement): HTMLDivElement { const menu = document.createElement('div'); menu.className = 'group-menu'; @@ -472,9 +469,28 @@ function buildGroupMenu(group: XmlElement): HTMLDivElement { }), ); + if (isTrashedGroup(group)) { + menu.appendChild(makeMenuItem('Undelete', () => undeleteGroup(group))); + } + return menu; } +/* 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 { + groupMenuFor = null; + moveGroupTo(group, must(app.db).getRootGroup()); +} + /** Deselect (back to root) if the current selection is `group` or nested * inside it — used when a group is moved or trashed out from under the * user's current view. */ @@ -522,11 +538,18 @@ 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( + 'Delete group?', + `"${groupName(group)}" and everything in it moves to the Recycle Bin.`, + () => { + // Created inside the callback so cancelling leaves no empty bin behind. + const bin = findOrCreateRecycleBin(db.root); + resetSelectionIfAffected(rootGroup, group); + moveGroupTo(group, bin); + }, + ); } function renderEntryPanel(): void { diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index 4bd7c24..70ca21c 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -632,10 +632,19 @@ test('0x67 app', async (t) => { liFor(name).querySelectorAll(':scope > .group-menu .group-menu-item'), ).find((b) => b.textContent === label) as HTMLButtonElement; }; - const deleteSelected = (name: string): void => { + 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"]')); @@ -658,6 +667,11 @@ 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 => @@ -707,9 +721,16 @@ test('0x67 app', async (t) => { click(dq('#dlg-move-to [data-action="cancel-move"]')); assert.equal(moveDlg.open, false); - // --- delete outside the bin: no confirmation, moves into Recycle Bin --- - deleteSelected('Renamed 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.match(byId('confirm-delete-message').textContent ?? '', /Recycle Bin/); + 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'), @@ -720,15 +741,29 @@ 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'); + 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 --- - deleteSelected('Renamed Group'); - const confirmDlg = byId('dlg-confirm-delete'); + clickHeaderDelete('Renamed Group'); assert.equal(confirmDlg.open, true); assert.equal(byId('confirm-delete-title').textContent, 'Delete group?'); // 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'); From d9b19d3554abe63c6c8eb88dd76595a3eacf0c5e Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Sat, 5 Sep 2026 15:22:52 -0400 Subject: [PATCH 4/7] fix(0x67):#63:guard recycle bin from delete, single-source width limits --- pages/0x67/page.css | 2 -- pages/0x67/page.html | 2 +- pages/0x67/page.ts | 25 ++++++++++++++++++++----- pages/tests/0x67-page.test.ts | 17 ++++++++++++++++- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/pages/0x67/page.css b/pages/0x67/page.css index 7449c64..3fceb96 100644 --- a/pages/0x67/page.css +++ b/pages/0x67/page.css @@ -38,8 +38,6 @@ --success: #0e7c5a; /* 25 chars of a first-level sub-group (#63) at .group-btn's size, plus icon, padding, nesting, and the ⋯ slot. */ --sidebar-width: 280px; - --sidebar-width-min: 180px; - --sidebar-width-max: 520px; } body { diff --git a/pages/0x67/page.html b/pages/0x67/page.html index 88978bc..4ac58e8 100644 --- a/pages/0x67/page.html +++ b/pages/0x67/page.html @@ -123,7 +123,7 @@

New database

- +
diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index 47315e2..1e98dbe 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -339,6 +339,8 @@ function setSidebarWidth(px: number): void { 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); handle.addEventListener('pointerdown', (down) => { @@ -381,7 +383,9 @@ function renderGroupTree(): void { ul.appendChild(buildGroupNode(rootGroup, true)); container.appendChild(ul); // Deleting acts on the selection (#63); the root group is the database itself. - qs('#delete-group-btn').disabled = app.currentGroup === rootGroup; + const selected = must(app.currentGroup); + qs('#delete-group-btn').disabled = + selected === rootGroup || isRecycleBinGroup(selected); } function buildGroupNode(group: XmlElement, isRoot: boolean): HTMLLIElement { @@ -442,7 +446,12 @@ function makeMenuItem(label: string, onClick: () => void): HTMLButtonElement { btn.type = 'button'; btn.className = 'group-menu-item'; btn.textContent = label; - btn.addEventListener('click', onClick); + // Closing here, not in each action, also covers a dialog the user cancels (#63). + btn.addEventListener('click', () => { + groupMenuFor = null; + renderGroupTree(); + onClick(); + }); return btn; } @@ -476,6 +485,13 @@ function buildGroupMenu(group: XmlElement): HTMLDivElement { return menu; } +/* 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 { @@ -487,7 +503,6 @@ function isTrashedGroup(group: XmlElement): boolean { /* 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 { - groupMenuFor = null; moveGroupTo(group, must(app.db).getRootGroup()); } @@ -541,8 +556,8 @@ function deleteGroupAction(group: XmlElement): void { /* Trashing is reversible, but a group carries its whole subtree with it, so it asks first (#63) where a single entry does not. */ openConfirmDelete( - 'Delete group?', - `"${groupName(group)}" and everything in it moves to the Recycle Bin.`, + '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); diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index 70ca21c..38416e2 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -720,12 +720,22 @@ test('0x67 app', async (t) => { 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', + ); // --- 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.match(byId('confirm-delete-message').textContent ?? '', /Recycle Bin/); + 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/); click(dq('#dlg-confirm-delete [data-action="cancel-delete"]')); assert.equal(groupBtnFor('Recycle Bin'), undefined, 'cancelling creates no empty bin'); @@ -745,6 +755,11 @@ test('0x67 app', async (t) => { 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'), From c14953fbd9a0456247ea7eefbe5fbcb53d30b9d2 Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Sat, 5 Sep 2026 16:16:29 -0400 Subject: [PATCH 5/7] refactor(0x67):#63:drive rail width by custom property, not inline style --- e2e/group-rail.test.ts | 27 +++++++++++++++++++++++++++ pages/0x67/page.ts | 16 ++++++++++++---- pages/tests/0x67-page.test.ts | 28 +++++++++++++--------------- 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/e2e/group-rail.test.ts b/e2e/group-rail.test.ts index e25c214..b5fee3e 100644 --- a/e2e/group-rail.test.ts +++ b/e2e/group-rail.test.ts @@ -159,3 +159,30 @@ test('at phone width the rail is a drawer: no resize handle, and ⋯ still reach 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/pages/0x67/page.ts b/pages/0x67/page.ts index 1e98dbe..ecc7948 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -329,25 +329,33 @@ 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))); - qs('#sidebar').style.width = `${sidebarWidth}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); + 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 = qs('#sidebar').getBoundingClientRect().width; + const startWidth = currentSidebarWidth(); const onMove = (move: PointerEvent) => setSidebarWidth(startWidth + move.clientX - startX); const onDone = () => { @@ -366,7 +374,7 @@ function wireSidebarResize(): void { if (key.key !== 'ArrowLeft' && key.key !== 'ArrowRight') return; key.preventDefault(); const step = key.key === 'ArrowLeft' ? -SIDEBAR_WIDTH_STEP : SIDEBAR_WIDTH_STEP; - setSidebarWidth(qs('#sidebar').getBoundingClientRect().width + step); + setSidebarWidth(currentSidebarWidth() + step); }); } diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index 38416e2..b66719e 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -792,7 +792,8 @@ 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 => byId('sidebar').style.width; + 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 @@ -813,25 +814,22 @@ test('0x67 app', async (t) => { dispatch(handle(), 'pointermove', { clientX: 250, pointerId: 1 }); assert.equal(railWidth(), '520px', 'releasing stops tracking the pointer'); - // Arrow keys step the width; every other key is left to the page. - dispatch(handle(), 'keydown', { key: 'ArrowRight' }); - assert.equal(railWidth(), '180px', '0 + one step, clamped up to the floor'); - dispatch(handle(), 'keydown', { key: 'a' }); - assert.equal(railWidth(), '180px', 'unchanged by a non-arrow key'); + // 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(), '180px'); - - // Re-rendering the screen replaces the rail element, so a chosen width has - // to be re-applied from memory — it is deliberately stored nowhere else. - dispatch(handle(), 'pointerdown', { clientX: 0, pointerId: 1 }); - dispatch(handle(), 'pointermove', { clientX: 300, pointerId: 1 }); - dispatch(handle(), 'pointerup', { clientX: 300, pointerId: 1 }); - assert.equal(railWidth(), '300px'); + 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(), '300px', 're-applied after the screen re-rendered'); + 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', () => { From a80eb57b14aead21473019d85d5712c98b91b7a4 Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Sat, 5 Sep 2026 17:56:44 -0400 Subject: [PATCH 6/7] fix(0x67):#63:select on menu open, label reversible confirm distinctly --- AGENTS.md | 2 +- pages/0x67/page.ts | 37 ++++++++++++++++++++++++++--------- pages/tests/0x67-page.test.ts | 14 +++++++++++++ 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7850457..c2803ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,7 +12,7 @@ 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 is reversible and so happens silently, while 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. +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 diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index ecc7948..f752d31 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -396,6 +396,18 @@ function renderGroupTree(): void { 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'); @@ -408,13 +420,8 @@ function buildGroupNode(group: XmlElement, isRoot: boolean): HTMLLIElement { 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 = ''; groupMenuFor = null; - const searchInput = document.querySelector('#search-input'); - if (searchInput) searchInput.value = ''; - renderGroupTree(); - renderEntryPanel(); + selectGroup(group); setSidebarOpen(false); }); row.appendChild(btn); @@ -426,7 +433,7 @@ function buildGroupNode(group: XmlElement, isRoot: boolean): HTMLLIElement { row.appendChild( makeIconButton('icon-btn group-menu-btn', 'Group actions', '⋯', () => { groupMenuFor = groupMenuFor === group ? null : group; - renderGroupTree(); + selectGroup(group); }), ); } @@ -572,6 +579,8 @@ function deleteGroupAction(group: XmlElement): void { resetSelectionIfAffected(rootGroup, group); moveGroupTo(group, bin); }, + 'Move to Bin', + false, ); } @@ -1650,12 +1659,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 b66719e..7bc9b22 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -684,6 +684,14 @@ test('0x67 app', async (t) => { 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(menuItem('Move Target', 'Move')); const moveDlg = byId('dlg-move-to'); @@ -736,6 +744,10 @@ test('0x67 app', async (t) => { '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'); @@ -775,6 +787,8 @@ test('0x67 app', async (t) => { 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/); From 96521f51eec9ab70a283fded8a1d75a9b0116cd8 Mon Sep 17 00:00:00 2001 From: Bishop Bettini Date: Sat, 5 Sep 2026 18:51:05 -0400 Subject: [PATCH 7/7] fix(0x67):#63:close the row menu when its group is deleted --- pages/0x67/page.ts | 4 ++++ pages/tests/0x67-page.test.ts | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index f752d31..d42ca04 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -544,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(); diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index 7bc9b22..957c4be 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -734,6 +734,13 @@ test('0x67 app', async (t) => { '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: confirmed, then moves into Recycle Bin --- clickHeaderDelete('Renamed Group'); const confirmDlg = byId('dlg-confirm-delete');