chore(svelte): Properly route to revision agnostic pages (#63444)

There are two kinds of repository pages: Those who depend on the
revision possibly present in the URL and those who are not.

This commit re-arranges the routes to better distinguish between these
two types. It also adds a visual indication to the commits that the
history at a specific revision is shown. Additionally it removes the
revision from links to revision agnostic pages.

Overall this should make it clearer to the user when and when not the
revision in the URL is relevant.

 Note: I originally added a redirect for revision agnostic pages that
 would remove the revision if it was present in the URL. I decided to
 not do that for now because that redirect introduces an additional
 history entry which makes it difficult to navigate back.

Closes srch-600


## Test plan

Manual testing.
This commit is contained in:
Felix Kling 2024-06-26 14:46:56 +02:00 committed by GitHub
parent bc49218e6c
commit 70ba49e46a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
28 changed files with 91 additions and 66 deletions

View File

@ -412,7 +412,8 @@
}
}
.message, .error {
.message,
.error {
padding: 1rem;
}

View File

@ -26,33 +26,33 @@ export const svelteKitRoutes: SvelteKitRoute[] = [
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/tree(?:/.*)?/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/branches',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/branches/all',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/all/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/commit/[...revspec]',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/commit(?:/.*)?/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/commits/[...path]',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/commits(?:/.*)?/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/stats/contributors',
id: '/[...repo=reporev]/-/branches',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/-/branches/all',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/all/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/-/commit/[...revspec]',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/commit(?:/.*)?/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/-/stats/contributors',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/stats/contributors/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/tags',
id: '/[...repo=reporev]/-/tags',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/tags/?$'),
isRepoRoot: false,
},

View File

@ -63,9 +63,15 @@
<title>{pageTitle}</title>
</svelte:head>
{#if data.path}
<h2>Commits in <code>{data.path}</code></h2>
{/if}
<h2>
Commits
{#if data.path}
in <code>{data.path}</code>
{/if}
at <Badge variant="link">
<a href={data.repoURL}>{data.displayRevision || data.resolvedRevision.defaultBranch}</a></Badge
>
</h2>
<section>
<Scroller bind:this={scroller} margin={600} on:more={fetchMore}>
{#if !$commitsQuery.restoring && commits}
@ -128,6 +134,13 @@
padding: 0;
}
h2 {
display: flex;
align-items: center;
gap: 0.5rem;
border-bottom: 1px solid var(--border-color);
}
h2,
ul.commits,
.footer {

View File

@ -40,14 +40,24 @@
* Who can see this entry.
*/
visibility: 'admin' | 'user'
/**
* Whether or not to preserve the revision in the URL.
*/
preserveRevision?: boolean
}
export let data: LayoutData
const menuOpen = writable(false)
const navEntries: MenuEntry[] = [
{ path: '', icon: ILucideCode, label: 'Code', visibility: 'user' },
{ path: '/-/commits', icon: ILucideGitCommitVertical, label: 'Commits', visibility: 'user' },
{ path: '', icon: ILucideCode, label: 'Code', visibility: 'user', preserveRevision: true },
{
path: '/-/commits',
icon: ILucideGitCommitVertical,
label: 'Commits',
visibility: 'user',
preserveRevision: true,
},
{ path: '/-/branches', icon: ILucideGitBranch, label: 'Branches', visibility: 'user' },
{ path: '/-/tags', icon: ILucideTag, label: 'Tags', visibility: 'user' },
{ path: '/-/stats/contributors', icon: ILucideUsers, label: 'Contributors', visibility: 'user' },
@ -55,7 +65,6 @@
const menuEntries: MenuEntry[] = [
{ path: '/-/compare', icon: ILucideGitCompare, label: 'Compare', visibility: 'user' },
{ path: '/-/own', icon: ILucideUsers, label: 'Ownership', visibility: 'admin' },
{ path: '/-/embeddings', icon: ILucideSpline, label: 'Embeddings', visibility: 'admin' },
{ path: '/-/code-graph', icon: ILucideCodesandbox, label: 'Code graph data', visibility: 'admin' },
{ path: '/-/batch-changes', icon: ISgBatchChanges, label: 'Batch changes', visibility: 'admin' },
{ path: '/-/settings', icon: ILucideSettings, label: 'Settings', visibility: 'admin' },
@ -96,7 +105,7 @@
id: entry.label,
title: entry.label,
icon: entry.icon,
href: data.repoURL + entry.path,
href: (entry.preserveRevision ? data.repoURL : data.repoURLWithoutRevision) + entry.path,
}))
$: selectedTab = tabs.findIndex(tab => isActive(tab.href, $page.url))
@ -160,7 +169,7 @@
</svelte:fragment>
{#each allMenuEntries as entry}
{#if entry.visibility === 'user' || (entry.visibility === 'admin' && data.user?.siteAdmin)}
{@const href = data.repoURL + entry.path}
{@const href = (entry.preserveRevision ? data.repoURL : data.repoURLWithoutRevision) + entry.path}
<MenuLink {href}>
<div class="overflow-entry">
{#if entry.icon}

View File

@ -61,6 +61,7 @@ export const load: LayoutLoad = async ({ params, url, depends }) => {
return {
repoURL: '/' + params.repo,
repoURLWithoutRevision: '/' + repoName,
repoName,
displayRepoName: displayRepoName(repoName),
/**

View File

@ -15,6 +15,7 @@ export const load: PageLoad = ({ params }) => {
repoName,
withBehindAhead: true,
})
.toPromise()
.then(
mapOrThrow(result => {
if (!result.data?.repository) {

View File

@ -1,4 +1,4 @@
import { expect, test } from '../../../../../../testing/integration'
import { expect, test } from '../../../../../testing/integration'
const repoName = 'github.com/sourcegraph/sourcegraph'
const url = `/${repoName}/-/branches/all`

View File

@ -1,4 +1,4 @@
import { expect, test } from '../../../../../testing/integration'
import { expect, test } from '../../../../testing/integration'
const repoName = 'github.com/sourcegraph/sourcegraph'
const url = `/${repoName}/-/branches`

View File

@ -15,7 +15,7 @@
import Badge from '$lib/wildcard/Badge.svelte'
import CopyButton from '$lib/wildcard/CopyButton.svelte'
import { getRepositoryPageContext } from '../../../../context'
import { getRepositoryPageContext } from '../../../context'
import type { PageData, Snapshot } from './$types'

View File

@ -1,4 +1,4 @@
import { expect, test } from '../../../../../../testing/integration'
import { expect, test } from '../../../../../testing/integration'
const repoName = 'github.com/sourcegraph/sourcegraph'
const url = `/${repoName}/-/commit/1234567890abcdef`

View File

@ -1,4 +1,4 @@
import { test, expect } from '../../../../../../testing/integration'
import { test, expect } from '../../../../../testing/integration'
const repoName = 'sourcegraph/sourcegraph'
const url = `/${repoName}/-/stats/contributors`

View File

@ -1,4 +1,4 @@
import { test, expect } from '../../../../../testing/integration'
import { test, expect } from '../../../../testing/integration'
const repoName = 'sourcegraph/sourcegraph'

View File

@ -26,33 +26,33 @@ export const svelteKitRoutes: SvelteKitRoute[] = [
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/tree(?:/.*)?/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/branches',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/branches/all',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/all/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/commit/[...revspec]',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/commit(?:/.*)?/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/commits/[...path]',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/commits(?:/.*)?/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/stats/contributors',
id: '/[...repo=reporev]/-/branches',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/-/branches/all',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/all/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/-/commit/[...revspec]',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/commit(?:/.*)?/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/-/stats/contributors',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/stats/contributors/?$'),
isRepoRoot: false,
},
{
id: '/[...repo=reporev]/(validrev)/-/tags',
id: '/[...repo=reporev]/-/tags',
pattern: new RegExp('^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/tags/?$'),
isRepoRoot: false,
},

View File

@ -33,33 +33,33 @@ var svelteKitRoutes = []svelteKitRoute{
Pattern: regexp.MustCompile("^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/tree(?:/.*)?/?$"),
Tag: tags.EnableOptIn | tags.EnableRollout,
},
{
Id: "/[...repo=reporev]/(validrev)/-/branches",
Pattern: regexp.MustCompile("^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/?$"),
Tag: tags.EnableOptIn | tags.EnableRollout,
},
{
Id: "/[...repo=reporev]/(validrev)/-/branches/all",
Pattern: regexp.MustCompile("^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/all/?$"),
Tag: tags.EnableOptIn | tags.EnableRollout,
},
{
Id: "/[...repo=reporev]/(validrev)/-/commit/[...revspec]",
Pattern: regexp.MustCompile("^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/commit(?:/.*)?/?$"),
Tag: tags.EnableOptIn | tags.EnableRollout,
},
{
Id: "/[...repo=reporev]/(validrev)/-/commits/[...path]",
Pattern: regexp.MustCompile("^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/commits(?:/.*)?/?$"),
Tag: tags.EnableOptIn | tags.EnableRollout,
},
{
Id: "/[...repo=reporev]/(validrev)/-/stats/contributors",
Id: "/[...repo=reporev]/-/branches",
Pattern: regexp.MustCompile("^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/?$"),
Tag: tags.EnableOptIn | tags.EnableRollout,
},
{
Id: "/[...repo=reporev]/-/branches/all",
Pattern: regexp.MustCompile("^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/branches/all/?$"),
Tag: tags.EnableOptIn | tags.EnableRollout,
},
{
Id: "/[...repo=reporev]/-/commit/[...revspec]",
Pattern: regexp.MustCompile("^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/commit(?:/.*)?/?$"),
Tag: tags.EnableOptIn | tags.EnableRollout,
},
{
Id: "/[...repo=reporev]/-/stats/contributors",
Pattern: regexp.MustCompile("^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/stats/contributors/?$"),
Tag: tags.EnableOptIn,
},
{
Id: "/[...repo=reporev]/(validrev)/-/tags",
Id: "/[...repo=reporev]/-/tags",
Pattern: regexp.MustCompile("^/(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,})))(?:@(?:(?:(?:[^@/-]|(?:[^/@]{2,}))/)*(?:[^@/-]|(?:[^/@]{2,}))))?/-/tags/?$"),
Tag: tags.EnableOptIn | tags.EnableRollout,
},