Fix performance problem with file drag & drop

- fixes manual use of a svelte action
- pass store to DraggableFile instead of updating selection list
- refactor file selection

apps/desktop/src/lib/branch/BranchLane.svelte

@@ -58,10 +58,10 @@
 	});
 
 	const project = getContext(Project);
-	const fileIdSelection = new FileIdSelection(project.id, branchFiles);
+	const fileIdSelection = new FileIdSelection(branchFiles);
 	const selectedFile = fileIdSelection.selectedFile;
-	const commitId = $derived($selectedFile?.[0]);
-	const selected = $derived($selectedFile?.[1]);
+	const commitId = $derived($selectedFile?.commitId);
+	const selected = $derived($selectedFile?.file);
 	setContext(FileIdSelection, fileIdSelection);
 
 	const userSettings = getContextStoreBySymbol<Settings>(SETTINGS);

apps/desktop/src/lib/components/BranchPreview.svelte

@@ -1,7 +1,6 @@
 <script lang="ts">
 	import BranchPreviewHeader from '../branch/BranchPreviewHeader.svelte';
 	import Resizer from '../shared/Resizer.svelte';
-	import { Project } from '$lib/backend/projects';
 	import CommitCard from '$lib/commit/CommitCard.svelte';
 	import { transformAnyCommit } from '$lib/commitLines/transformers';
 	import Markdown from '$lib/components/Markdown.svelte';
@@ -24,16 +23,15 @@
 	export let remoteBranch: Branch | undefined = undefined;
 	export let pr: PullRequest | undefined;
 
-	const project = getContext(Project);
 	const remoteBranchService = getContext(RemoteBranchService);
 	const forge = getForge();
 
-	const fileIdSelection = new FileIdSelection(project.id, writable([]));
+	const fileIdSelection = new FileIdSelection(writable([]));
 	setContext(FileIdSelection, fileIdSelection);
 
 	const selectedFile = fileIdSelection.selectedFile;
-	$: commitId = $selectedFile?.[0];
-	$: selected = $selectedFile?.[1];
+	$: commitId = $selectedFile?.commitId;
+	$: selected = $selectedFile?.file;
 
 	const defaultBranchWidthRem = 30;
 	const laneWidthKey = 'branchPreviewLaneWidth';

apps/desktop/src/lib/dragging/draggable.ts

@@ -175,6 +175,7 @@ function setupDragHandlers(
 		node.removeEventListener('dragstart', handleDragStart);
 		node.removeEventListener('drag', handleDrag);
 		node.removeEventListener('dragend', handleDragEnd);
+		node.removeEventListener('mousedown', handleMouseDown, { capture: false });
 	}
 
 	setup(opts);

apps/desktop/src/lib/dragging/draggables.ts

