Escape does not clear a clicked selection in Files (focus not on the list); fix in the shared collection primitive

kayg commented

2026-09-30 17:25:05 +00:00

Owner

Owner report (2026-09-30): "with a folder selected, esc doesn't dismiss it?"

Likely cause (verify it): FilesBrowser.svelte clears the selection on Escape only in the listbox's own onKeyDown. After a pointer click selects a row, focus is often not in the listbox (the row is not focused, or focus stays on the body or the header), so Escape reaches onWindowKey, which ignores it. The job/files-sel-keys branch (#466, in merge round 3) keeps the same listbox-only placement.
Fix (after merge round 3 lands, same file):

Clicking a row moves focus to that row (roving tabindex, as the list keyboard model expects), so every key works right after a click.
As a safety net, the window handler clears a non-empty Files selection on Escape when no overlay, menu, sheet, rename field or text input has focus. Escape closes the topmost layer first; only a second Escape clears the selection (macOS Finder behaviour).
The same rule applies to the Photos grid, Mail list, Tasks, Money register and Calendar selection: move it into the shared selection/collection primitive so every list behaves the same (reuse gate).
There is no animation for keyboard-driven changes (#527).
Test: an e2e that clicks a folder, presses Escape and expects no selection; with a menu open, Escape closes the menu first and a second Escape clears the selection; the same in Photos and Mail. Web gates.

## Owner report (2026-09-30): "with a folder selected, esc doesn't dismiss it?" **Likely cause (verify it):** `FilesBrowser.svelte` clears the selection on Escape only in the listbox's own `onKeyDown`. After a pointer click selects a row, focus is often not in the listbox (the row is not focused, or focus stays on the body or the header), so Escape reaches `onWindowKey`, which ignores it. The job/files-sel-keys branch (#466, in merge round 3) keeps the same listbox-only placement. **Fix (after merge round 3 lands, same file):** - Clicking a row moves focus to that row (roving tabindex, as the list keyboard model expects), so every key works right after a click. - As a safety net, the window handler clears a non-empty Files selection on Escape when no overlay, menu, sheet, rename field or text input has focus. Escape closes the topmost layer first; only a second Escape clears the selection (macOS Finder behaviour). - The same rule applies to the Photos grid, Mail list, Tasks, Money register and Calendar selection: move it into the shared selection/collection primitive so every list behaves the same (reuse gate). - There is no animation for keyboard-driven changes (#527). **Test:** an e2e that clicks a folder, presses Escape and expects no selection; with a menu open, Escape closes the menu first and a second Escape clears the selection; the same in Photos and Mail. Web gates.

kayg commented

2026-10-01 00:52:25 +00:00

Author

Owner

Starting #537 on branch job/esc-537, based on origin/dev at cc25c441b7a974185622a1dee853cf38686d2b67. I am tracing the shared collection/selection behavior before editing.

Starting #537 on branch `job/esc-537`, based on `origin/dev` at `cc25c441b7a974185622a1dee853cf38686d2b67`. I am tracing the shared collection/selection behavior before editing.

kayg commented

2026-10-01 01:00:11 +00:00

Author

Owner

Finding: FileCollection.svelte renders every option with tabindex="-1" and its click handler only calls the selection callback, so a pointer click does not move focus into the collection. FilesBrowser.svelte clears Escape only in the collection's onKeyDown; its window handler has no selection-Escape fallback. Both files are on the origin/dev base SHA above. The app already shares selection state logic between Files and Photos in apps/web/src/lib/files/selection.ts, so I am extending that existing model and the existing collection instead of adding another collection component.

Finding: `FileCollection.svelte` renders every option with `tabindex="-1"` and its click handler only calls the selection callback, so a pointer click does not move focus into the collection. `FilesBrowser.svelte` clears Escape only in the collection's `onKeyDown`; its window handler has no selection-Escape fallback. Both files are on the `origin/dev` base SHA above. The app already shares selection state logic between Files and Photos in `apps/web/src/lib/files/selection.ts`, so I am extending that existing model and the existing collection instead of adding another collection component.

kayg commented

2026-10-01 01:23:49 +00:00

Author

Owner

Finding: the new roving option model could leave DOM focus on a row after Escape changed its tabindex to -1. The collection now returns focus to the listbox tab stop when a focused row is cleared; the window fallback preserves focus when Escape starts outside the collection. The regression test checks the restored tab stop. Targeted Vitest output: 2 files passed, 10 tests passed. Fix committed as f2477e963.

Finding: the new roving option model could leave DOM focus on a row after Escape changed its tabindex to -1. The collection now returns focus to the listbox tab stop when a focused row is cleared; the window fallback preserves focus when Escape starts outside the collection. The regression test checks the restored tab stop. Targeted Vitest output: 2 files passed, 10 tests passed. Fix committed as f2477e963.

kayg commented

2026-10-01 01:34:38 +00:00

Author

Owner

Decision not covered by DESIGN §45: Escape from a selected Mail thread or message returns to /mail. The thread and message deep links do not encode their source category or folder, so /mail gives a deterministic list destination for a cold deep link. I will include this behavior in the final report.

Decision not covered by DESIGN §45: Escape from a selected Mail thread or message returns to `/mail`. The thread and message deep links do not encode their source category or folder, so `/mail` gives a deterministic list destination for a cold deep link. I will include this behavior in the final report.

kayg commented

2026-10-01 01:52:54 +00:00

Author

Owner

Finding: a selected preview can keep Escape ownership even when focus remains on the collection row. PopoverSurface.svelte installs its selected-preview Escape handler on window; the selection fallback also runs there, so a focus-only check can clear selection first and prevent the preview from closing. The shared selection rule now detects an open floating or overlay surface independently of focus, and reports blocked versus empty selection so local list handlers do not consume the overlay's Escape. Added a focused regression test for a floating surface with focus left on the row.

Finding: a selected preview can keep Escape ownership even when focus remains on the collection row. `PopoverSurface.svelte` installs its selected-preview Escape handler on `window`; the selection fallback also runs there, so a focus-only check can clear selection first and prevent the preview from closing. The shared selection rule now detects an open floating or overlay surface independently of focus, and reports blocked versus empty selection so local list handlers do not consume the overlay's Escape. Added a focused regression test for a floating surface with focus left on the row.

kayg commented

2026-10-01 02:29:23 +00:00

Author

Owner

Finding: the production Photos E2E closes the cold /p/<id> viewer with Escape, but the underlying timeline does not restore the capture day. Evidence: .ql detached, then the existing May 2024 heading assertion timed out after 30 seconds. closeViewer() returned every cold Photo link to /photos; it now returns to the viewed capture day, as required by the Photos deep-link grammar in DESIGN §33. The existing expectation is unchanged.

Finding: the production Photos E2E closes the cold `/p/<id>` viewer with Escape, but the underlying timeline does not restore the capture day. Evidence: `.ql` detached, then the existing `May 2024` heading assertion timed out after 30 seconds. `closeViewer()` returned every cold Photo link to `/photos`; it now returns to the viewed capture day, as required by the Photos deep-link grammar in DESIGN §33. The existing expectation is unchanged.

kayg commented

2026-10-01 02:37:46 +00:00

Author

Owner

Finding: after the cold /p/<id> return-path fix, the full Photos E2E passed the existing date-restore assertion and continued through month and burst checks. It then timed out in the unchanged HLS fallback proof: Chromium reported an empty canPlayType('application/vnd.apple.mpegurl'); the video element requested the direct source twice, and the test observed zero HLS master, variant, or segment requests. The dedicated --escape-only Photos proof passes. I kept the existing HLS expectation unchanged because it is outside #537.

Finding: after the cold `/p/<id>` return-path fix, the full Photos E2E passed the existing date-restore assertion and continued through month and burst checks. It then timed out in the unchanged HLS fallback proof: Chromium reported an empty `canPlayType('application/vnd.apple.mpegurl')`; the video element requested the direct source twice, and the test observed zero HLS master, variant, or segment requests. The dedicated `--escape-only` Photos proof passes. I kept the existing HLS expectation unchanged because it is outside #537.

kayg commented

2026-10-01 02:44:51 +00:00

Author

Owner

Finding: the Files responsive screenshot helper can observe the selection header before its file rows finish loading. Evidence: after the Inbox header checkbox appeared, the reduced-motion query for .fc-head, .fc-item-inner returned only one node and the unchanged two-duration assertion failed. I am adding a wait for a real row before that style assertion; the expected values stay unchanged.

Finding: the Files responsive screenshot helper can observe the selection header before its file rows finish loading. Evidence: after the `Inbox` header checkbox appeared, the reduced-motion query for `.fc-head, .fc-item-inner` returned only one node and the unchanged two-duration assertion failed. I am adding a wait for a real row before that style assertion; the expected values stay unchanged.

kayg commented

2026-10-01 02:53:43 +00:00

Author

Owner

Finding: the existing Recent/Trash screenshot helper calls applyEvidenceTheme(target, theme), but no such function exists in files.mjs. Evidence: the screenshot-only run reached that line after the Files keyboard/rename flows and threw ReferenceError: applyEvidenceTheme is not defined. I am adding the missing helper; it saves the scheme, reloads the current route, and checks the rendered theme before capture.

Finding: the existing Recent/Trash screenshot helper calls `applyEvidenceTheme(target, theme)`, but no such function exists in `files.mjs`. Evidence: the screenshot-only run reached that line after the Files keyboard/rename flows and threw `ReferenceError: applyEvidenceTheme is not defined`. I am adding the missing helper; it saves the scheme, reloads the current route, and checks the rendered theme before capture.

kayg commented

2026-10-01 03:03:33 +00:00

Author

Owner

Finding: the Recent/Trash screenshot helper enters phone touch-selection mode, asserts its marks are visible, then asserts they are hidden before activating the existing Done action. Evidence: the production Files component kept .fc.marks-visible true at the unchanged assertion. I am moving the existing Done click before that assertion; the expected value remains unchanged, and the capture still checks mixed selection.

Finding: the Recent/Trash screenshot helper enters phone touch-selection mode, asserts its marks are visible, then asserts they are hidden before activating the existing Done action. Evidence: the production Files component kept `.fc.marks-visible` true at the unchanged assertion. I am moving the existing Done click before that assertion; the expected value remains unchanged, and the capture still checks mixed selection.

kayg commented

2026-10-01 03:28:37 +00:00

Author

Owner

Evidence update for #537:

The Money E2E setup sent auto_scheme.location: null. The current Appearance contract denies that removed field because location belongs in Location settings (DESIGN §47); the server returned 422. I updated the test setup to send only the colour mode. The full Money E2E then passed, including the new transaction Escape assertion, and captured 36 production screenshots.
The full Calendar E2E passed its selected-preview Escape check, then stopped at the unchanged composer snapshot assertion: expected the payload to end in Frozen snapshot; observed 05:18 nFrozen snapshot. The focused Calendar drag-only run also failed an unchanged timing-label assertion: expected 08:30 – 09:15, observed 08:45 – 09:45. I did not alter either expectation. The new focused Calendar Escape proof passed at 390, 820 and 1440 px in light and dark, with six screenshots.
Files screenshots passed after the touch flow exited selection mode through its existing Done action. The harness produced zero CSP reports across 21 pages.

Evidence update for #537: - The Money E2E setup sent `auto_scheme.location: null`. The current Appearance contract denies that removed field because location belongs in Location settings (DESIGN §47); the server returned 422. I updated the test setup to send only the colour mode. The full Money E2E then passed, including the new transaction Escape assertion, and captured 36 production screenshots. - The full Calendar E2E passed its selected-preview Escape check, then stopped at the unchanged composer snapshot assertion: expected the payload to end in ` Frozen snapshot`; observed `05:18 nFrozen snapshot`. The focused Calendar drag-only run also failed an unchanged timing-label assertion: expected `08:30 – 09:15`, observed `08:45 – 09:45`. I did not alter either expectation. The new focused Calendar Escape proof passed at 390, 820 and 1440 px in light and dark, with six screenshots. - Files screenshots passed after the touch flow exited selection mode through its existing Done action. The harness produced zero CSP reports across 21 pages.

kayg commented

2026-10-01 03:30:19 +00:00

Author

Owner

Local performance profile completed because the shared perf VM lock was busy. The load average was 18.31 / 24.59 / 25.19 before the run and 29.80 / 29.94 / 27.64 after it. With one run per viewport, Escape on one selected row measured p50 265.7 ms / p95 641.2 ms across 15 samples. On 5,000 selected items, 24 Escape cycles measured p50 402.7 ms / p95 898.5 ms and 27.6 s total. Server RSS averaged 373.6 MB; Escape samples averaged 1.7% CPU for the one-row case and 1.34% for the burst, with peaks near 20%.

docs/perf/baseline.json has no Escape-selection series, so there is no like-for-like comparison. The route profile also marked the 5k-folder first-row p95 at 1,312 ms against its 500 ms profile budget; this local result ran under the high shared-host load above, so it is not evidence of a product regression. The complete JSON is at artifacts/esc-537/perf.json in this worktree.

Local performance profile completed because the shared perf VM lock was busy. The load average was 18.31 / 24.59 / 25.19 before the run and 29.80 / 29.94 / 27.64 after it. With one run per viewport, Escape on one selected row measured p50 265.7 ms / p95 641.2 ms across 15 samples. On 5,000 selected items, 24 Escape cycles measured p50 402.7 ms / p95 898.5 ms and 27.6 s total. Server RSS averaged 373.6 MB; Escape samples averaged 1.7% CPU for the one-row case and 1.34% for the burst, with peaks near 20%. `docs/perf/baseline.json` has no Escape-selection series, so there is no like-for-like comparison. The route profile also marked the 5k-folder first-row p95 at 1,312 ms against its 500 ms profile budget; this local result ran under the high shared-host load above, so it is not evidence of a product regression. The complete JSON is at `artifacts/esc-537/perf.json` in this worktree.

kayg commented

2026-10-01 03:40:35 +00:00

Author

Owner

Finished

Branch: job/esc-537
Base: origin/dev was already up to date at the one required merge check.
Head: f8ab419af968c348dc3e30de769cd71a533cac29

Built

Shared Escape selection precedence for Files, Recent, Trash, Photos, Mail, Calendar and Money. Open overlays and text-editing surfaces keep the first Escape; the next Escape clears the selection.
Files row clicks focus their roving option. Escape restores focus to the listbox after selection clears.
A cold /p/<id> link returns to that photo's capture day after Escape closes the viewer.
Added focused Mail and Calendar Escape E2Es, expanded menu-first Photos and Files proofs, and added a 1-row / 5,000-row Files Escape performance profile.

Files changed: apps/web/src/lib/files/selection.ts, apps/web/src/lib/files/{FilesBrowser,RecentView,TrashView}.svelte, packages/ui/src/components/files/FileCollection.svelte, apps/web/src/lib/files/{FileCollection.svelte.test.ts,selection.escape.svelte.test.ts}, apps/web/src/lib/{mail/MailView,photos/PhotoTimeline,photos/PhotosView}.svelte, apps/web/src/routes/calendar/[view]/[date]/+page.svelte, apps/web/src/routes/money/[budget]/accounts/[[account]]/+page.svelte, and E2E/performance evidence in apps/web/e2e/{calendar,files,mail-escape,money,photos,route-perf}.mjs plus apps/web/package.json.

Decisions

Mail clears a selected thread to /mail: a Mail thread deep link does not retain its category or folder.
Calendar Task previews use the Calendar Escape rule. This checkout has no separate Tasks collection view.
Closing a cold photo deep link returns to its capture day, as the nearest exact timeline view under DESIGN §33.

Gates

bun run check (verbatim output):

$ node scripts/check-type-tokens.mjs && node scripts/check-motion-tokens.mjs && svelte-kit sync && svelte-check --tsconfig ./tsconfig.json
Text sizes and UI shape values use shared role tokens.
UI transitions and animation options use shared motion tokens or documented exceptions.
Loading svelte-check in workspace: /home/kayg/Developer/calternal-wt/esc-537/apps/web
Getting Svelte diagnostics...

svelte-check found 0 errors and 0 warnings

bun run test (verbatim summary):

 Test Files  141 passed (141)
      Tests  936 passed (936)
   Start at  05:31:54
   Duration  93.17s (transform 50%, environment 20%, import 16%, tests 10%, setup 4%)

Vitest also printed Not implemented: Window's scrollTo() method and Could not parse CSS stylesheet; exit status was 0. No Rust source changed, so Rust clippy/test gates did not apply. cargo clean removed 7,169 files (4.5 GiB). Web build output was removed.

Production E2E and screenshots

Files Escape proof passed; the screenshot run passed with 0 CSP reports across 21 pages.
Mail Escape proof passed; 12 list/thread captures cover 390, 820 and 1440 px in light and dark.
Photos Escape proof and screenshot-only run passed; both use the production SPA.
Money E2E passed its deep-linked transaction Escape assertion and captured 36 screenshots. I removed the obsolete auto_scheme.location test field after the API returned 422; DESIGN §47 puts location in Location settings.
Calendar Escape proof passed at 390, 820 and 1440 px in both themes and captured six screenshots.

The ignored local evidence is in artifacts/esc-537/: 321 PNGs totaling 169,504,221 bytes, plus perf.json. No screenshots are committed. The installed fj issue comment --help supports a text body or body file and has no attachment option, so screenshots remain in the worktree. I did not put Money screenshot data on the public issue.

Performance

The shared perf VM lock was busy, so the profile ran locally. Load average rose from 18.31 / 24.59 / 25.19 before the run to 29.80 / 29.94 / 27.64 after it. One selected row: p50 265.7 ms, p95 641.2 ms (15 samples). Five thousand selected rows: p50 402.7 ms, p95 898.5 ms; 24 Escape cycles took 27,593.9 ms. The server RSS averaged about 373.6 MB; mean server CPU was 1.7% for one-row clears and 1.34% for the burst, with peaks near 20%. docs/perf/baseline.json has no Escape series. The local 5k-folder first-row p95 was 1,312 ms against the profile's 500 ms budget under the high host load, so it is not a like-for-like regression result.

Known gaps

The full Photos E2E reached the existing HLS fallback check, then timed out: canPlayType() was empty and no master, variant or segment request occurred. The check expectation is unchanged; the focused Photos Escape proof passed.
The full Calendar E2E passed its selected-preview Escape check, then failed the unchanged composer snapshot assertion. It expected a payload ending in Frozen snapshot; the observed payload was 05:18 nFrozen snapshot.
Calendar's drag-only proof failed its unchanged label assertion: expected 08:30 – 09:15; observed 08:45 – 09:45. The focused Calendar Escape proof passed.

## Finished Branch: `job/esc-537` Base: `origin/dev` was already up to date at the one required merge check. Head: `f8ab419af968c348dc3e30de769cd71a533cac29` ## Built - Shared Escape selection precedence for Files, Recent, Trash, Photos, Mail, Calendar and Money. Open overlays and text-editing surfaces keep the first Escape; the next Escape clears the selection. - Files row clicks focus their roving option. Escape restores focus to the listbox after selection clears. - A cold `/p/<id>` link returns to that photo's capture day after Escape closes the viewer. - Added focused Mail and Calendar Escape E2Es, expanded menu-first Photos and Files proofs, and added a 1-row / 5,000-row Files Escape performance profile. Files changed: `apps/web/src/lib/files/selection.ts`, `apps/web/src/lib/files/{FilesBrowser,RecentView,TrashView}.svelte`, `packages/ui/src/components/files/FileCollection.svelte`, `apps/web/src/lib/files/{FileCollection.svelte.test.ts,selection.escape.svelte.test.ts}`, `apps/web/src/lib/{mail/MailView,photos/PhotoTimeline,photos/PhotosView}.svelte`, `apps/web/src/routes/calendar/[view]/[date]/+page.svelte`, `apps/web/src/routes/money/[budget]/accounts/[[account]]/+page.svelte`, and E2E/performance evidence in `apps/web/e2e/{calendar,files,mail-escape,money,photos,route-perf}.mjs` plus `apps/web/package.json`. ## Decisions - Mail clears a selected thread to `/mail`: a Mail thread deep link does not retain its category or folder. - Calendar Task previews use the Calendar Escape rule. This checkout has no separate Tasks collection view. - Closing a cold photo deep link returns to its capture day, as the nearest exact timeline view under DESIGN §33. ## Gates `bun run check` (verbatim output): ```text $ node scripts/check-type-tokens.mjs && node scripts/check-motion-tokens.mjs && svelte-kit sync && svelte-check --tsconfig ./tsconfig.json Text sizes and UI shape values use shared role tokens. UI transitions and animation options use shared motion tokens or documented exceptions. Loading svelte-check in workspace: /home/kayg/Developer/calternal-wt/esc-537/apps/web Getting Svelte diagnostics... svelte-check found 0 errors and 0 warnings ``` `bun run test` (verbatim summary): ```text Test Files 141 passed (141) Tests 936 passed (936) Start at 05:31:54 Duration 93.17s (transform 50%, environment 20%, import 16%, tests 10%, setup 4%) ``` Vitest also printed `Not implemented: Window's scrollTo() method` and `Could not parse CSS stylesheet`; exit status was 0. No Rust source changed, so Rust clippy/test gates did not apply. `cargo clean` removed 7,169 files (4.5 GiB). Web build output was removed. ## Production E2E and screenshots - Files Escape proof passed; the screenshot run passed with 0 CSP reports across 21 pages. - Mail Escape proof passed; 12 list/thread captures cover 390, 820 and 1440 px in light and dark. - Photos Escape proof and screenshot-only run passed; both use the production SPA. - Money E2E passed its deep-linked transaction Escape assertion and captured 36 screenshots. I removed the obsolete `auto_scheme.location` test field after the API returned 422; DESIGN §47 puts location in Location settings. - Calendar Escape proof passed at 390, 820 and 1440 px in both themes and captured six screenshots. The ignored local evidence is in `artifacts/esc-537/`: 321 PNGs totaling 169,504,221 bytes, plus `perf.json`. No screenshots are committed. The installed `fj issue comment --help` supports a text body or body file and has no attachment option, so screenshots remain in the worktree. I did not put Money screenshot data on the public issue. ## Performance The shared perf VM lock was busy, so the profile ran locally. Load average rose from `18.31 / 24.59 / 25.19` before the run to `29.80 / 29.94 / 27.64` after it. One selected row: p50 `265.7 ms`, p95 `641.2 ms` (15 samples). Five thousand selected rows: p50 `402.7 ms`, p95 `898.5 ms`; 24 Escape cycles took `27,593.9 ms`. The server RSS averaged about `373.6 MB`; mean server CPU was `1.7%` for one-row clears and `1.34%` for the burst, with peaks near `20%`. `docs/perf/baseline.json` has no Escape series. The local 5k-folder first-row p95 was `1,312 ms` against the profile's `500 ms` budget under the high host load, so it is not a like-for-like regression result. ## Known gaps - The full Photos E2E reached the existing HLS fallback check, then timed out: `canPlayType()` was empty and no master, variant or segment request occurred. The check expectation is unchanged; the focused Photos Escape proof passed. - The full Calendar E2E passed its selected-preview Escape check, then failed the unchanged composer snapshot assertion. It expected a payload ending in ` Frozen snapshot`; the observed payload was `05:18 nFrozen snapshot`. - Calendar's drag-only proof failed its unchanged label assertion: expected `08:30 – 09:15`; observed `08:45 – 09:45`. The focused Calendar Escape proof passed.

kayg referenced this issue

2026-10-01 03:42:42 +00:00

PERF: verification batch on the perf VM for finished branches (#498 #423 #494 #527 #530) #563

kayg referenced this issue

2026-10-01 05:29:13 +00:00

⌘A / Ctrl+A selects all items in the current folder from anywhere in Files (not only when the list has focus) #578

kayg referenced this issue

2026-10-01 10:01:33 +00:00

PERF: verification batch on the perf VM for finished branches (#498 #423 #494 #527 #530) #563

kayg referenced this issue

2026-10-03 12:51:31 +00:00

Issue hygiene sweep 2026-10-03 #1008