Skip to content

Commit 4971e85

Browse files
authored
Merge pull request #68 from keepass-web/issue-63-group-navigation
#63: clearer, resizable group navigation
2 parents 1e1fb63 + 96521f5 commit 4971e85

7 files changed

Lines changed: 627 additions & 74 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ The concrete implication for an agent: don't reach for a framework, a general-pu
1212

1313
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.
1414

15+
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.
16+
1517
## Approach
1618

1719
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.

e2e/group-rail.test.ts

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
/** Real-browser coverage for the group rail (issue #63). Both assertions here
2+
* need a layout engine, which jsdom does not have: that the rail's default
3+
* width really does show 25 characters of a sub-group name with no
4+
* intervention, and that dragging the handle really does resize it. */
5+
import assert from 'node:assert/strict';
6+
import { after, before, test } from 'node:test';
7+
import { fileURLToPath } from 'node:url';
8+
import puppeteer, { type Browser, type ElementHandle, type Frame, type Page } from 'puppeteer-core';
9+
import { resolveChromePath } from './support/chrome.ts';
10+
import { type DistServer, startDistServer } from './support/dist-server.ts';
11+
import { type KdbxFixture, writeKdbxFixture } from './support/fixture.ts';
12+
import { resolveLaunchOptions } from './support/launch-options.ts';
13+
14+
const distDir = fileURLToPath(new URL('../dist', import.meta.url));
15+
16+
let server: DistServer;
17+
let browser: Browser;
18+
let page: Page;
19+
let app: Frame;
20+
let fixture: KdbxFixture;
21+
22+
before(async () => {
23+
server = await startDistServer(distDir);
24+
browser = await puppeteer.launch({
25+
executablePath: resolveChromePath(),
26+
...resolveLaunchOptions(),
27+
args: ['--no-sandbox'],
28+
});
29+
page = await browser.newPage();
30+
// Comfortably wider than the 700px drawer breakpoint, so the rail is the
31+
// resizable side rail rather than the mobile drawer.
32+
await page.setViewport({ width: 1280, height: 900 });
33+
fixture = await writeKdbxFixture();
34+
35+
app = await openApp(page);
36+
});
37+
38+
/** Upload the fixture to local.html and unlock the app it embeds, returning
39+
* the app's frame. */
40+
async function openApp(target: Page): Promise<Frame> {
41+
await target.goto(`${server.origin}/local.html`, { waitUntil: 'networkidle0' });
42+
const fileInput = (await target.waitForSelector(
43+
'#file-input',
44+
)) as ElementHandle<HTMLInputElement>;
45+
await fileInput.uploadFile(fixture.path);
46+
const frameElement = await target.waitForSelector('#app-frame');
47+
assert.ok(frameElement, 'the app is embedded in an iframe');
48+
const frame = (await frameElement.contentFrame()) as Frame;
49+
const passwordInput = await frame.waitForSelector('#master-password');
50+
assert.ok(passwordInput, 'the embedded app shows its unlock screen');
51+
await passwordInput.type(fixture.password);
52+
await frame.click('#unlock-btn');
53+
await frame.waitForSelector('#group-tree .group-btn');
54+
return frame;
55+
}
56+
57+
after(async () => {
58+
await browser.close();
59+
await server.close();
60+
});
61+
62+
test('the rail shows 25 characters of a sub-group name without any intervention', async (t) => {
63+
const measured = await app.$$eval(
64+
'#group-tree .group-btn',
65+
(buttons, name) => {
66+
const button = buttons.find((b) => b.textContent?.endsWith(name));
67+
if (!button) return null;
68+
// scrollWidth is clamped to clientWidth, so it can only ever report
69+
// "overflowing" or "not" — never by how much. Measuring the text itself
70+
// against the content box gives a margin that can be watched over time.
71+
const text = document.createRange();
72+
text.selectNodeContents(button);
73+
const style = getComputedStyle(button);
74+
const padding = Number.parseFloat(style.paddingLeft) + Number.parseFloat(style.paddingRight);
75+
return {
76+
needed: text.getBoundingClientRect().width,
77+
available: button.clientWidth - padding,
78+
};
79+
},
80+
fixture.groupName,
81+
);
82+
83+
assert.ok(measured, `the rail lists "${fixture.groupName}"`);
84+
// Reported on every run: the monospace fallback differs between developer
85+
// machines and CI, so a shrinking margin here is the early warning that the
86+
// default width is drifting towards truncation.
87+
t.diagnostic(
88+
`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)`,
89+
);
90+
assert.ok(
91+
measured.needed <= measured.available,
92+
`"${fixture.groupName}" does not fit the default rail: needs ${measured.needed.toFixed(1)}px, has ${measured.available.toFixed(1)}px`,
93+
);
94+
});
95+
96+
test('dragging the handle resizes the rail', async () => {
97+
const railWidth = (): Promise<number> =>
98+
app.$eval('#sidebar', (el) => el.getBoundingClientRect().width);
99+
100+
const handle = await app.$('#sidebar-resize');
101+
assert.ok(handle, 'the rail has a resize handle');
102+
const box = await handle.boundingBox();
103+
assert.ok(box, 'the handle is laid out');
104+
105+
const startWidth = await railWidth();
106+
const y = box.y + 20;
107+
await page.mouse.move(box.x + box.width / 2, y);
108+
await page.mouse.down();
109+
await page.mouse.move(box.x + box.width / 2 + 80, y, { steps: 8 });
110+
await page.mouse.up();
111+
112+
const endWidth = await railWidth();
113+
assert.ok(
114+
endWidth > startWidth,
115+
`dragging right widened the rail (${startWidth}px -> ${endWidth}px)`,
116+
);
117+
});
118+
119+
test('at phone width the rail is a drawer: no resize handle, and ⋯ still reaches rename', async () => {
120+
const phone = await browser.newPage();
121+
await phone.setViewport({ width: 375, height: 812 });
122+
const phoneApp = await openApp(phone);
123+
124+
assert.equal(
125+
await phoneApp.$eval('#sidebar-resize', (el) => getComputedStyle(el).display),
126+
'none',
127+
'the drawer has no edge to drag, so the handle is not rendered',
128+
);
129+
130+
await phoneApp.click('[data-action="toggle-sidebar"]');
131+
await phoneApp.waitForSelector('#sidebar.sidebar-open');
132+
// The drawer slides in over 0.2s; clicking mid-flight misses the button.
133+
await phoneApp.waitForFunction(() => {
134+
const drawer = document.querySelector('#sidebar');
135+
return drawer !== null && getComputedStyle(drawer).transform === 'matrix(1, 0, 0, 1, 0, 0)';
136+
});
137+
138+
// Every drawer row exposes its ⋯, because tapping a group to make it active
139+
// would close the drawer and cost a second visit.
140+
const menuButton = await phoneApp.evaluateHandle((name) => {
141+
const rows = Array.from(document.querySelectorAll('#group-tree .group-row'));
142+
const row = rows.find((r) => r.querySelector('.group-btn')?.textContent?.endsWith(name));
143+
return row?.querySelector('.group-menu-btn') ?? null;
144+
}, fixture.groupName);
145+
const menuElement = menuButton.asElement() as ElementHandle<HTMLElement> | null;
146+
assert.ok(menuElement, 'the sub-group row has a ⋯ button in the drawer');
147+
assert.notEqual(
148+
await menuElement.evaluate((el) => getComputedStyle(el).visibility),
149+
'hidden',
150+
'⋯ is visible without first selecting the row',
151+
);
152+
153+
await menuElement.click();
154+
const labels = await phoneApp.$$eval('.group-menu-item', (items) =>
155+
items.map((i) => i.textContent),
156+
);
157+
assert.deepEqual(labels, ['Rename', 'Move'], 'the menu opens with rename and move');
158+
159+
assert.ok(await phoneApp.$('#sidebar.sidebar-open'), 'opening the menu left the drawer open');
160+
await phone.close();
161+
});
162+
163+
test('a rail widened on desktop does not follow the user into the phone drawer', async () => {
164+
const handle = await app.$('#sidebar-resize');
165+
assert.ok(handle, 'the rail has a resize handle');
166+
const box = await handle.boundingBox();
167+
assert.ok(box, 'the handle is laid out');
168+
169+
const y = box.y + 20;
170+
await page.mouse.move(box.x + box.width / 2, y);
171+
await page.mouse.down();
172+
await page.mouse.move(box.x + box.width / 2 + 400, y, { steps: 8 });
173+
await page.mouse.up();
174+
175+
const railWidth = (): Promise<number> =>
176+
app.$eval('#sidebar', (el) => el.getBoundingClientRect().width);
177+
const wide = await railWidth();
178+
assert.ok(wide > 400, `the rail is dragged wide first (${wide}px)`);
179+
180+
await page.setViewport({ width: 375, height: 812 });
181+
const drawer = await railWidth();
182+
assert.ok(
183+
drawer <= 375,
184+
`the drawer keeps its own width at phone size (${drawer}px inside a 375px viewport)`,
185+
);
186+
187+
await page.setViewport({ width: 1280, height: 900 });
188+
});

