svelte: Improve commit page loading state (#62842)

client/web-sveltekit/src/lib/repo/filePopover/FilePopover.gql

@@ -2,10 +2,10 @@ query FileOrDirPopoverQuery($repoName: String!, $revision: String!, $filePath: S
     repository(name: $repoName) {
         commit(rev: $revision) {
             path(path: $filePath) {
-                ...on GitBlob {
+                ... on GitBlob {
                     ...FilePopoverFragment
                 }
-                ...on GitTree {
+                ... on GitTree {
                     ...DirPopoverFragment
                 }
             }
@@ -49,7 +49,7 @@ fragment DirPopoverFragment on GitTree {
     }
 }
 
-fragment FilePopoverLastCommitFragment  on GitCommit {
+fragment FilePopoverLastCommitFragment on GitCommit {
     abbreviatedOID
     oid
     subject

client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/-/commit/[...revspec]/+page.svelte

@@ -15,7 +15,6 @@
     import CopyButton from '$lib/wildcard/CopyButton.svelte'
 
     import type { PageData, Snapshot } from './$types'
-    import type { CommitPage_DiffConnection } from './page.gql'
 
     interface Capture {
         scroll: ScrollerCapture
@@ -45,14 +44,9 @@
 
     let scroller: Scroller
     let expandedDiffs = new Map<number, boolean>()
-    let diffs: CommitPage_DiffConnection | null = null
 
     $: diffQuery = data.diff
-    // We conditionally check for the ancestors field to be able to show
-    // previously loaded commits when an error occurs while fetching more commits.
-    $: if ($diffQuery?.data?.repository) {
-        diffs = $diffQuery.data.repository.comparison.fileDiffs
-    }
+    $: diffs = $diffQuery?.data?.repository?.comparison.fileDiffs ?? null
 </script>
 
 <svelte:head>
@@ -115,7 +109,7 @@
             {#if $diffQuery?.fetching || $diffQuery?.restoring}
                 <LoadingSpinner />
             {:else if $diffQuery?.error}
-                <div class="m-4">
+                <div class="error">
                     <Alert variant="danger">
                         Unable to fetch file diffs: {$diffQuery.error.message}
                     </Alert>
@@ -157,11 +151,17 @@
         font-size: inherit;
     }
 
+    .error,
     ul.diffs {
-        list-style: none;
         padding: 1rem;
+    }
 
-        li {
+    ul.diffs {
+        // Removes globally set margin
+        margin: 0;
+        list-style: none;
+
+        li:not(:last-child) {
             margin-bottom: 1rem;
         }
     }

client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/-/commit/[...revspec]/+page.ts

@@ -39,7 +39,10 @@ export const load: PageLoad = async ({ params }) => {
                       after: null as string | null,
                   },
                   nextVariables: previousResult => {
-                      if (previousResult?.data?.repository?.comparison?.fileDiffs?.pageInfo?.hasNextPage) {
+                      if (
+                          !previousResult.error &&
+                          previousResult?.data?.repository?.comparison?.fileDiffs?.pageInfo?.hasNextPage
+                      ) {
                           return {
                               after: previousResult.data.repository.comparison.fileDiffs.pageInfo.endCursor,
                           }
@@ -48,7 +51,12 @@ export const load: PageLoad = async ({ params }) => {
                   },
                   combine: (previousResult, nextResult) => {
                       if (!nextResult.data?.repository?.comparison) {
-                          return nextResult
+                          return {
+                              ...nextResult,
+                              // When this code path is executed we probably have an error.
+                              // We still want to show the data that was loaded before the error occurred.
+                              data: previousResult.data,
+                          }
                       }
                       const previousNodes = previousResult.data?.repository?.comparison?.fileDiffs?.nodes ?? []
                       const nextNodes = nextResult.data.repository?.comparison?.fileDiffs?.nodes ?? []

client/web-sveltekit/src/routes/[...repo=reporev]/(validrev)/-/commit/[...revspec]/page.spec.ts

@@ -56,3 +56,37 @@ test('error loading diff information', async ({ page, sg }) => {
     await page.goto(url)
     await expect(page.getByText(/Test error/)).toBeVisible()
 })
+
+test('shows previous diffs when error occurs', async ({ page, sg }) => {
+    let callCount = 0
+    sg.mockOperations({
+        CommitPage_DiffQuery: () => {
+            if (callCount === 1) {
+                throw new Error('Test error')
+            }
+            callCount++
+            return {
+                repository: {
+                    comparison: {
+                        fileDiffs: {
+                            nodes: [
+                                {
+                                    __typename: 'FileDiff',
+                                    oldPath: null,
+                                    newPath: '<new path>',
+                                },
+                            ],
+                            pageInfo: {
+                                hasNextPage: true,
+                                endCursor: 'cursor',
+                            },
+                        },
+                    },
+                },
+            }
+        },
+    })
+    await page.goto(url)
+    await expect(page.getByText('<new path>')).toBeVisible()
+    await expect(page.getByText('Test error')).toBeVisible()
+})