Skip to content

Commit 0787779

Browse files
committed
Go to path: Backend file path, a11y, cleanup
- Make the `File` recents path backend-authoritative: `GoToPathResolution::File` now carries the canonical normalized `path`, so the handler records `resolution.path` directly instead of reconstructing `${parentDir}/${fileName}`. - Drop the dead `getRecentPathsLoaded` helper (no callers); the internal `loaded` idempotency flag stays. - Make the recents `[x]` remove button keyboard-accessible: removed its `tabindex="-1"` so it joins the natural tab order (textbox → each `[x]` → Cancel → Go to path), keeping the existing `aria-label` and `--shadow-focus` focus ring. Row body stays out of the tab order (digit keys cover jumping). - Regenerated typed bindings; updated Rust + Vitest tests and the colocated CLAUDE.md docs.
1 parent 3a768fc commit 0787779

10 files changed

Lines changed: 64 additions & 23 deletions

File tree

apps/desktop/src-tauri/src/go_to_path/CLAUDE.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@ all the path reasoning plus the recent-paths store. The IPC layer (`commands/go_
1212

1313
## `GoToPathResolution`
1414

15-
The resolution outcome the frontend branches on. Four variants: `Directory { path }`, `File { parentDir, fileName }`,
16-
`NearestAncestor { requested, ancestorDir }`, `Invalid { reason }`. The tagged-enum attrs
15+
The resolution outcome the frontend branches on. Four variants: `Directory { path }`,
16+
`File { path, parentDir, fileName }`, `NearestAncestor { requested, ancestorDir }`, `Invalid { reason }`. The `File`
17+
variant carries the canonical normalized full `path` (the frontend records it into recents verbatim, no client-side
18+
reconstruction) alongside `parentDir` / `fileName` (which drive navigate-to-parent + select). The tagged-enum attrs
1719
(`#[serde(tag = "kind", rename_all = "camelCase", rename_all_fields = "camelCase")]`) are required so the tag and the
1820
struct-variant fields ship camelCase through tauri-specta (enforced by `ipc-enum-camelcase`).
1921

apps/desktop/src-tauri/src/go_to_path/mod.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,14 @@ use crate::commands::file_system::expand_tilde;
6666
pub enum GoToPathResolution {
6767
/// The resolved path is an existing directory. Navigate into it.
6868
Directory { path: String },
69-
/// The resolved path is an existing file. Navigate to its parent and select it.
70-
File { parent_dir: String, file_name: String },
69+
/// The resolved path is an existing file. `path` is the canonical normalized
70+
/// full path (recorded into recents); `parent_dir` / `file_name` drive the
71+
/// navigate-to-parent + select.
72+
File {
73+
path: String,
74+
parent_dir: String,
75+
file_name: String,
76+
},
7177
/// The resolved path doesn't exist. `ancestor_dir` is the nearest existing
7278
/// ancestor (worst case `/`). Navigate there and fire an INFO toast.
7379
NearestAncestor { requested: String, ancestor_dir: String },
@@ -158,7 +164,11 @@ pub fn resolve(input: &str, base_dir: &str) -> GoToPathResolution {
158164
.file_name()
159165
.map(|n| n.to_string_lossy().to_string())
160166
.unwrap_or_else(|| normalized_str.clone());
161-
GoToPathResolution::File { parent_dir, file_name }
167+
GoToPathResolution::File {
168+
path: normalized_str,
169+
parent_dir,
170+
file_name,
171+
}
162172
}
163173
Err(_) => {
164174
// Doesn't exist (or unreadable): fall back to the nearest ancestor.
@@ -201,6 +211,7 @@ mod tests {
201211
assert_eq!(
202212
res,
203213
GoToPathResolution::File {
214+
path: file.to_string_lossy().to_string(),
204215
parent_dir: dir.path().to_string_lossy().to_string(),
205216
file_name: "notes.txt".to_string(),
206217
}
@@ -309,11 +320,13 @@ mod tests {
309320
#[test]
310321
fn resolution_serializes_with_camelcase_kind_and_fields() {
311322
let res = GoToPathResolution::File {
323+
path: "/Users/x/a.txt".to_string(),
312324
parent_dir: "/Users/x".to_string(),
313325
file_name: "a.txt".to_string(),
314326
};
315327
let json = serde_json::to_string(&res).unwrap();
316328
assert!(json.contains("\"kind\":\"file\""), "got {json}");
329+
assert!(json.contains("\"path\""), "got {json}");
317330
assert!(json.contains("\"parentDir\""), "got {json}");
318331
assert!(json.contains("\"fileName\""), "got {json}");
319332

apps/desktop/src/lib/go-to-path/CLAUDE.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ actual jump — one source of truth, no drift.
5353
all three; the `$state` mirror re-reads the authoritative list after each write rather than guessing the new order.
5454
- Rows are middle-truncated for display (`useShortenMiddle`, `preferBreakAt: '/'`) with the full path in a `title`
5555
tooltip (`tooltipWhenTruncated`).
56+
- **Keyboard access.** The row body is out of the tab order (`tabindex="-1"`): the digit keys (1-9, 0) are the keyboard
57+
path to jumping a recent, so tabbing every row body would be redundant. The `[x]` remove button keeps its natural tab
58+
order (a real `<button>` with `aria-label="Remove from list"` and a `--shadow-focus` `:focus-visible` ring) so
59+
keyboard-only users can remove a recent — digits can't express "remove." Removing the row refocuses the textbox.
5660

5761
## Key decisions
5862

@@ -79,8 +83,8 @@ Matches the downloads toast snapshot rule.
7983

8084
A command with a native menu accelerator AND a `command-registry` shortcut fires both paths on macOS. The
8185
`showGoToPathDialog` callback in `+page.svelte` guards with `if (show && showGoToPathDialog) return`, so a double-fire
82-
opens the dialog exactly once. (The native Go-menu item lands in M3; this guard is already in place so it'll be correct
83-
when it does.)
86+
opens the dialog exactly once. The native `Go to path…` menu item carries ⌘G as an accelerator, so this guard is what
87+
keeps a single ⌘G press from opening the dialog twice.
8488

8589
## v1 limitations
8690

apps/desktop/src/lib/go-to-path/GoToPathDialog.svelte

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,11 @@
223223
<ul class="recents" aria-label="Recent paths">
224224
{#each recents as recent, index (recent.id)}
225225
<li class="recent-row">
226+
<!-- Row body is out of the tab order on purpose: the digit
227+
keys (1-9, 0) are the keyboard path to jumping a recent,
228+
so tabbing through every row body would be redundant. The
229+
`[x]` remove button keeps its natural tab order so keyboard
230+
users can remove a recent (digits can't express that). -->
226231
<button
227232
type="button"
228233
class="recent-main"
@@ -245,7 +250,6 @@
245250
aria-label="Remove from list"
246251
use:tooltip={'Remove from list'}
247252
onclick={(event) => void handleRemoveRecent(event, recent.id)}
248-
tabindex="-1"
249253
>
250254
<XIcon />
251255
</button>

apps/desktop/src/lib/go-to-path/GoToPathDialog.svelte.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,27 @@ describe('GoToPathDialog', () => {
163163
cleanup()
164164
})
165165

166+
it('the remove button is keyboard-reachable and operable', async () => {
167+
getRecentPathsListMock.mockReturnValue([{ id: 'a', path: '/first', timestamp: 1 }])
168+
const onGo = goMock({ kind: 'directory', path: '/x' })
169+
const { target, cleanup } = setup({ onGo })
170+
await tick()
171+
const removeButton = target.querySelector('.remove-button') as HTMLButtonElement
172+
// A real `<button>` in the natural tab order: no negative tabindex removing
173+
// it. Keyboard-only users can focus and operate it (Enter/Space dispatch a
174+
// native click on a button), so it must not be excluded from tabbing.
175+
expect(removeButton.getAttribute('tabindex')).toBeNull()
176+
expect(removeButton.getAttribute('aria-label')).toBe('Remove from list')
177+
// Enter/Space on a focused native button fire a `click`; simulate that path.
178+
removeButton.focus()
179+
expect(target.contains(document.activeElement)).toBe(true)
180+
removeButton.dispatchEvent(new MouseEvent('click', { bubbles: true }))
181+
await tick()
182+
expect(removeRecentPathMock).toHaveBeenCalledWith('a')
183+
expect(onGo).not.toHaveBeenCalled()
184+
cleanup()
185+
})
186+
166187
it('Enter confirms: jumps and closes on a directory outcome', async () => {
167188
const onGo = goMock({ kind: 'directory', path: '/typed' })
168189
const { target, onCancel, cleanup } = setup({ onGo })

apps/desktop/src/lib/go-to-path/go-to-path.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ describe('goToPath handler', () => {
9494
expect(addRecentPathStateMock).toHaveBeenCalledWith(expect.objectContaining({ path: '/tmp/here' }))
9595
})
9696

97-
it('file → navigateToFileInPane + records the full file path', async () => {
98-
okResolve({ kind: 'file', parentDir: '/tmp', fileName: 'a.txt' })
97+
it('file → navigateToFileInPane + records the backend-authoritative path', async () => {
98+
okResolve({ kind: 'file', path: '/tmp/a.txt', parentDir: '/tmp', fileName: 'a.txt' })
9999
await goToPath(makeExplorerStub(), '/tmp/a.txt')
100100
expect(navigateToFileMock).toHaveBeenCalledWith(expect.anything(), 'left', '/tmp', 'a.txt')
101101
expect(navigateToDirMock).not.toHaveBeenCalled()
@@ -184,7 +184,7 @@ describe('digitToRecentIndex', () => {
184184
describe('shouldPrefillClipboard', () => {
185185
it('is true for directory and file resolutions', () => {
186186
expect(shouldPrefillClipboard({ kind: 'directory', path: '/x' })).toBe(true)
187-
expect(shouldPrefillClipboard({ kind: 'file', parentDir: '/x', fileName: 'a' })).toBe(true)
187+
expect(shouldPrefillClipboard({ kind: 'file', path: '/x/a', parentDir: '/x', fileName: 'a' })).toBe(true)
188188
})
189189

190190
it('is false for nearestAncestor and invalid resolutions', () => {

apps/desktop/src/lib/go-to-path/go-to-path.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ export async function goToPath(
6969
return resolution
7070
case 'file':
7171
await navigateToFileInPane(explorer, pane, resolution.parentDir, resolution.fileName)
72-
await recordRecent(`${resolution.parentDir}/${resolution.fileName}`.replace(/\/+/g, '/'))
72+
await recordRecent(resolution.path)
7373
return resolution
7474
case 'nearestAncestor': {
7575
await navigateToDirInPane(explorer, pane, resolution.ancestorDir)

apps/desktop/src/lib/go-to-path/recent-paths-state.svelte.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,6 @@ export function getRecentPathsList(): RecentPathEntry[] {
2828
return entries
2929
}
3030

31-
/** Whether `loadRecentPaths()` has completed at least once this session. */
32-
export function getRecentPathsLoaded(): boolean {
33-
return loaded
34-
}
35-
3631
/**
3732
* Loads the persisted recent paths from the backend. Idempotent: subsequent
3833
* calls in the same session are no-ops unless `force` is set. The dialog calls

apps/desktop/src/lib/go-to-path/recent-paths-state.test.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ vi.mock('$lib/ipc/bindings', () => ({
2222

2323
import {
2424
getRecentPathsList,
25-
getRecentPathsLoaded,
2625
loadRecentPaths,
2726
addRecentPath,
2827
removeRecentPath,
@@ -67,12 +66,11 @@ describe('recent-paths-state mirror', () => {
6766
})
6867
})
6968

70-
it('load() pulls the backend list and marks loaded', async () => {
69+
it('load() pulls the backend list', async () => {
7170
store = [makeEntry('/a'), makeEntry('/b')]
72-
expect(getRecentPathsLoaded()).toBe(false)
71+
expect(getRecentPathsList()).toEqual([])
7372
await loadRecentPaths()
7473
expect(getRecentPathsList().map((e) => e.path)).toEqual(['/a', '/b'])
75-
expect(getRecentPathsLoaded()).toBe(true)
7674
})
7775

7876
it('load() is idempotent unless forced', async () => {

apps/desktop/src/lib/ipc/bindings.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2703,8 +2703,12 @@ export type GoToLatestError =
27032703
export type GoToPathResolution =
27042704
// The resolved path is an existing directory. Navigate into it.
27052705
| { kind: 'directory'; path: string }
2706-
// The resolved path is an existing file. Navigate to its parent and select it.
2707-
| { kind: 'file'; parentDir: string; fileName: string }
2706+
/**
2707+
* The resolved path is an existing file. `path` is the canonical normalized
2708+
* full path (recorded into recents); `parent_dir` / `file_name` drive the
2709+
* navigate-to-parent + select.
2710+
*/
2711+
| { kind: 'file'; path: string; parentDir: string; fileName: string }
27082712
/**
27092713
* The resolved path doesn't exist. `ancestor_dir` is the nearest existing
27102714
* ancestor (worst case `/`). Navigate there and fire an INFO toast.

0 commit comments

Comments
 (0)