Skip to content

Commit c7b192c

Browse files
committed
refactor(ui): derive the nav from the route table, normalize collection paths
1 parent 91c0277 commit c7b192c

17 files changed

Lines changed: 279 additions & 144 deletions

src/main/assets/src/api/types.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,17 @@ export interface JobStatusResponse {
126126
result: string | null
127127
}
128128

129+
export interface SessionCounts {
130+
repositories: number
131+
issues: number
132+
pullRequests: number
133+
}
134+
129135
export interface SessionInfo {
130136
/** null until the first GitHub import completes */
131137
organization: { login: string; name: string; url: string } | null
132138
user: { login: string; name: string | null; url: string } | null
133-
counts: { repositories: number; issues: number; pullRequests: number }
139+
counts: SessionCounts
134140
}
135141

136142
export interface ToolCheck {

src/main/assets/src/components/AppNav.vue

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,21 @@
11
<script setup lang="ts">
22
import { computed, ref } from 'vue'
3-
import { useRoute } from 'vue-router'
3+
import { useRoute, useRouter } from 'vue-router'
44
import ThemeToggle from './ThemeToggle.vue'
55
import { useSession } from '@/composables/useSession'
66
import { useBackgroundJob } from '@/composables/useBackgroundJob'
7+
import { isNavActive, navLinks } from '@/domain/nav'
78
89
const emit = defineEmits<{ validatePom: [] }>()
910
1011
const { session } = useSession()
1112
const { active: backgroundJob } = useBackgroundJob()
1213
const route = useRoute()
14+
const router = useRouter()
1315
const expanded = ref(false)
1416
15-
const counts = computed(() => session.value?.counts)
16-
17-
const links = computed(() => [
18-
{ to: '/', label: 'Repos', badge: counts.value?.repositories, exact: true },
19-
{ to: '/issue', label: 'Issues', badge: counts.value?.issues },
20-
{ to: '/pr', label: 'PRs', badge: counts.value?.pullRequests },
21-
{ to: '/milestone', label: 'Milestones' },
22-
{ to: '/branches', label: 'Branches' },
23-
{ to: '/maven', label: 'Maven projects' },
24-
{ to: '/release', label: 'Release' },
25-
{ to: '/release-notes', label: 'Release notes' },
26-
{ to: '/tool-check', label: 'Tool check' },
27-
])
28-
29-
function isActive(to: string, exact?: boolean): boolean {
30-
// segment-aware, otherwise /release-notes would also light up the /release entry
31-
return exact ? route.path === to : route.path === to || route.path.startsWith(`${to}/`)
32-
}
17+
// derived from the route table, so the labels cannot drift from the page titles again
18+
const links = computed(() => navLinks(router.getRoutes(), session.value?.counts))
3319
</script>
3420

3521
<template>
@@ -64,7 +50,7 @@ function isActive(to: string, exact?: boolean): boolean {
6450
<RouterLink
6551
class="nav-link"
6652
:to="link.to"
67-
:class="{ active: isActive(link.to, link.exact) }"
53+
:class="{ active: isNavActive(link.to, route.path) }"
6854
>
6955
{{ link.label }}
7056
<span v-if="link.badge != null" class="badge chip-secondary">

src/main/assets/src/components/PageShell.vue

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,21 @@
11
<script setup lang="ts">
2-
defineProps<{ title: string; subtitle?: string }>()
2+
import { computed } from 'vue'
3+
import { useRoute } from 'vue-router'
4+
5+
const props = defineProps<{
6+
/** Escape hatch. Normally the heading comes from the route, so it lives in exactly one place. */
7+
title?: string
8+
subtitle?: string
9+
}>()
10+
11+
const route = useRoute()
12+
const heading = computed(() => props.title ?? route.meta.title ?? '')
313
</script>
414

515
<template>
616
<header class="app-page__header d-flex flex-wrap align-items-center justify-content-between gap-2">
717
<div>
8-
<h1 class="mb-0">{{ title }}</h1>
18+
<h1 class="mb-0">{{ heading }}</h1>
919
<p v-if="subtitle" class="text-body-secondary small mb-0">{{ subtitle }}</p>
1020
</div>
1121
<div class="d-flex align-items-center gap-2">
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { isNavActive, navLinks, type NavRouteLike } from './nav'
3+
4+
const COUNTS = { repositories: 12, issues: 34, pullRequests: 5 }
5+
6+
describe('navLinks', () => {
7+
it('includes only routes that ask to be in the nav', () => {
8+
const routes: NavRouteLike[] = [
9+
{ path: '/', meta: { title: 'Repositories', nav: { order: 1 } } },
10+
{ path: '/release/board', meta: { title: 'Release' } },
11+
]
12+
expect(navLinks(routes).map((l) => l.to)).toEqual(['/'])
13+
})
14+
15+
// getRoutes() is sorted by matcher score, not by declaration order, so `order` is load-bearing
16+
it('sorts by the declared order rather than the input order', () => {
17+
const routes: NavRouteLike[] = [
18+
{ path: '/b', meta: { title: 'B', nav: { order: 2 } } },
19+
{ path: '/a', meta: { title: 'A', nav: { order: 1 } } },
20+
]
21+
expect(navLinks(routes).map((l) => l.label)).toEqual(['A', 'B'])
22+
})
23+
24+
it('falls back to the route title, so the two cannot drift', () => {
25+
const routes: NavRouteLike[] = [
26+
{ path: '/', meta: { title: 'Repositories', nav: { order: 1, label: 'Repos' } } },
27+
{ path: '/branches', meta: { title: 'Branches', nav: { order: 2 } } },
28+
]
29+
expect(navLinks(routes).map((l) => l.label)).toEqual(['Repos', 'Branches'])
30+
})
31+
32+
it('resolves a badge against the session counts', () => {
33+
const routes: NavRouteLike[] = [
34+
{ path: '/issues', meta: { title: 'Issues', nav: { order: 1, badge: 'issues' } } },
35+
{ path: '/branches', meta: { title: 'Branches', nav: { order: 2 } } },
36+
]
37+
const links = navLinks(routes, COUNTS)
38+
expect(links[0]!.badge).toBe(34)
39+
expect(links[1]!.badge).toBeUndefined()
40+
})
41+
42+
it('omits the badge entirely until the session has loaded', () => {
43+
const routes: NavRouteLike[] = [
44+
{ path: '/issues', meta: { title: 'Issues', nav: { order: 1, badge: 'issues' } } },
45+
]
46+
expect(navLinks(routes)[0]!.badge).toBeUndefined()
47+
})
48+
49+
it('shows a zero count rather than hiding the badge', () => {
50+
const routes: NavRouteLike[] = [
51+
{ path: '/pull-requests', meta: { title: 'PRs', nav: { order: 1, badge: 'pullRequests' } } },
52+
]
53+
expect(navLinks(routes, { ...COUNTS, pullRequests: 0 })[0]!.badge).toBe(0)
54+
})
55+
})
56+
57+
describe('isNavActive', () => {
58+
it('matches the route itself and anything below it', () => {
59+
expect(isNavActive('/release', '/release')).toBe(true)
60+
expect(isNavActive('/release', '/release/board')).toBe(true)
61+
})
62+
63+
// the whole reason this is segment-aware rather than a prefix test
64+
it('does not let /release-notes activate /release', () => {
65+
expect(isNavActive('/release', '/release-notes')).toBe(false)
66+
})
67+
68+
it('treats the root as exact, so it does not activate on every page', () => {
69+
expect(isNavActive('/', '/')).toBe(true)
70+
expect(isNavActive('/', '/branches')).toBe(false)
71+
expect(isNavActive('/', '/issues')).toBe(false)
72+
})
73+
})

src/main/assets/src/domain/nav.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import type { SessionCounts } from '@/api/types'
2+
3+
/** Where a route appears in the main nav. A route without this is reachable but not linked. */
4+
export interface NavMeta {
5+
/** Defaults to the route's title; set only where the nav wants a shorter word. */
6+
label?: string
7+
order: number
8+
badge?: keyof SessionCounts
9+
}
10+
11+
/** The shape this needs from a vue-router record, so the logic stays free of vue-router. */
12+
export interface NavRouteLike {
13+
path: string
14+
meta: { title?: string; nav?: NavMeta }
15+
}
16+
17+
export interface NavLink {
18+
to: string
19+
label: string
20+
badge?: number
21+
}
22+
23+
/**
24+
* The main nav, derived from the route table so the two cannot drift - they had already, with the
25+
* nav saying "Repos" where the route title said "Repositories".
26+
*
27+
* Ordering is explicit because vue-router's getRoutes() returns matcher-score order, not
28+
* declaration order.
29+
*/
30+
export function navLinks(routes: NavRouteLike[], counts?: SessionCounts): NavLink[] {
31+
return routes
32+
.filter((route) => route.meta.nav !== undefined)
33+
.sort((a, b) => a.meta.nav!.order - b.meta.nav!.order)
34+
.map((route) => {
35+
const nav = route.meta.nav!
36+
const badge = nav.badge ? counts?.[nav.badge] : undefined
37+
return {
38+
to: route.path,
39+
label: nav.label ?? route.meta.title ?? route.path,
40+
...(badge === undefined ? {} : { badge }),
41+
}
42+
})
43+
}
44+
45+
/**
46+
* Segment-aware, so /release-notes does not light up the /release entry. `/` is naturally exact,
47+
* since no other path starts with `//`.
48+
*/
49+
export function isNavActive(to: string, currentPath: string): boolean {
50+
return currentPath === to || currentPath.startsWith(`${to}/`)
51+
}

src/main/assets/src/router/index.ts

Lines changed: 3 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,100 +1,7 @@
1-
import { createRouter, createWebHistory, type RouteRecordRaw } from 'vue-router'
2-
import { useReleaseSummary } from '@/composables/useReleaseSummary'
1+
import { createRouter, createWebHistory } from 'vue-router'
2+
import { routes } from './routes'
33

4-
export type IssueKind = 'issue' | 'pr'
5-
6-
declare module 'vue-router' {
7-
interface RouteMeta {
8-
title?: string
9-
kind?: IssueKind
10-
}
11-
}
12-
13-
/**
14-
* Which release page to show depends on whether a descriptor is persisted - the same decision the
15-
* server used to make when it picked a template. On a failed probe the selection page is the safe
16-
* landing: it cannot act on a release that may not exist.
17-
*/
18-
async function hasRelease(): Promise<boolean> {
19-
const summary = await useReleaseSummary().refresh()
20-
return summary?.hasDescriptor === true
21-
}
22-
23-
const routes: RouteRecordRaw[] = [
24-
{
25-
path: '/',
26-
name: 'repos',
27-
component: () => import('@/views/RepositoriesView.vue'),
28-
meta: { title: 'Repositories' },
29-
},
30-
{
31-
path: '/issue',
32-
name: 'issues',
33-
component: () => import('@/views/IssueListView.vue'),
34-
meta: { title: 'Issues', kind: 'issue' },
35-
},
36-
{
37-
path: '/pr',
38-
name: 'pullRequests',
39-
component: () => import('@/views/IssueListView.vue'),
40-
meta: { title: 'Pull requests', kind: 'pr' },
41-
},
42-
{
43-
path: '/milestone',
44-
name: 'milestones',
45-
component: () => import('@/views/MilestonesView.vue'),
46-
meta: { title: 'Milestones' },
47-
},
48-
{
49-
path: '/branches',
50-
name: 'branches',
51-
component: () => import('@/views/BranchesView.vue'),
52-
meta: { title: 'Branches' },
53-
},
54-
{
55-
path: '/maven',
56-
name: 'maven',
57-
component: () => import('@/views/MavenView.vue'),
58-
meta: { title: 'Maven projects' },
59-
},
60-
{
61-
// the selection page's own guard forwards to the board when a release is already in flight
62-
path: '/release',
63-
redirect: '/release/select-projects',
64-
},
65-
{
66-
path: '/release/select-projects',
67-
name: 'releaseSelect',
68-
component: () => import('@/views/ReleaseSelectView.vue'),
69-
meta: { title: 'Start a release' },
70-
beforeEnter: async () => ((await hasRelease()) ? '/release/board' : true),
71-
},
72-
{
73-
path: '/release/board',
74-
name: 'releaseBoard',
75-
component: () => import('@/views/ReleaseBoardView.vue'),
76-
meta: { title: 'Release' },
77-
beforeEnter: async () => ((await hasRelease()) ? true : '/release/select-projects'),
78-
},
79-
{
80-
path: '/release-notes',
81-
name: 'releaseNotes',
82-
component: () => import('@/views/ReleaseNotesView.vue'),
83-
meta: { title: 'Release notes' },
84-
},
85-
{
86-
path: '/tool-check',
87-
name: 'toolCheck',
88-
component: () => import('@/views/ToolCheckView.vue'),
89-
meta: { title: 'Tool check' },
90-
},
91-
{
92-
path: '/:pathMatch(.*)*',
93-
name: 'notFound',
94-
component: () => import('@/views/NotFoundView.vue'),
95-
meta: { title: 'Not found' },
96-
},
97-
]
4+
export type { IssueKind } from './routes'
985

996
export const router = createRouter({
1007
history: createWebHistory('/ui/'),

0 commit comments

Comments
 (0)