File selection improvements (#4134)

PavelLaptev · web-flow · commit 8578ba32ff4a · 2024-06-20T16:50:10.000+02:00
* Select multiple files with arrow keys + Shift

There are some corner cases to cover, but it serves the basic usage

* Refactor: remove unused code

Looks like this code was used for the tree view structure

* Check/uncheck files based on the file selection

* Lint error fixes
diff --git a/app/src/lib/components/BranchFilesList.svelte b/app/src/lib/components/BranchFilesList.svelte
@@ -92,7 +92,15 @@
 				}}
 				on:keydown={(e) => {
 					e.preventDefault();
-					maybeMoveSelection(e.key, file, displayedFiles, fileIdSelection);
+					maybeMoveSelection(
+						allowMultiple,
+						e.shiftKey,
+						e.key,
+						file,
+						displayedFiles,
+						$fileIdSelection,
+						fileIdSelection
+					);
 				}}
 			/>
 		{/each}
diff --git a/app/src/lib/components/FileListItem.svelte b/app/src/lib/components/FileListItem.svelte
@@ -29,18 +29,17 @@
 	const selectedFiles = fileIdSelection.files;
 
 	let checked = false;
-	let indeterminate = false;
 	let draggableElt: HTMLDivElement;
 
 	$: if (file && $selectedOwnership) {
-		const fileId = file.id;
-		checked = file.hunks.every((hunk) => $selectedOwnership?.contains(fileId, hunk.id));
-		const selectedCount = file.hunks.filter((hunk) =>
-			$selectedOwnership?.contains(fileId, hunk.id)
-		).length;
-		indeterminate = selectedCount > 0 && file.hunks.length - selectedCount > 0;
+		checked = file.hunks.every((hunk) => $selectedOwnership?.contains(file.id, hunk.id));
 	}
 
+	$: if ($fileIdSelection && draggableElt)
+		updateFocus(draggableElt, file, fileIdSelection, $commit?.id);
+
+	$: popupMenu = updateContextMenu();
+
 	function updateContextMenu() {
 		if (popupMenu) unmount(popupMenu);
 		return mount(FileContextMenu, {
@@ -49,11 +48,6 @@
 		});
 	}
 
