Svelte: unrevert RepoPopover (#62744)

This re-applies #61989 after it was reverted. In addition to reapplying the change:

- It reverts the changes to Popover.svelte that removed the border.
- We only start loading data on hover, not on mount
- Various fixes in text overflow conditions
- Removes the language from the popover data because it can be very expensive to calculate 
   (another reason to pre-calculate language, but that's for another day)
- Moves the data loading out of the page loader. Exports the data loading function from 
   the component so data loading is still orchestrated by the caller. (I know this will be controversial, reasoning inline)
- Adds a delay to the popover so it doesn't get in the way as your mouse moves over the page.
- Uses the display name instead of the author name
- Linkifies the commit message

client/web-sveltekit/src/lib/Popover.svelte

@@ -1,8 +1,8 @@
 <script lang="ts">
     import type { Placement } from '@floating-ui/dom'
+    import type { Action } from 'svelte/action'
 
     import { popover, onClickOutside, portal } from './dom'
-    import type { Action } from 'svelte/action'
 
     export let placement: Placement = 'bottom'
     /**
@@ -105,9 +105,19 @@
         font-size: 0.875rem;
         background-clip: padding-box;
         background-color: var(--dropdown-bg);
-        border: 1px solid var(--dropdown-border-color);
-        border-radius: var(--popover-border-radius);
         color: var(--body-color);
         box-shadow: var(--popover-shadow);
+
+        border: 1px solid var(--dropdown-border-color);
+        border-radius: var(--popover-border-radius);
+        // Ensure child elements do not overflow the border radius
+        overflow: hidden;
+
+        // We always display the popover on hover, but there may not be anything
+        // inside until something we load something. This ensures we do not
+        // render an empty border if there is nothing to show.
+        &:empty {
+            display: none;
+        }
     }
 </style>

client/web-sveltekit/src/lib/repo/RepoPopover/RepoPopover.gql

@@ -1,20 +1,21 @@
-fragment RepoPopoverFields on Repository {
+query RepoPopoverQuery($repoName: String!) {
+    repository(name: $repoName) {
+        ...RepoPopoverFragment
+    }
+}
+
+fragment RepoPopoverFragment on Repository {
     name
     description
     stars
     isPrivate
-    language
+    topics
     externalServices {
         totalCount
         nodes {
             kind
         }
     }
-    tags {
-        nodes {
-            name
-        }
-    }
 
     commit(rev: "HEAD") {
         id

client/web-sveltekit/src/lib/repo/RepoPopover/RepoPopover.stories.svelte

@@ -2,7 +2,7 @@
     import { faker } from '@faker-js/faker'
     import { Story } from '@storybook/addon-svelte-csf'
 
-    import { type RepoPopoverFields, ExternalServiceKind } from '$testing/graphql-type-mocks'
+    import { type RepoPopoverFragment, ExternalServiceKind } from '$testing/graphql-type-mocks'
 
     import RepoPopover from './RepoPopover.svelte'
 
@@ -13,18 +13,18 @@
 
 <script lang="ts">
     faker.seed(1)
-    let repo: RepoPopoverFields = {
+    let repo: RepoPopoverFragment = {
         name: `${faker.lorem.word()} / ${faker.lorem.word()}`,
         description: faker.lorem.sentence(),
         stars: faker.datatype.number(),
-        tags: {
-            nodes: [
-                { name: faker.lorem.word() },
-                { name: faker.lorem.word() },
-                { name: faker.lorem.word() },
-                { name: faker.lorem.word() },
-            ],
-        },
+        topics: [
+            faker.lorem.word(),
+            faker.lorem.word(),
+            faker.lorem.word(),
+            faker.lorem.word(),
+            faker.lorem.word(),
+            faker.lorem.word(),
+        ],
         isPrivate: false,
         language: 'Go',
         externalServices: {
@@ -40,9 +40,11 @@
             subject: faker.lorem.sentence(),
             canonicalURL: faker.internet.url(),
             oid: '7b4d3ad230d9078a70219f2befa1be1fe00377a0',
+            abbreviatedOID: '7b4d3ad',
             author: {
                 date: new Date().toISOString(),
                 person: {
+                    __typename: 'Person',
                     displayName: `${faker.person.firstName()} ${faker.person.lastName()}`,
                     avatarURL: faker.internet.avatar(),
                     name: faker.internet.userName(),
@@ -54,9 +56,9 @@
 
 <Story name="Default">
     <h2>With header</h2>
-    <RepoPopover {repo} withHeader />
+    <RepoPopover data={repo} withHeader />
     <br />
     <br />
     <h2>Without header</h2>
-    <RepoPopover {repo} />
+    <RepoPopover data={repo} />
 </Story>

client/web-sveltekit/src/lib/repo/RepoPopover/RepoPopover.svelte

@@ -1,149 +1,124 @@
-<!--
-This Component should be instantiated inside of a Popover component.
+<script lang="ts" context="module">
+    import type { GraphQLClient } from '$lib/graphql'
 
-For example:
+    import { RepoPopoverQuery } from './RepoPopover.gql'
+
+    export async function fetchRepoPopoverData(client: GraphQLClient, repoName: string): Promise<RepoPopoverFragment> {
+        const response = await client.query(RepoPopoverQuery, { repoName })
+        if (!response.data?.repository || response.error) {
+            throw new Error(`Failed to fetch repo info: ${response.error}`)
+        }
+        return response.data.repository
+    }
+</script>
 
-<Popover ...>
-    [trigger button ...]
-    <div slot="content">
-        <RepoPopover ... />
-    </div>
-</Popover>
--->
 <script lang="ts">
+    import { mdiSourceMerge } from '@mdi/js'
     import { capitalize } from 'lodash'
 
     import Avatar from '$lib/Avatar.svelte'
     import Icon from '$lib/Icon.svelte'
+    import { displayRepoName } from '$lib/shared'
     import Timestamp from '$lib/Timestamp.svelte'
+    import Badge from '$lib/wildcard/Badge.svelte'
 
     import RepoStars from '../RepoStars.svelte'
     import { getIconPathForCodeHost } from '../shared/codehost'
 
-    import { RepoPopoverFields } from './RepoPopover.gql'
+    import type { RepoPopoverFragment } from './RepoPopover.gql'
 
-    export let repo: RepoPopoverFields
+    export let data: RepoPopoverFragment
     export let withHeader = false
 
-    const CENTER_DOT = '\u00B7' // interpunct
-
-    function truncateCommitNumber(numStr: string, length: number) {
-        return numStr.substring(numStr.length - length)
-    }
-
-    $: subject = repo.commit?.subject
-    $: url = repo.commit?.canonicalURL
-    $: commitSHA = repo.commit?.oid
-    $: author = repo.commit?.author.person.name
-    $: commitDate = repo.commit?.author.date
-    $: avatar = repo.commit?.author.person
-    $: codeHostKind = repo.externalServices.nodes[0].kind
-    $: codeHostIcon = getIconPathForCodeHost(codeHostKind)
+    $: commit = data.commit
+    $: author = commit?.author
+    $: codeHostKind = data.externalServices.nodes[0].kind
 </script>
 
 <div class="root">
     {#if withHeader}
         <div class="header">
-            <div class="icon-name-access">
-                <!-- @TODO: We need to use our customer's logo here, not the code host's -->
-                <!--Icon svgPath={mdiGitlab} /-->
-                <h4 class="repo-name">{repo.name}</h4>
-                <div class="access">
-                    <small>{repo.isPrivate ? 'Private' : 'Public'}</small>
-                </div>
+            <div class="left">
+                <Icon svgPath={mdiSourceMerge} --icon-fill-color="var(--primary)" />
+                <h4>{displayRepoName(data.name)}</h4>
+                <Badge variant="outlineSecondary" small pill>
+                    {data.isPrivate ? 'Private' : 'Public'}
+                </Badge>
             </div>
-            <div class="code-host">
-                <Icon svgPath={codeHostIcon} --color="var(--text-body)" --icon-size="24px" />
-                <div><small>{capitalize(codeHostKind)}</small></div>
+            <div class="right">
+                <Icon svgPath={getIconPathForCodeHost(codeHostKind)} --icon-fill-color="var(--text-body)" --size={24} />
+                <small>{capitalize(codeHostKind)}</small>
             </div>
         </div>
-        <div class="divider" />
     {/if}
 
-    <div class="description-and-tags">
-        <div class="description">{repo.description}</div>
-        <div class="tags">
-            {#if repo.tags.nodes.length > 0}
-                {#each repo.tags.nodes as tag}
-                    <div class="tag"><small>{tag.name}</small></div>
-                {/each}
+    {#if data.description || data.topics.length}
+        <div class="description-and-tags">
+            <div class="description">
+                {data.description}
+            </div>
+            {#if data.topics.length}
+                <div class="tags">
+                    {#each data.topics as topic}
+                        <Badge variant="link" small pill>{topic}</Badge>
+                    {/each}
+                </div>
             {/if}
         </div>
-    </div>
+    {/if}
 
-    <div class="divider" />
-
-    <div class="last-commit">
-        <div class="heading">
+    {#if commit}
+        <div class="last-commit">
             <small>Last Commit</small>
-        </div>
 
-        <div class="commit-info">
-            <div class="commit">
-                <!--
-                A <div> element is needed for subject and commit message
-                because the <small> element alone doesn't work with
-                text-overflow: ellipsis.
-                -->
-                <div class="subject">
-                    <small>{subject}</small>
-                </div>
-                {#if commitSHA}
-                    <div class="commit-number">
-                        <small class="commit-number"
-                            ><a href={url} target="_blank">#{truncateCommitNumber(commitSHA, 6)}</a></small
-                        >
+            <div class="commit-info">
+                <small class="subject"><a href={commit.canonicalURL}>{commit.subject}</a></small>
+                {#if author?.person}
+                    <div class="author">
+                        <Avatar avatar={author.person} --avatar-size="1.0rem" />
+                        <small>{author.person.displayName} · <Timestamp date={author?.date} /></small>
                     </div>
                 {/if}
             </div>
-            <div class="author">
-                {#if avatar}
-                    <Avatar {avatar} --avatar-size="1.0rem" />
-                {/if}
-                <small>{author}</small>
-                <small>{CENTER_DOT}</small>
-                {#if commitDate}
-                    <small><Timestamp date={commitDate} /></small>
-                {/if}
-            </div>
         </div>
-    </div>
-
-    <div class="divider" />
+    {/if}
 
     <div class="footer">
-        <small>{repo.language}</small>
-        <RepoStars repoStars={repo.stars} small={true} />
+        <RepoStars repoStars={data.stars} small />
     </div>
 </div>
 
 <style lang="scss">
     .root {
-        border: 1px solid var(--border-color);
-        border-radius: var(--popover-border-radius);
-        width: 400px;
+        width: 480px;
+
+        & > div {
+            padding: 0.75rem;
+
+            &:not(:last-child) {
+                border-bottom: 1px solid var(--border-color);
+            }
+        }
     }
 
     .header {
         display: flex;
-        flex-flow: row nowrap;
         justify-content: space-between;
         align-items: center;
-        padding: 0.5rem 0.75rem;
         background-color: var(--subtle-bg);
 
-        .icon-name-access {
+        .left {
             display: flex;
-            flex-flow: row nowrap;
-            justify-content: space-between;
+            justify-content: flex-start;
             align-items: center;
+            gap: 0.25rem;
 
-            .repo-name {
+            h4 {
                 color: var(--text-body);
-                margin: 0rem 0.5rem 0rem 0rem;
+                margin: 0;
             }
 
-            .access {
+            small {
                 border: 1px solid var(--text-muted);
                 color: var(--text-muted);
                 padding: 0rem 0.5rem;
@@ -151,34 +126,26 @@ For example:
             }
         }
 
-        .code-host {
+        .right {
             display: flex;
-            flex-flow: row nowrap;
             justify-content: flex-end;
             align-items: center;
+            gap: 0.25rem;
 
-            div {
+            small {
                 color: var(--text-muted);
-                margin-left: 0.25rem;
             }
         }
     }
 
-    .divider {
-        border-bottom: 1px solid var(--border-color);
-        width: 100%;
-    }
-
     .description-and-tags {
-        padding: 0.75rem;
+        display: flex;
+        flex-flow: column nowrap;
+        justify-content: center;
+        align-items: flex-start;
+        gap: 0.5rem 0.5rem;
         width: 100%;
-
-        .description {
-            padding: 0rem;
-            text-overflow: ellipsis;
-            overflow: hidden;
-            white-space: nowrap;
-        }
+        color: var(--text-body);
 
         .tags {
             align-content: space-around;
@@ -187,72 +154,51 @@ For example:
             flex-flow: row wrap;
             gap: 0.5rem 0.5rem;
             justify-content: flex-start;
-            margin-top: 0.5rem;
-
-            .tag {
-                background-color: var(--subtle-bg);
-                border-radius: 1rem;
-                color: var(--primary);
-                font-family: var(--monospace-font-family);
-                padding: 0rem 0.5rem;
-            }
         }
     }
 
     .last-commit {
         display: flex;
-        flex-flow: row nowrap;
         justify-content: space-between;
-        align-items: flex-start;
-        padding: 0.75rem;
+        gap: 2rem;
+        font-size: var(--font-size-small);
+        color: var(--text-muted);
 
-        .heading {
-            color: var(--text-muted);
+        small {
+            flex-shrink: 0;
         }
 
         .commit-info {
             display: flex;
             flex-flow: column nowrap;
-            justify-content: center;
+            text-align: end;
             align-items: flex-end;
-            gap: 0.25rem 0rem;
+            gap: 0.25rem;
+            min-width: 0;
 
-            .commit {
-                display: flex;
-                flex-flow: row nowrap;
-                justify-content: flex-end;
-                align-items: center;
-                gap: 0.25rem 0rem;
-                width: 250px;
-
-                .subject {
-                    text-overflow: ellipsis;
-                    overflow: hidden;
-                    white-space: nowrap;
+            .subject {
+                text-overflow: ellipsis;
+                overflow: hidden;
+                white-space: nowrap;
+                width: 100%;
+                a {
                     color: var(--text-body);
                 }
-
-                .commit-number {
-                    color: var(--text-muted);
-                    align-self: center;
-                }
             }
 
             .author {
                 display: flex;
-                flex-flow: row nowrap;
                 color: var(--text-muted);
-                gap: 0.5rem 0.25rem;
+                gap: 0.5rem;
+                align-items: center;
             }
         }
     }
 
     .footer {
-        color: var(--text-muted);
         display: flex;
-        flex-flow: row nowrap;
-        justify-content: space-between;
+        color: var(--text-muted);
         align-items: center;
-        padding: 0.75rem;
+        justify-content: flex-end;
     }
 </style>

client/web-sveltekit/src/lib/utils/index.ts

@@ -2,3 +2,4 @@ export * from './assert'
 export * from './formatting'
 export * from './promises'
 export * from './stores'
+export * from './promises'

client/web-sveltekit/src/lib/utils/promises.ts

@@ -97,3 +97,11 @@ export function toReadable<D, E = Error>(promise: PromiseLike<D>): Readable<Load
     )
     return { subscribe }
 }
+
+// Returns a promise that is guaranteed to take at least `delayMillis` milliseconds to resolve.
+// If the wrapped promise resolves before then, the returned promise will wait until `delayMillis`
+// has elapsed before resolving.
+export async function delay<T>(promise: Promise<T>, delayMillis: number): Promise<T> {
+    const [awaited] = await Promise.all([promise, new Promise(resolve => setTimeout(resolve, delayMillis))])
+    return awaited
+}

client/web-sveltekit/src/routes/+layout.ts

@@ -6,7 +6,7 @@ import type { Settings } from '$lib/shared'
 
 import type { LayoutLoad } from './$types'
 import { Init, EvaluatedFeatureFlagsQuery, GlobalAlertsSiteFlags, DisableSveltePrototype } from './layout.gql'
-import { mainNavigation, dotcomMainNavigation } from './navigation'
+import { dotcomMainNavigation, mainNavigation } from './navigation'
 
 // Disable server side rendering for the whole app
 export const ssr = false

client/web-sveltekit/src/routes/[...repo=reporev]/+layout.svelte

@@ -9,15 +9,15 @@
         mdiTag,
         mdiDotsHorizontal,
     } from '@mdi/js'
-
     import { writable } from 'svelte/store'
-    import { page } from '$app/stores'
+
     import { getButtonClassName } from '@sourcegraph/wildcard'
 
+    import { page } from '$app/stores'
     import { computeFit } from '$lib/dom'
-    import { DropdownMenu, MenuLink } from '$lib/wildcard'
     import Icon from '$lib/Icon.svelte'
     import GlobalHeaderPortal from '$lib/navigation/GlobalHeaderPortal.svelte'
+    import { DropdownMenu, MenuLink } from '$lib/wildcard'
 
     import type { LayoutData } from './$types'
     import RepoSearchInput from './RepoSearchInput.svelte'

client/web-sveltekit/src/routes/search/RepoRev.svelte

@@ -1,12 +1,18 @@
 <script lang="ts">
     import { highlightRanges } from '$lib/dom'
+    import { getGraphQLClient } from '$lib/graphql'
+    import Popover from '$lib/Popover.svelte'
+    import { default as RepoPopover, fetchRepoPopoverData } from '$lib/repo/RepoPopover/RepoPopover.svelte'
     import CodeHostIcon from '$lib/search/CodeHostIcon.svelte'
     import { displayRepoName } from '$lib/shared'
+    import { delay } from '$lib/utils'
 
     export let repoName: string
     export let rev: string | undefined
     export let highlights: [number, number][] = []
 
+    const client = getGraphQLClient()
+
     $: href = `/${repoName}${rev ? `@${rev}` : ''}`
     $: displayName = displayRepoName(repoName)
     $: if (displayName !== repoName) {
@@ -21,12 +27,19 @@
     <CodeHostIcon repository={repoName} />
     <!-- #key is needed here to recreate the link because use:highlightRanges changes the DOM -->
     {#key highlights}
-        <a class="repo-link" {href} use:highlightRanges={{ ranges: highlights }}>
-            {displayRepoName(repoName)}
-            {#if rev}
-                <small class="rev"> @ {rev}</small>
-            {/if}
-        </a>
+        <Popover showOnHover let:registerTrigger placement="bottom-start">
+            <a class="repo-link" {href} use:highlightRanges={{ ranges: highlights }} use:registerTrigger>
+                {displayRepoName(repoName)}
+                {#if rev}
+                    <small class="rev"> @ {rev}</small>
+                {/if}
+            </a>
+            <svelte:fragment slot="content">
+                {#await delay(fetchRepoPopoverData(client, repoName), 200) then data}
+                    <RepoPopover {data} withHeader />
+                {/await}
+            </svelte:fragment>
+        </Popover>
     {/key}
 </span>
 

client/web-sveltekit/src/routes/search/SearchResults.svelte

@@ -174,6 +174,13 @@
                             <SearchAlert alert={$stream.alert} />
                         </div>
                     {/if}
+                    <!--
+                        TODO: Address accessibility issues
+                        1. A11y: visible, non-interactive elements with an on:click event
+                           must be accompanied by an on:keydown, on:keyup, or on:keypress event.
+                        2. A11y: Non-interactive element <ol> should not be assigned mouse
+                           or keyboard event listeners.
+                    -->
                     <ol on:click={handleSearchResultClick} on:copy={handleResultCopy}>
                         {#each resultsToShow as result, i}
                             {@const component = getSearchResultComponent(result)}