@@ -1,3 +1,4 @@
+import { get, type Readable } from 'svelte/store';
 import type {
 	AnyCommit,
 	AnyFile,
@@ -32,12 +33,23 @@ export class DraggableFile {
 		public readonly branchId: string,
 		public file: AnyFile,
 		public commit: AnyCommit | undefined,
-		private selection: AnyFile[] | undefined
+		/**
+		 * When a a file is dragged we compare it to what is already selected,
+		 * if dragged item is part of the selection we consider that to be to
+		 * be dragging all of them. If it is not part of the selection, we
+		 * want to ignore what is selected and only drag the actual file being
+		 * dragged.
+		 */
+		private selection: Readable<AnyFile[]> | undefined
 	) {}
 
 	get files(): AnyFile[] {
-		if (this.selection && this.selection.length > 0) return this.selection;
-		return [this.file];
+		const selectedFiles = this.selection ? get(this.selection) : undefined;
+		if (selectedFiles?.some((selectedFile) => selectedFile.id === this.file.id)) {
+			return selectedFiles;
+		} else {
+			return [this.file];
+		}
 	}
 
 	get isCommitted(): boolean {

apps/desktop/src/lib/file/BranchFilesList.svelte

@@ -9,7 +9,7 @@
 	import { selectFilesInList } from '$lib/utils/selectFilesInList';
 	import { updateSelection } from '$lib/utils/selection';
 	import { getCommitStore } from '$lib/vbranches/contexts';
-	import { FileIdSelection, stringifyFileKey } from '$lib/vbranches/fileIdSelection';
+	import { FileIdSelection, stringifyKey } from '$lib/vbranches/fileIdSelection';
 	import { sortLikeFileTree } from '$lib/vbranches/filetree';
 	import { SelectedOwnership, updateOwnership } from '$lib/vbranches/ownership';
 	import { getContext, maybeGetContextStore } from '@gitbutler/shared/context';
@@ -150,7 +150,7 @@
 				{readonly}
 				{isUnapplied}
 				showCheckbox={showCheckboxes}
-				selected={$fileIdSelection.includes(stringifyFileKey(file.id, $commit?.id))}
+				selected={$fileIdSelection.includes(stringifyKey(file.id, $commit?.id))}
 				onclick={(e) => {
 					selectFilesInList(e, file, fileIdSelection, displayedFiles, allowMultiple, $commit);
 				}}

apps/desktop/src/lib/file/FileListItem.svelte

@@ -1,6 +1,6 @@
 <script lang="ts">
 	import FileContextMenu from './FileContextMenu.svelte';
-	import { draggableChips } from '$lib/dragging/draggable';
+	import { draggableChips, type DraggableConfig } from '$lib/dragging/draggable';
 	import { DraggableFile } from '$lib/dragging/draggables';
 	import { itemsSatisfy } from '$lib/utils/array';
 	import { computeFileStatus } from '$lib/utils/fileStatus';
@@ -29,6 +29,7 @@
 		$props();
 
 	const branch = maybeGetContextStore(VirtualBranch);
+	const branchId = $derived($branch?.id);
 	const selectedOwnership: Writable<SelectedOwnership> | undefined =
 		maybeGetContextStore(SelectedOwnership);
 	const fileIdSelection = getContext(FileIdSelection);
@@ -45,22 +46,12 @@
 	);
 
 	const selectedFiles = fileIdSelection.files;
-
-	const draggableFiles = $derived.by(() => {
-		if ($selectedFiles?.some((selectedFile) => selectedFile.id === file.id)) {
-			return $selectedFiles || [];
-		} else {
-			return [file];
-		}
-	});
+	const draggable = !readonly && !isUnapplied;
 
 	let contextMenu = $state<ReturnType<typeof FileContextMenu>>();
-
 	let draggableEl: HTMLDivElement | undefined = $state();
-	let checked = $state(false);
 	let indeterminate = $state(false);
-
-	const draggable = !readonly && !isUnapplied;
+	let checked = $state(false);
 
 	let animationEndHandler: () => void;
 
@@ -72,6 +63,14 @@
 		element.addEventListener('animationend', animationEndHandler);
 	}
 
