Skip to content

Commit a0be5b9

Browse files
committed
Derive selection stores and subscribe to lastAdded
Convert computed getters for selected paths and list selection into derived stores and replace the derived-lastAdded setup with a direct subscription. This aligns selection state with reactive stores, avoids creating intermediate derived getters, and ensures focus sync uses the latest lastAdded value via subscribe so focus updates happen reliably when active. Return true from key handlers and guard onclick onselect Fix navigation and selection handling by making keyboard handlers return true when they successfully handle an event, and by guarding onclick onselect so it only fires when the clicked item is selected. Previously keyboard activation and extra handlers returned nothing which prevented higher-level logic from stopping further processing; navigation also didn't short-circuit so focus could move to the wrong index. The onclick change prevents emitting an onselect callback for a click that doesn't actually result in selection. Only claim navigation events when index changes Prevent the FileListItems component from swallowing arrow/vim navigation events if the navigation result doesn't move focus. Previously, reaching the end of the list would still be treated as handled and would not fall through to the focus manager, preventing users from moving to the next commit. This change makes the component only claim the event when the navigated index actually differs from the current index, restoring the expected fall-through behavior.
1 parent bb1c38d commit a0be5b9

2 files changed

Lines changed: 25 additions & 25 deletions

File tree

apps/desktop/src/components/FileListItems.svelte

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,27 +127,32 @@
127127
// 1. Activation keys (Enter/Space/l)
128128
if (controller.handleActivation(change, idx, e)) {
129129
onselect?.(change, idx);
130-
return;
130+
return true;
131131
}
132132
// 2. Extra handlers (e.g. AI shortcuts)
133133
if (extraKeyHandlers) {
134134
for (const handler of extraKeyHandlers) {
135-
if (handler(change, idx, e)) return;
135+
if (handler(change, idx, e)) return true;
136136
}
137137
}
138-
// 3. Arrow/vim navigation
138+
// 3. Arrow/vim navigation — only claim the event if we moved
139139
const navigatedIndex = controller.handleNavigation(e);
140-
if (navigatedIndex !== undefined) {
140+
if (navigatedIndex !== undefined && navigatedIndex !== idx) {
141141
const navigatedChange = controller.changes[navigatedIndex];
142-
if (navigatedChange) onselect?.(navigatedChange, navigatedIndex);
142+
if (navigatedChange) {
143+
onselect?.(navigatedChange, navigatedIndex);
144+
}
145+
return true;
143146
}
144147
},
145148
focusable: true,
146149
}}
147150
onclick={(e) => {
148151
e.stopPropagation();
149152
controller.select(e, change, idx);
150-
onselect?.(change, idx);
153+
if (controller.isSelected(change.path)) {
154+
onselect?.(change, idx);
155+
}
151156
}}
152157
{conflictEntries}
153158
/>

apps/desktop/src/lib/selection/fileListController.svelte.ts

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ export class FileListController {
5858
private getAllowUnselect: () => boolean;
5959

6060
active = $state(false);
61+
readonly selectedPaths = $derived(new Set(this.selectedFileIds.map((f) => f.path)));
62+
readonly hasSelectionInList = $derived(
63+
this.changes.some((change) => this.selectedPaths.has(change.path)),
64+
);
6165

6266
constructor(params: {
6367
changes: () => TreeChange[];
@@ -70,18 +74,17 @@ export class FileListController {
7074
this.getSelectionId = params.selectionId;
7175
this.getAllowUnselect = params.allowUnselect ?? (() => true);
7276

73-
const currentSelection = $derived(this.idSelection.getById(this.getSelectionId()));
74-
const lastAdded = $derived(currentSelection.lastAdded);
75-
const lastAddedIndex = $derived(get(lastAdded)?.index);
76-
7777
$effect(() => {
78-
if (lastAddedIndex !== undefined) {
79-
untrack(() => {
80-
if (this.active) {
81-
this.focusManager.focusNthSibling(lastAddedIndex);
82-
}
83-
});
84-
}
78+
const store = this.idSelection.getById(this.getSelectionId()).lastAdded;
79+
return store.subscribe((value) => {
80+
if (value?.index !== undefined) {
81+
untrack(() => {
82+
if (this.active) {
83+
this.focusManager.focusNthSibling(value.index);
84+
}
85+
});
86+
}
87+
});
8588
});
8689
}
8790

@@ -101,14 +104,6 @@ export class FileListController {
101104
return this.idSelection.values(this.selectionId);
102105
}
103106

104-
get selectedPaths(): Set<string> {
105-
return new Set(this.selectedFileIds.map((f) => f.path));
106-
}
107-
108-
get hasSelectionInList(): boolean {
109-
return this.changes.some((change) => this.selectedPaths.has(change.path));
110-
}
111-
112107
isSelected(path: string): boolean {
113108
return this.idSelection.has(path, this.selectionId);
114109
}

0 commit comments

Comments
 (0)