Fix selection not resolving when it should (navigating back to initial

load state)

Author: jpenilla

web/src/lib/components/diff/concise-diff-view.svelte.ts

@@ -19,6 +19,7 @@ import { DEFAULT_THEME_LIGHT } from "$lib/global-options.svelte";
 import type { WritableBoxedValues } from "svelte-toolbelt";
 import type { Attachment } from "svelte/attachments";
 import { on } from "svelte/events";
+import { watch } from "runed";
 
 export interface UnresolvedLineRef {
     /**
@@ -1157,7 +1158,6 @@ export class ConciseDiffViewState<K> {
 
     private selectionAnchor: { hunkIdx: number; lineIdx: number } | null = null;
     private dragSelectionState: { hunk: DiffViewerPatchHunk; didMove: boolean } | null = null;
-    private suppressNextClick = false;
 
     constructor(props: ConciseDiffViewStateProps<K>) {
         this.props = props;
@@ -1166,6 +1166,15 @@ export class ConciseDiffViewState<K> {
             this.update();
         });
 
+        watch([() => this.diffViewerPatch, () => this.props.unresolvedSelection.current], ([patchPromise, unresolvedSelection], [oldPatchPromise]) => {
+            // Update or resolve the selection whenever the patch changes or unresolvedSelection is set
+            if (patchPromise !== oldPatchPromise || unresolvedSelection !== undefined) {
+                patchPromise.then((patch) => {
+                    this.resolveOrUpdateSelection(patch);
+                });
+            }
+        });
+
         $effect(() => {
             const promise = getBaseColors(this.props.syntaxHighlightingTheme.current, this.props.syntaxHighlighting.current);
             promise.then(
@@ -1209,10 +1218,9 @@ export class ConciseDiffViewState<K> {
         );
         this.cachedState = new ConciseDiffViewCachedState(promise, this.props);
         promise.then(
-            (patch) => {
+            () => {
                 // Don't replace a potentially completed promise with a pending one, wait until the replacement is ready for smooth transitions
                 this.diffViewerPatch = promise;
-                this.resolveOrUpdateSelection(patch);
             },
             () => {
                 // Propagate errors
@@ -1334,11 +1342,6 @@ export class ConciseDiffViewState<K> {
         const onDragEnd = (e: PointerEvent) => {
             element.releasePointerCapture(e.pointerId);
             abortController.abort();
-
-            // Suppress the click event only if we actually moved during the drag
-            if (this.dragSelectionState?.didMove) {
-                this.suppressNextClick = true;
-            }
             this.dragSelectionState = null;
         };
 

web/src/lib/diff-viewer.svelte.ts

@@ -395,7 +395,6 @@ export class MultiFileDiffViewerState {
 
             if (page.state.initialLoad ?? false) {
                 if (this.selection) {
-                    // TODO: restoring an unresolved selection does not work until something triggers resolveOrUpdateSelection
                     const hasLines = this.selection.lines || this.selection.unresolvedLines;
                     this.scrollToFile(this.selection.file.index, {
                         focus: !hasLines,
@@ -673,6 +672,8 @@ export class MultiFileDiffViewerState {
             const selectedFile = urlSelection !== undefined ? this.fileDetails.find((f) => f.toFile === urlSelection.file) : undefined;
 
             if (urlSelection && selectedFile && this.diffMetadata.linkable) {
+                // this jump doesn't work as consistently as I'd like, but at least it's consistent between navigation and initial load for a given diff...
+                // seems like the jump to the file header ends up taking priority in some cases, in particular when the file is close to the top of the list?
                 this.jumpToSelection = urlSelection.lines !== undefined;
                 this.selection = {
                     file: selectedFile,