e2e/support/fixture.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@
33
import { writeFile } from 'node:fs/promises';
44
import { tmpdir } from 'node:os';
55
import { join } from 'node:path';
6-
import { appendChild, Credentials, createEntry, Kdbx } from '../../packages/kdbx/src/index.ts';
6+
import {
7+
appendChild,
8+
Credentials,
9+
createEntry,
10+
createGroup,
11+
Kdbx,
12+
} from '../../packages/kdbx/src/index.ts';
713

814
// Fast KDF settings (matches pages/tests/*.test.ts) — a throwaway fixture,
915
// no reason to pay real Argon2id cost.
@@ -13,11 +19,14 @@ export interface KdbxFixture {
1319
path: string;
1420
password: string;
1521
entryTitle: string;
22+
groupName: string;
1623
}
1724

1825
export async function writeKdbxFixture(): Promise<KdbxFixture> {
1926
const password = 'e2e-test-password';
2027
const entryTitle = 'Example Entry';
28+
// Exactly 25 characters, the floor issue #63 sets for the group rail.
29+
const groupName = 'Financial Institutions XY';
2130

2231
const credentials = new Credentials({ password });
2332
const kdbx = await Kdbx.create(credentials, {
@@ -32,6 +41,7 @@ export async function writeKdbxFixture(): Promise<KdbxFixture> {
3241
kdbx.getRootGroup(),
3342
createEntry({ title: entryTitle, username: 'octocat', password: 'hunter2' }),
3443
);
44+
appendChild(kdbx.getRootGroup(), createGroup(groupName));
3545
const bytes = await kdbx.save();
3646

3747
const path = join(
@@ -40,5 +50,5 @@ export async function writeKdbxFixture(): Promise<KdbxFixture> {
4050
);
4151
await writeFile(path, bytes);
4252

43-
return { path, password, entryTitle };
53+
return { path, password, entryTitle, groupName };
4454
}

pages/0x67/page.css

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@
3636
--warning-bg: #fbf0e7;
3737
--warning-border: #ead7c2;
3838
--success: #0e7c5a;
39-
--sidebar-width: 210px;
39+
/* 25 chars of a first-level sub-group (#63) at .group-btn's size, plus icon, padding, nesting, and the ⋯ slot. */
40+
--sidebar-width: 280px;
4041
}
4142

4243
body {
@@ -470,6 +471,21 @@ through to notice unsaved edits, unlike every other dirty-aware prompt. */
470471
overflow: hidden;
471472
}
472473

474+
/* A flex sibling of the rail, not an overlay on it (#63): the tree scrolls, and
475+
an overlaid handle would sit on top of its scrollbar. */
476+
.sidebar-resize {
477+
flex: 0 0 6px;
478+
margin: 0;
479+
border: none;
480+
cursor: col-resize;
481+
touch-action: none; /* a drag here resizes (#63); scrolling must not claim it */
482+
}
483+
484+
.sidebar-resize:hover,
485+
.sidebar-resize:focus-visible {
486+
background: var(--accent-dim);
487+
}
488+
473489
.sidebar-header {
474490
display: flex;
475491
align-items: center;
@@ -479,6 +495,11 @@ through to notice unsaved edits, unlike every other dirty-aware prompt. */
479495
flex-shrink: 0;
480496
}
481497

498+
.sidebar-header-actions {
499+
display: flex;
500+
gap: 0.15rem;
501+
}
502+
482503
.sidebar-label {
483504
font-size: 0.75rem;
484505
font-weight: 600;
@@ -508,14 +529,41 @@ through to notice unsaved edits, unlike every other dirty-aware prompt. */
508529
gap: 0.1rem;
509530
}
510531

511-
.group-actions {
512-
display: flex;
532+
/* Reserved on every row, shown only on the active one (#63), so selecting a
533+
group doesn't reflow its name. */
534+
.group-menu-btn {
513535
flex-shrink: 0;
514-
}
515-
516-
.group-action-btn {
517536
padding: 0.2rem 0.3rem;
518537
font-size: 0.8rem;
538+
visibility: hidden;
539+
}
540+
541+
.group-row-active .group-menu-btn {
542+
visibility: visible;
543+
}
544+
545+
/* Inline, not a floating popup (#63): the rail and the tree both clip overflow,
546+
so an absolutely positioned menu would be cut off. */
547+
.group-menu {
548+
display: flex;
549+
gap: 0.25rem;
550+
padding: 0.15rem 0 0.35rem 1rem;
551+
}
552+
553+
.group-menu-item {
554+
background: none;
555+
border: 1px solid var(--border);
556+
border-radius: 4px;
557+
color: var(--text);
558+
cursor: pointer;
559+
font-family: inherit;
560+
font-size: 0.75rem;
561+
padding: 0.2rem 0.5rem;
562+
white-space: nowrap;
563+
}
564+
565+
.group-menu-item:hover {
566+
background: var(--surface-2);
519567
}
520568

521569
.group-btn {
@@ -823,6 +871,16 @@ through to notice unsaved edits, unlike every other dirty-aware prompt. */
823871
transition: transform 0.2s ease;
824872
}
825873

874+
.sidebar-resize {
875+
display: none;
876+
}
877+
878+
/* Tapping a group closes the drawer (#63), so gating ⋯ on the active row
879+
would put rename and move two drawer visits away; show it on every row. */
880+
.group-menu-btn {
881+
visibility: visible;
882+
}
883+
826884
.sidebar.sidebar-open {
827885
transform: translateX(0);
828886
}

pages/0x67/page.html

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,14 @@ <h1 class="db-filename">New database</h1>
116116
<aside id="sidebar" class="sidebar">
117117
<div class="sidebar-header">
118118
<span class="sidebar-label">Groups</span>
119-
<button type="button" class="icon-btn" data-action="add-group" title="New group">+</button>
119+
<div class="sidebar-header-actions">
120+
<button type="button" class="icon-btn" data-action="add-group" title="New group"></button>
121+
<button type="button" class="icon-btn" id="delete-group-btn" data-action="delete-group" title="Delete group" aria-label="Delete group">🗑</button>
122+
</div>
120123
</div>
121124
<nav id="group-tree" class="group-tree"></nav>
122125
</aside>
126+
<hr id="sidebar-resize" class="sidebar-resize" tabindex="0" aria-orientation="vertical" aria-label="Resize groups panel" aria-valuenow="280">
123127
<main class="entry-panel">
124128
<div class="panel-header">
125129
<span id="panel-title" class="panel-title"></span>
@@ -143,7 +147,7 @@ <h1 class="db-filename">New database</h1>
143147
<option value="modified:asc">Modified (oldest)</option>
144148
</select>
145149
</div>
146-
<button type="button" class="icon-btn" data-action="add-entry" title="New entry">+</button>
150+
<button type="button" class="icon-btn" data-action="add-entry" title="New entry"></button>
147151
</div>
148152
</div>
149153
<div id="entry-list" class="entry-list"></div>

0 commit comments

Comments
 (0)