Merge pull request #7366 from gitbutlerapp/e-branch-69

Correctly clear the code selection

apps/web/src/lib/components/review/DiffSection.svelte

@@ -9,13 +9,8 @@
 		section: DiffSection;
 		selectedSha: string | undefined;
 		selectedLines: LineSelector[];
-		clearLineSelection: () => void;
-		toggleDiffLine: (
-			fileName: string,
-			hunkIndex: number,
-			diffSha: string,
-			params: LineClickParams
-		) => void;
+		clearLineSelection: (fileName: string) => void;
+		toggleDiffLine: (fileName: string, diffSha: string, params: LineClickParams) => void;
 		onCopySelection: (contentSections: ContentSection[]) => void;
 		onQuoteSelection: () => void;
 	}
@@ -32,8 +27,8 @@
 	const hunks = $derived(section.diffPatch ? splitDiffIntoHunks(section.diffPatch) : []);
 	const filePath = $derived(section.newPath || 'unknown');
 
-	function handleLineClick(index: number, params: LineClickParams) {
-		toggleDiffLine(section.newPath || 'unknown', index, section.diffSha, params);
+	function handleLineClick(params: LineClickParams) {
+		toggleDiffLine(section.newPath || 'unknown', section.diffSha, params);
 	}
 
 	const selectedLines = $derived(selectedSha === section.diffSha ? lines : []);
@@ -44,13 +39,13 @@
 		<FileIcon fileName={filePath} size={16} />
 		<p title={filePath} class="text-12 text-body file-name">{filePath}</p>
 	</div>
-	{#each hunks as hunkStr, idx}
+	{#each hunks as hunkStr}
 		<HunkDiff
 			filePath={section.newPath || 'unknown'}
 			{hunkStr}
 			diffLigatures={false}
 			{selectedLines}
-			onLineClick={(p) => handleLineClick(idx, p)}
+			onLineClick={handleLineClick}
 			{onCopySelection}
 			{onQuoteSelection}
 			{clearLineSelection}

apps/web/src/lib/components/review/ReviewSections.svelte

@@ -9,13 +9,8 @@
 		patchSections: Section[] | undefined;
 		selectedSha: string | undefined;
 		selectedLines: LineSelector[];
-		clearLineSelection: () => void;
-		toggleDiffLine: (
-			fileName: string,
-			hunkIndex: number,
-			diffSha: string,
-			params: LineClickParams
-		) => void;
+		clearLineSelection: (fileName: string) => void;
+		toggleDiffLine: (fileName: string, diffSha: string, params: LineClickParams) => void;
 		onCopySelection: (contentSections: ContentSection[]) => void;
 		onQuoteSelection: () => void;
 	}

apps/web/src/lib/components/review/Section.svelte

@@ -8,13 +8,8 @@
 		section: Section;
 		selectedSha: string | undefined;
 		selectedLines: LineSelector[];
-		clearLineSelection: () => void;
-		toggleDiffLine: (
-			fileName: string,
-			hunkIndex: number,
-			diffSha: string,
-			params: LineClickParams
-		) => void;
+		clearLineSelection: (fileName: string) => void;
+		toggleDiffLine: (fileName: string, diffSha: string, params: LineClickParams) => void;
 		onCopySelection: (contentSections: ContentSection[]) => void;
 		onQuoteSelection: () => void;
 	}

apps/web/src/lib/diff/lineSelection.svelte.ts

@@ -2,7 +2,7 @@ import { copyToClipboard } from '@gitbutler/shared/clipboard';
 import {
 	readDiffLineKey,
 	type DiffLineKey,
-	type DiffFileHunkKey,
+	type DiffFileKey,
 	createDiffFileHunkKey,
 	createDiffLineKey,
 	readDiffFileHunkKey,
@@ -20,7 +20,6 @@ export interface DiffLineSelected extends LineSelector {
 export interface DiffSelection {
 	diffSha: string;
 	fileName: string;
-	hunkIndex: number;
 	lines: DiffLineSelected[];
 }
 
@@ -77,27 +76,33 @@ function calculateSelectedLines(selectedDiffLines: SvelteSet<DiffLineKey>): Diff
 
 export default class DiffLineSelection {
 	private _quote = $state<boolean>(false);
-	private _diffSha = $state<string>();
 	private _selectedDiffLines = new SvelteSet<DiffLineKey>();
 	private _selectedLines: DiffLineSelected[] = $derived(
 		calculateSelectedLines(this._selectedDiffLines)
 	);
-	private _selectedDiffFileHunk = $state<DiffFileHunkKey>();
+	private _selectedDiffFile = $state<DiffFileKey>();
+
+	clear(fileName?: string) {
+		if (fileName && this._selectedDiffFile) {
+			const parsed = readDiffFileHunkKey(this._selectedDiffFile);
+			if (!parsed) return; // This should never happen
+
+			const [selectedFileName, _] = parsed;
+
+			if (selectedFileName !== fileName) return;
+		}
 
-	clear() {
 		this._selectedDiffLines.clear();
-		this._selectedDiffFileHunk = undefined;
-		this._diffSha = undefined;
+		this._selectedDiffFile = undefined;
 		this._quote = false;
 	}
 
-	toggle(fileName: string, hunkIndex: number, diffSha: string, params: LineClickParams) {
-		this._diffSha = diffSha;
-		const diffFileHunkKey = createDiffFileHunkKey(fileName, hunkIndex);
+	toggle(fileName: string, diffSha: string, params: LineClickParams) {
+		const diffFileHunkKey = createDiffFileHunkKey(fileName, diffSha);
 
-		if (this._selectedDiffFileHunk !== diffFileHunkKey) {
+		if (this._selectedDiffFile !== diffFileHunkKey) {
 			this._selectedDiffLines.clear();
-			this._selectedDiffFileHunk = diffFileHunkKey;
+			this._selectedDiffFile = diffFileHunkKey;
 		}
 
 		const key = createDiffLineKey(params.index, params.oldLine, params.newLine);
@@ -150,26 +155,25 @@ export default class DiffLineSelection {
 	}
 
 	get selectedSha() {
-		return this._diffSha;
+		if (!this._selectedDiffFile) return;
+
+		const parsed = readDiffFileHunkKey(this._selectedDiffFile);
+		if (!parsed) return;
+
+		const [_, sha] = parsed;
+		return sha;
 	}
 
 	get diffSelection(): DiffSelection | undefined {
-		if (
-			!this._quote ||
-			!this._selectedDiffFileHunk ||
-			this._selectedLines.length === 0 ||
-			this._diffSha === undefined
-		)
-			return;
-
-		const parsed = readDiffFileHunkKey(this._selectedDiffFileHunk);
+		if (!this._quote || !this._selectedDiffFile || this._selectedLines.length === 0) return;
+
+		const parsed = readDiffFileHunkKey(this._selectedDiffFile);
 		if (!parsed) return;
-		const [fileName, hunkIndex] = parsed;
+		const [fileName, diffSha] = parsed;
 
 		return {
-			diffSha: this._diffSha,
+			diffSha,
 			fileName,
-			hunkIndex,
 			lines: this._selectedLines
 		};
 	}

apps/web/src/routes/[ownerSlug]/[projectSlug]/reviews/[branchId]/commit/[changeId]/+page.svelte

@@ -136,12 +136,12 @@
 				<ReviewSections
 					{patch}
 					patchSections={patchSections?.current}
-					toggleDiffLine={(f, h, s, p) => diffLineSelection.toggle(f, h, s, p)}
+					toggleDiffLine={(f, s, p) => diffLineSelection.toggle(f, s, p)}
 					selectedSha={diffLineSelection.selectedSha}
 					selectedLines={diffLineSelection.selectedLines}
 					onCopySelection={(sections) => diffLineSelection.copy(sections)}
 					onQuoteSelection={() => diffLineSelection.quote()}
-					clearLineSelection={() => diffLineSelection.clear()}
+					clearLineSelection={(fileName) => diffLineSelection.clear(fileName)}
 				/>
 			</div>
 

packages/ui/src/lib/HunkDiff.svelte

@@ -25,7 +25,7 @@
 		onchange?: (selected: boolean) => void;
 		selectedLines?: LineSelector[];
 		onLineClick?: (params: LineSelectionParams) => void;
-		clearLineSelection?: () => void;
+		clearLineSelection?: (fileName: string) => void;
 		onQuoteSelection?: () => void;
 		onCopySelection?: (contentSections: ContentSection[]) => void;
 	}
@@ -105,7 +105,7 @@
 			{filePath}
 			content={hunk.contentSections}
 			{onLineClick}
-			{clearLineSelection}
+			clearLineSelection={() => clearLineSelection?.(filePath)}
 			{wrapText}
 			{tabSize}
 			{inlineUnifiedDiffs}

packages/ui/src/lib/hunkDiff/HunkDiffBody.svelte

@@ -59,6 +59,8 @@
 	);
 
 	$effect(() => lineSelection.setRows(renderRows));
+
+	const hasSelectedLines = $derived(renderRows.filter((row) => row.isSelected).length > 0);
 </script>
 
 {#snippet countColumn(row: Row, side: CountColumnSide, idx: number)}
@@ -79,16 +81,17 @@
 	</td>
 {/snippet}
 
-<tbody class="contrast-{diffContrast}" style="--diff-font: {diffFont};">
+<tbody
+	class="contrast-{diffContrast}"
+	style="--diff-font: {diffFont};"
+	use:clickOutside={{
+		handler: () => {
+			if (hasSelectedLines) clearLineSelection?.();
+		}
+	}}
+>
 	{#each renderRows as row, idx}
-		<tr
-			id={getHunkLineId(row.encodedLineId)}
-			class="table__row"
-			data-no-drag
-			use:clickOutside={{
-				handler: () => row.isSelected && clearLineSelection?.()
-			}}
-		>
+		<tr id={getHunkLineId(row.encodedLineId)} class="table__row" data-no-drag>
 			{@render countColumn(row, CountColumnSide.Before, idx)}
 			{@render countColumn(row, CountColumnSide.After, idx)}
 			<td
@@ -100,6 +103,9 @@
 				class:diff-line-addition={row.type === SectionType.AddedLines}
 				class:selected={row.isSelected}
 				class:is-last={row.isLast}
+				onclick={() => {
+					if (!row.isSelected && hasSelectedLines) clearLineSelection?.();
+				}}
 			>
 				{#if row.isSelected}
 					<div

packages/ui/src/lib/utils/clickOutside.ts

@@ -1,4 +1,4 @@
-export type ClickOpts = { excludeElement?: Element; handler: () => void };
+export type ClickOpts = { excludeElement?: Element; handler: (event: MouseEvent) => void };
 
 export function clickOutside(node: HTMLElement, params: ClickOpts) {
 	function onClick(event: MouseEvent) {
@@ -7,7 +7,7 @@ export function clickOutside(node: HTMLElement, params: ClickOpts) {
 			!node.contains(event.target as HTMLElement) &&
 			!params.excludeElement?.contains(event.target as HTMLElement)
 		) {
-			params.handler();
+			params.handler(event);
 		}
 	}
 	document.addEventListener('pointerdown', onClick, true);

packages/ui/src/lib/utils/diffParsing.ts

@@ -319,7 +319,7 @@ class CodeHighlighter {
 }
 
 export type DiffLineKey = BrandedId<'DiffLine'>;
-export type DiffFileHunkKey = BrandedId<'DiffFileHunk'>;
+export type DiffFileKey = BrandedId<'DiffFile'>;
 export type DiffLineRange = BrandedId<'DiffLineRange'>;
 export type DiffFileLineId = BrandedId<'DiffFileLineId'>;
 
@@ -351,18 +351,18 @@ export function readDiffLineKey(key: DiffLineKey): ParsedDiffLineKey | undefined
 	};
 }
 
-export function createDiffFileHunkKey(fileName: string, hunkIndex: number): DiffFileHunkKey {
-	return `${fileName}-${hunkIndex}` as DiffFileHunkKey;
+export function createDiffFileHunkKey(fileName: string, diffSha: string): DiffFileKey {
+	return `${fileName}-${diffSha}` as DiffFileKey;
 }
 
-export function readDiffFileHunkKey(key: DiffFileHunkKey): [string, number] | undefined {
-	const [fileName, hunkIndex] = key.split('-');
+export function readDiffFileHunkKey(key: DiffFileKey): [string, string] | undefined {
+	const [fileName, diffSha] = key.split('-');
 
-	if (fileName === undefined || hunkIndex === undefined) {
+	if (fileName === undefined || diffSha === undefined) {
 		return undefined;
 	}
 
-	return [fileName, parseInt(hunkIndex)];
+	return [fileName, diffSha];
 }
 
 export function encodeSingleDiffLine(