+	// TODO: Refactor to use this as a Svelte action, e.g. `use:draggableChips()`.
+	let chips:
+		| {
+				update: (opts: DraggableConfig) => void;
+				destroy: () => void;
+		  }
+		| undefined;
+
 	$effect(() => {
 		if (file && $selectedOwnership) {
 			const hunksContained = itemsSatisfy(file.hunks, (h) =>
@@ -84,20 +83,28 @@
 
 	$effect(() => {
 		if (draggableEl) {
-			draggableChips(draggableEl, {
+			const draggableFile = new DraggableFile(branchId || '', file, $commit, selectedFiles);
+			const config = {
 				label: `${file.filename}`,
 				filePath: file.path,
-				data: new DraggableFile($branch?.id || '', file, $commit, draggableFiles),
+				data: draggableFile,
 				disabled: !draggable,
 				viewportId: 'board-viewport',
 				selector: '.selected-draggable'
-			});
+			};
+			if (chips) {
+				chips.update(config);
+			} else {
+				chips = draggableChips(draggableEl, config);
+			}
+		} else {
+			chips?.destroy();
 		}
 	});
 
 	async function handleDragStart() {
 		// Add animation end listener to files
-		draggableFiles.forEach((f) => {
+		$selectedFiles.forEach((f) => {
 			if (f.locked) {
 				const lockedElement = document.getElementById(`file-${f.id}`);
 				if (lockedElement) {
@@ -109,10 +116,11 @@
 	}
 
 	onDestroy(() => {
+		chips?.destroy();
 		if (draggableEl && animationEndHandler) {
 			draggableEl.removeEventListener('animationend', animationEndHandler);
 		}
-		draggableFiles.forEach((f) => {
+		$selectedFiles.forEach((f) => {
 			const lockedElement = document.getElementById(`file-${f.id}`);
 			if (lockedElement && animationEndHandler) {
 				lockedElement.removeEventListener('animationend', animationEndHandler);
@@ -155,16 +163,16 @@
 			return ownership;
 		});
 
-		if (draggableFiles.length > 0 && draggableFiles.includes(file)) {
+		if ($selectedFiles.length > 0 && $selectedFiles.includes(file)) {
 			if (isChecked) {
-				draggableFiles.forEach((f) => {
+				$selectedFiles.forEach((f) => {
 					selectedOwnership?.update((ownership) => {
 						f.hunks.forEach((h) => ownership.select(f.id, h));
 						return ownership;
 					});
 				});
 			} else {
-				draggableFiles.forEach((f) => {
+				$selectedFiles.forEach((f) => {
 					selectedOwnership?.update((ownership) => {
 						f.hunks.forEach((h) => ownership.ignore(f.id, h.id));
 						return ownership;
@@ -176,7 +184,7 @@
 	ondragstart={handleDragStart}
 	oncontextmenu={(e) => {
 		if (fileIdSelection.has(file.id, $commit?.id)) {
-			contextMenu?.open(e, { files: draggableFiles });
+			contextMenu?.open(e, { files: selectedFiles });
 		} else {
 			contextMenu?.open(e, { files: [file] });
 		}

apps/desktop/src/lib/utils/flattenPromises.ts

@@ -1,17 +0,0 @@
-import { derived, type Readable } from 'svelte/store';
-
-export function flattenPromises<T>(readable: Readable<Promise<T>>): Readable<T | undefined> {
-	return derived(readable, (value, set) => {
-		let discarded = false;
-
-		value.then((value) => {
-			// Don't try to set after the readable has been disposed of
-			if (discarded) return;
-			set(value);
-		});
-
-		return () => {
-			discarded = true;
-		};
-	});
-}

apps/desktop/src/lib/utils/selectFilesInList.ts

@@ -1,5 +1,5 @@
 import { getSelectionDirection } from './getSelectionDirection';
-import { stringifyFileKey, type FileIdSelection } from '$lib/vbranches/fileIdSelection';
+import { type FileIdSelection } from '$lib/vbranches/fileIdSelection';
 import { get } from 'svelte/store';
 import type { AnyCommit, AnyFile } from '$lib/vbranches/types';
 
@@ -19,7 +19,7 @@ export function selectFilesInList(
 		if (isAlreadySelected) {
 			fileIdSelection.remove(file.id, commit?.id);
 		} else {
-			fileIdSelection.add(file.id, commit?.id);
+			fileIdSelection.add(file, commit?.id);
 		}
 	} else if (e.shiftKey && allowMultiple) {
 		// TODO(CTO): Not sure that this is accurate.
@@ -44,20 +44,18 @@ export function selectFilesInList(
 			) + 1
 		);
 
-		selectedFileIds = updatedSelection.map((f) => stringifyFileKey(f.id, commit?.id));
-
 		// if the selection is in the opposite direction, reverse the selection
 		if (selectionDirection === 'down') {
 			selectedFileIds = selectedFileIds.reverse();
 		}
-
-		fileIdSelection.set(selectedFileIds);
+		fileIdSelection.select_many(updatedSelection, commit?.id);
 	} else {
 		// if only one file is selected and it is already selected, unselect it
 		if (selectedFileIds.length === 1 && isAlreadySelected) {
 			fileIdSelection.clear();
 		} else {
-			fileIdSelection.set([stringifyFileKey(file.id, commit?.id)]);
+			fileIdSelection.clear();
+			fileIdSelection.add(file, commit?.id);
 		}
 	}
 }

apps/desktop/src/lib/utils/selection.ts

@@ -3,7 +3,7 @@
  */
 import { getSelectionDirection } from './getSelectionDirection';
 import { KeyName } from './hotkeys';
-import { stringifyFileKey, unstringifyFileKey } from '$lib/vbranches/fileIdSelection';
+import { stringifyKey, unstringifyFileKey } from '$lib/vbranches/fileIdSelection';
 import type { FileIdSelection } from '$lib/vbranches/fileIdSelection';
 import type { AnyFile } from '$lib/vbranches/types';
 
@@ -23,7 +23,7 @@ function getPreviousFile(files: AnyFile[], currentId: string): AnyFile | undefin
 
 function getTopFile(files: AnyFile[], selectedFileIds: string[]): AnyFile | undefined {
 	for (const file of files) {
-		if (selectedFileIds.includes(stringifyFileKey(file.id))) {
+		if (selectedFileIds.includes(stringifyKey(file.id))) {
 			return file;
 		}
 	}
@@ -33,7 +33,7 @@ function getTopFile(files: AnyFile[], selectedFileIds: string[]): AnyFile | unde
 function getBottomFile(files: AnyFile[], selectedFileIds: string[]): AnyFile | undefined {
 	for (let i = files.length - 1; i >= 0; i--) {
 		const file = files[i];
-		if (selectedFileIds.includes(stringifyFileKey(file!.id))) {
+		if (selectedFileIds.includes(stringifyKey(file!.id))) {
 			return file;
 		}
 	}
@@ -83,9 +83,9 @@ export function updateSelection({
 		const file = getFileFunc?.(files, id) ?? getFile(files, id);
 		if (file) {
 			// if file is already selected, do nothing
-			if (selectedFileIds.includes(stringifyFileKey(file.id, commitId))) return;
+			if (selectedFileIds.includes(stringifyKey(file.id, commitId))) return;
 
-			fileIdSelection.add(file.id, commitId);
+			fileIdSelection.add(file, commitId);
 		}
 	}
 
@@ -104,7 +104,7 @@ export function updateSelection({
 		case 'a':
 			if (allowMultiple && metaKey) {
 				for (const file of files) {
-					fileIdSelection.add(file.id, commitId);
+					fileIdSelection.add(file, commitId);
 				}
 			}
 			break;

apps/desktop/src/lib/vbranches/fileIdSelection.ts

@@ -1,15 +1,18 @@
-import { flattenPromises } from '$lib/utils/flattenPromises';
-import { listRemoteCommitFiles } from '$lib/vbranches/remoteCommits';
+import { RemoteFile, type AnyFile, type LocalFile } from '$lib/vbranches/types';
 import { isDefined } from '@gitbutler/ui/utils/typeguards';
-import { derived, type Readable } from 'svelte/store';
-import type { AnyFile, LocalFile } from '$lib/vbranches/types';
+import { get, writable, type Readable, type Unsubscriber } from 'svelte/store';
 
 export interface FileKey {
 	fileId: string;
 	commitId?: string;
 }
 
-export function stringifyFileKey(fileId: string, commitId?: string) {
+export type SelectedFile = {
+	commitId?: string;
+	file: AnyFile;
+};
+
+export function stringifyKey(fileId: string, commitId?: string) {
 	return fileId + '|' + commitId;
 }
 
@@ -26,129 +29,173 @@ export function parseFileKey(fileKeyString: string): FileKey {
 	};
 }
 
-export type SelectedFile = {
-	context?: string;
-	fileId: string;
-};
-
 type CallBack = (value: string[]) => void;
 
+/**
+ * Custom store for managing the set of selected files.
+ */
 export class FileIdSelection implements Readable<string[]> {
-	private value: string[];
+	// It should not be possible to select files from different
+	// so we keep track of the current commit id.
+	private currentCommitId: string | undefined;
+
+	// A string based selection so we do not emit selection changes
+	// when e.g. list_virtual_branches emits.
+	private selection: string[];
+
+	// Subscribed callback functions for this custom store.
 	private callbacks: CallBack[];
 
+	// If there is a commit id, this should hold the file.
+	private remoteFiles = new Map<string, RemoteFile>();
+
+	// Selected file, if selection contains only one file.
+	readonly selectedFile = writable<SelectedFile | undefined>();
+
+	// Currently selected files, refreshed when currently selected
+	// id's have changed.
+	readonly files = writable<AnyFile[]>([]);
+
+	// Unsubscribe function for the readable of uncommitted files.
+	private unsubscribeLocalFiles: Unsubscriber | undefined;
+
 	constructor(
-		private projectId: string,
-		private localFiles: Readable<LocalFile[]>,
+		private uncommittedFiles: Readable<LocalFile[]>,
 		value: FileKey[] = []
 	) {
 		this.callbacks = [];
-		this.value = value.map((key) => stringifyFileKey(key.fileId, key.commitId));
+		this.selection = value.map((key) => stringifyKey(key.fileId, key.commitId));
 	}
 
 	subscribe(callback: (value: string[]) => void) {
-		callback(this.value);
+		callback(this.selection);
 		this.callbacks.push(callback);
+		if (this.callbacks.length === 1) {
+			this.setup();
+		}
 		return () => this.unsubscribe(callback);
 	}
 
 	private unsubscribe(callback: CallBack) {
 		this.callbacks = this.callbacks.filter((cb) => cb !== callback);
+		if (this.callbacks.length === 0) {
+			this.teardown();
+		}
 	}
 
-	add(fileId: string, commitId?: string) {
-		const fileKey = stringifyFileKey(fileId, commitId);
-		if (!this.value.includes(fileKey)) {
-			this.value.push(fileKey);
+	/**
+	 * Calls each subscriber with the latest selection.
+	 */
+	private emit() {
+		for (const callback of this.callbacks) {
+			callback(this.selection);
+		}
+	}
+
+	/**
+	 * Called when subscriber count goes from 1 -> 0.
+	 */
+	async setup() {
+		this.unsubscribeLocalFiles = this.uncommittedFiles.subscribe(() => {
+			this.clear();
+		});
+	}
+
+	/**
+	 * Called when subscriber count goes from 0 -> 1.
+	 */
+	teardown() {
+		this.unsubscribeLocalFiles?.();
+		this.clear();
+	}
+
+	/**
+	 * Selection is managed as string values to deduplicate events, we therefore
+	 * need a way of keeping a corresponding list of files up-to-date.
+	 */
+	async reloadFiles() {
+		const files = this.selection
+			.map((selected) => {
+				const key = parseFileKey(selected);
+				return this.findFileByKey(key);
+			})
+			.filter(isDefined);
+
+		this.files.set(files);
+		if (files.length === 1) {
+			this.selectedFile.set({
+				commitId: this.currentCommitId,
+				file: files[0] as AnyFile
+			});
+		} else {
+			this.selectedFile.set(undefined);
+		}
+	}
+
+	add(file: AnyFile, commitId?: string) {
+		this.select_many([file], commitId);
+	}
+
+	select_many(files: AnyFile[], commitId?: string) {
+		if (this.selection.length > 0 && commitId !== this.currentCommitId) {
+			throw 'Selecting files from multiple commits not allowed.';
+		}
+		for (const file of files) {
+			const fileKey = stringifyKey(file.id, commitId);
+			if (!this.selection.includes(fileKey)) {
+				this.selection.push(fileKey);
+				if (file instanceof RemoteFile) {
+					this.remoteFiles.set(fileKey, file);
+				}
+			}
+		}
+		this.emit();
+		this.reloadFiles();
+	}
+
+	has(fileId: string, commitId?: string) {
+		return this.selection.includes(stringifyKey(fileId, commitId));
+	}
+
+	remove(fileId: string, commitId?: string) {
+		const strFileKey = stringifyKey(fileId, commitId);
+		this.selection = this.selection.filter((key) => key !== strFileKey);
+		if (commitId) {
+			this.remoteFiles.delete(strFileKey);
+		}
+		if (this.selection.length === 0) {
+			this.clear();
+		} else {
+			this.reloadFiles();
 			this.emit();
 		}
 	}
 
-	has(fileId: string, commitId?: string) {
-		return this.value.includes(stringifyFileKey(fileId, commitId));
-	}
-
-	remove(fileId: string, commitId?: string) {
-		this.value = this.value.filter((key) => key !== stringifyFileKey(fileId, commitId));
-		this.emit();
-	}
-
-	set(values: string[]) {
-		this.value = values;
-		this.emit();
-	}
-
 	clear() {
-		this.value = [];
+		this.selection = [];
+		this.remoteFiles.clear();
+		this.currentCommitId = undefined;
+		this.selectedFile.set(undefined);
 		this.emit();
 	}
 
 	clearExcept(fileId: string, commitId?: string) {
-		this.value = [stringifyFileKey(fileId, commitId)];
+		this.selection = [stringifyKey(fileId, commitId)];
+		this.reloadFiles();
 		this.emit();
 	}
 
-	private emit() {
-		for (const callback of this.callbacks) {
-			callback(this.value);
-		}
-	}
-
 	only(): FileKey | undefined {
-		if (this.value.length === 0) return;
-		const fileKey = parseFileKey(this.value[0]!);
+		if (this.selection.length === 0) return;
+		const fileKey = parseFileKey(this.selection[0]!);
 		return fileKey;
 	}
 
-	#selectedFile: Readable<[string | undefined, AnyFile | undefined] | undefined> | undefined;
-	get selectedFile() {
-		if (this.#selectedFile) return this.#selectedFile;
-
-		const files = derived(
-			[this as Readable<string[]>, this.localFiles],
-			async ([selection, localFiles]): Promise<
-				[string | undefined, AnyFile | undefined] | undefined
-			> => {
-				if (selection.length !== 1) return undefined;
-				const fileKey = parseFileKey(selection[0]!);
-				const file = await findFileByKey(localFiles, this.projectId, fileKey);
-				return [fileKey.commitId, file];
-			}
-		);
-
-		this.#selectedFile = flattenPromises(files);
-
-		return this.#selectedFile;
-	}
-
-	#files: Readable<AnyFile[] | undefined> | undefined;
-	get files() {
-		if (this.#files) return this.#files;
-
-		const files = derived(
-			[this as Readable<string[]>, this.localFiles],
-			async ([selection, localFiles]): Promise<AnyFile[]> => {
-				const files = await Promise.all(
-					selection.map(async (fileKey) => {
-						return await findFileByKey(localFiles, this.projectId, parseFileKey(fileKey));
-					})
-				);
-
-				return files.filter(isDefined);
-			}
-		);
-
-		this.#files = flattenPromises(files);
-
-		return this.#files;
-	}
-}
-
-async function findFileByKey(localFiles: LocalFile[], projectId: string, key: FileKey) {
-	if (key.commitId) {
-		const remoteFiles = await listRemoteCommitFiles(projectId, key.commitId);
-		return remoteFiles.find((file) => file.id === key.fileId);
-	} else {
-		return localFiles.find((file) => file.id === key.fileId);
+	findFileByKey(key: FileKey) {
+		if (key.commitId) {
+			return this.remoteFiles.get(stringifyKey(key.fileId, key.commitId));
+		} else {
+			return get(this.uncommittedFiles).find((file) => file.id === key.fileId);
+		}
 	}
 }

apps/desktop/src/routes/[projectId]/base/+page.svelte

@@ -1,5 +1,4 @@
 <script lang="ts">
-	import { Project } from '$lib/backend/projects';
 	import { BaseBranchService } from '$lib/baseBranch/baseBranchService';
 	import BaseBranch from '$lib/components/BaseBranch.svelte';
 	import FullviewLoading from '$lib/components/FullviewLoading.svelte';
@@ -19,15 +18,14 @@
 
 	const baseBranchService = getContext(BaseBranchService);
 	const baseBranch = baseBranchService.base;
-	const project = getContext(Project);
 
-	const fileIdSelection = new FileIdSelection(project.id, writable([]));
+	const fileIdSelection = new FileIdSelection(writable([]));
 	setContext(FileIdSelection, fileIdSelection);
 
 	const selectedFile = fileIdSelection.selectedFile;
 
-	$: commitId = $selectedFile?.[0];
-	$: selected = $selectedFile?.[1];
+	$: commitId = $selectedFile?.commitId;
+	$: selected = $selectedFile?.file;
 
 	let rsViewport: HTMLDivElement;
 	let laneWidth: number;