-	$: if ($fileIdSelection && draggableElt)
-		updateFocus(draggableElt, file, fileIdSelection, $commit?.id);
-
-	$: popupMenu = updateContextMenu();
-
 	onDestroy(() => {
 		if (popupMenu) {
 			unmount(popupMenu);
@@ -124,13 +118,36 @@
 		<Checkbox
 			small
 			{checked}
-			{indeterminate}
 			on:change={(e) => {
+				const isChecked = e.detail;
 				selectedOwnership?.update((ownership) => {
-					if (e.detail) file.hunks.forEach((h) => ownership.add(file.id, h));
-					if (!e.detail) file.hunks.forEach((h) => ownership.remove(file.id, h.id));
+					if (isChecked) {
+						file.hunks.forEach((h) => ownership.add(file.id, h));
+					} else {
+						file.hunks.forEach((h) => ownership.remove(file.id, h.id));
+					}
 					return ownership;
 				});
+
+				$selectedFiles.then((files) => {
+					if (files.length > 0 && files.includes(file)) {
+						if (isChecked) {
+							files.forEach((f) => {
+								selectedOwnership?.update((ownership) => {
+									f.hunks.forEach((h) => ownership.add(f.id, h));
+									return ownership;
+								});
+							});
+						} else {
+							files.forEach((f) => {
+								selectedOwnership?.update((ownership) => {
+									f.hunks.forEach((h) => ownership.remove(f.id, h.id));
+									return ownership;
+								});
+							});
+						}
+					}
+				});
 			}}
 		/>
 	{/if}
@@ -176,14 +193,9 @@
 	}
 
 	.draggable {
-		/* cursor: grab; */
-
 		&:hover {
 			& .draggable-handle {
-				/* width: 10px; */
-				/* width: 6px; */
 				opacity: 1;
-				/* transition-delay: 0.5s; */
 			}
 		}
 	}
@@ -200,8 +212,6 @@
 		transition:
 			width var(--transition-fast),
 			opacity var(--transition-fast);
-		/* transition-delay: 0s; */
-		/* background-color: rgb(184, 150, 201); */
 	}
 
 	.info {
diff --git a/app/src/lib/utils/getSelectionDirection.ts b/app/src/lib/utils/getSelectionDirection.ts
@@ -0,0 +1,6 @@
+export function getSelectionDirection(firstFileIndex: number, lastFileIndex: number) {
+	// detect the direction of the selection
+	const selectionDirection = lastFileIndex < firstFileIndex ? 'down' : 'up';
+
+	return selectionDirection;
+}
diff --git a/app/src/lib/utils/selectFilesInList.ts b/app/src/lib/utils/selectFilesInList.ts
@@ -1,3 +1,4 @@
+import { getSelectionDirection } from './getSelectionDirection';
 import { stringifyFileKey, type FileIdSelection } from '$lib/vbranches/fileIdSelection';
 import { get } from 'svelte/store';
 import type { AnyCommit, AnyFile } from '$lib/vbranches/types';
@@ -26,8 +27,10 @@ export function selectFilesInList(
 		);
 
 		// detect the direction of the selection
-		const selectionDirection =
-			initiallySelectedIndex < sortedFiles.findIndex((f) => f.id === file.id) ? 'down' : 'up';
+		const selectionDirection = getSelectionDirection(
+			initiallySelectedIndex,
+			sortedFiles.findIndex((f) => f.id === file.id)
+		);
 
 		const updatedSelection = sortedFiles.slice(
 			Math.min(
@@ -42,9 +45,11 @@ export function selectFilesInList(
 
 		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);
 	} else {
 		// if only one file is selected and it is already selected, unselect it
diff --git a/app/src/lib/utils/selection.ts b/app/src/lib/utils/selection.ts
@@ -1,25 +1,19 @@
 /**
  * Shared helper functions for manipulating selected files with keyboard.
  */
+import { getSelectionDirection } from './getSelectionDirection';
+import { stringifyFileKey, unstringifyFileKey } from '$lib/vbranches/fileIdSelection';
 import type { FileIdSelection } from '$lib/vbranches/fileIdSelection';
 import type { AnyFile } from '$lib/vbranches/types';
 
-export function getNextFile(files: AnyFile[], current: string) {
-	const fileIndex = files.findIndex((f) => f.id === current);
-	if (fileIndex !== -1 && fileIndex + 1 < files.length) return files[fileIndex + 1];
+export function getNextFile(files: AnyFile[], currentId: string): AnyFile | undefined {
+	const fileIndex = files.findIndex((f) => f.id === currentId);
+	return fileIndex !== -1 && fileIndex + 1 < files.length ? files[fileIndex + 1] : undefined;
 }
 
-export function getPreviousFile(files: AnyFile[], current: string) {
-	const fileIndex = files.findIndex((f) => f.id === current);
-	if (fileIndex > 0) return files[fileIndex - 1];
-}
-
-export function getFileByKey(key: string, current: string, files: AnyFile[]): AnyFile | undefined {
-	if (key === 'ArrowUp') {
-		return getPreviousFile(files, current);
-	} else if (key === 'ArrowDown') {
-		return getNextFile(files, current);
-	}
+export function getPreviousFile(files: AnyFile[], currentId: string): AnyFile | undefined {
+	const fileIndex = files.findIndex((f) => f.id === currentId);
+	return fileIndex > 0 ? files[fileIndex - 1] : undefined;
 }
 
 /**
@@ -36,21 +30,92 @@ export function updateFocus(
 	commitId?: string
 ) {
 	const selected = fileIdSelection.only();
-	if (!selected) return;
-	if (selected.fileId === file.id && selected.commitId === commitId) elt.focus();
+	if (selected && selected.fileId === file.id && selected.commitId === commitId) {
+		elt.focus();
+	}
 }
 
 export function maybeMoveSelection(
+	allowMultiple: boolean,
+	shiftKey: boolean,
 	key: string,
 	file: AnyFile,
 	files: AnyFile[],
+	selectedFileIds: string[],
 	fileIdSelection: FileIdSelection
 ) {
-	if (key !== 'ArrowUp' && key !== 'ArrowDown') return;
+	if (selectedFileIds.length === 0) return;
+
+	const firstFileId = unstringifyFileKey(selectedFileIds[0]);
+	const lastFileId = unstringifyFileKey(selectedFileIds[selectedFileIds.length - 1]);
+	let selectionDirection = getSelectionDirection(
+		files.findIndex((f) => f.id === lastFileId),
+		files.findIndex((f) => f.id === firstFileId)
+	);
+
+	function getAndAddFile(
+		getFileFunc: (files: AnyFile[], id: string) => AnyFile | undefined,
+		id: string
+	) {
+		const file = getFileFunc(files, id);
+		if (file) {
+			// if file is already selected, do nothing
+			if (selectedFileIds.includes(stringifyFileKey(file.id))) return;
+
+			fileIdSelection.add(file.id);
+		}
+	}
+
+	function getAndClearAndAddFile(
+		getFileFunc: (files: AnyFile[], id: string) => AnyFile | undefined,
+		id: string
+	) {
+		const file = getFileFunc(files, id);
+		if (file) {
+			fileIdSelection.clear();
+			fileIdSelection.add(file.id);
+		}
+	}
+
+	switch (key) {
+		case 'ArrowUp':
+			if (shiftKey && allowMultiple) {
+				// Handle case if only one file is selected
+				// we should update the selection direction
+				if (selectedFileIds.length === 1) {
+					selectionDirection = 'up';
+				} else if (selectionDirection === 'down') {
+					fileIdSelection.remove(lastFileId);
+				}
+				getAndAddFile(getPreviousFile, lastFileId);
+			} else {
+				// Handle reset of selection
+				if (selectedFileIds.length > 1) {
+					getAndClearAndAddFile(getPreviousFile, lastFileId);
+				} else {
+					getAndClearAndAddFile(getPreviousFile, file.id);
+				}
+			}
+			break;
 
-	const newSelection = getFileByKey(key, file.id, files);
-	if (newSelection) {
-		fileIdSelection.clear();
-		fileIdSelection.add(newSelection.id);
+		case 'ArrowDown':
+			if (shiftKey && allowMultiple) {
+				// Handle case if only one file is selected
+				// we should update the selection direction
+				if (selectedFileIds.length === 1) {
+					selectionDirection = 'down';
+				} else if (selectionDirection === 'up') {
+					fileIdSelection.remove(lastFileId);
+				}
+				getAndAddFile(getNextFile, lastFileId);
+			} else {
+				// Handle reset of selection
+				if (selectedFileIds.length > 1) {
+					getAndClearAndAddFile(getNextFile, lastFileId);
+				} else {
+					getAndClearAndAddFile(getNextFile, file.id);
+				}
+			}
+			break;
 	}
 }
diff --git a/app/src/lib/vbranches/fileIdSelection.ts b/app/src/lib/vbranches/fileIdSelection.ts
@@ -12,6 +12,10 @@ export function stringifyFileKey(fileId: string, commitId?: string) {
 	return fileId + '|' + commitId;
 }
 
+export function unstringifyFileKey(fileKeyString: string): string {
+	return fileKeyString.split('|')[0];
+}
+
 export function parseFileKey(fileKeyString: string): FileKey {
 	const [fileId, commitId] = fileKeyString.split('|');
 

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,10 @@ export function stringifyFileKey(fileId: string, commitId?: string) {`
`12`	`12`	`return fileId + '\|' + commitId;`
`13`	`13`	`}`
`14`	`14`
	`15`	`+export function unstringifyFileKey(fileKeyString: string): string {`
	`16`	`+ return fileKeyString.split('\|')[0];`
	`17`	`+}`
	`18`	`+`
`15`	`19`	`export function parseFileKey(fileKeyString: string): FileKey {`
`16`	`20`	`const [fileId, commitId] = fileKeyString.split('\|');`
`17`	`21`