fix(gui-v2): SidebarNav uses RouterLink (a11y) + review-nit cleanup
- FIX 1: Replace <button @click="router.push"> with <RouterLink custom> + <a> for real link semantics (middle-click, ⌘-click, screen-reader); custom isNavItemActive prefix-match stays the active source of truth; adds :aria-current="page" on active items; drops useRouter/router.push. RouterLink to prop cast via itemTo() helper (RouteLocationRaw from unplugin-vue-router) to satisfy typed RouterLinkTyped<RouteNamedMap>. - FIX 2: Align .nav-item comment to actual template values (py-[9px] rounded-md, not CSS vars); replace inaccurate Tailwind v3/v4 before: composability justification in <style scoped> with the real reason (accent bar at left:-10px is clipped by the overflow-y-auto nav). - FIX 3: text-left → text-start (logical property, RTL-safe). - FIX 4: Document id=route-name assumption in useV2Nav.ts with a one-line comment at the id: assignment. - FIX 5: Reword misleading "dotted names" spec description to state the real invariant (id = v1 route name, already kebab-case). - FIX 6: Add 2 tests — useV2Nav wrapper .value equality, and consecutive-headings edge case (empty-items group produced). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
<script setup lang="ts">
|
||||
import { useRoute, useRouter } from 'vue-router'
|
||||
import { useRoute } from 'vue-router'
|
||||
import type { RouteLocationRaw } from 'unplugin-vue-router'
|
||||
import Icon from '@/components/Icon.vue'
|
||||
import { isNavItemActive } from '@/components-v2/layout/sidebarNavActive'
|
||||
import type { V2NavGroup, V2NavItem } from '@/types/v2/nav'
|
||||
@@ -9,16 +10,18 @@ defineProps<{
|
||||
collapsed: boolean
|
||||
}>()
|
||||
|
||||
const router = useRouter()
|
||||
const route = useRoute()
|
||||
|
||||
function navigate(item: V2NavItem): void {
|
||||
router.push(item.to)
|
||||
}
|
||||
|
||||
function checkActive(item: V2NavItem): boolean {
|
||||
return isNavItemActive(item, route.name)
|
||||
}
|
||||
|
||||
// Cast V2NavItem.to (vue-router generic RouteLocationRaw) to the typed version
|
||||
// expected by RouterLinkTyped. V2NavItem.to values are always named-route objects
|
||||
// from the RouteNamedMap so the cast is sound.
|
||||
function itemTo(item: V2NavItem): RouteLocationRaw {
|
||||
return item.to as RouteLocationRaw
|
||||
}
|
||||
</script>
|
||||
|
||||
<template>
|
||||
@@ -30,10 +33,10 @@ function checkActive(item: V2NavItem): boolean {
|
||||
:not(:first-child) pseudo equivalent — `[&:not(:first-child)]:mt-[18px]`)
|
||||
.nav-label → text-[11px] font-semibold text-surface-500 dark:text-surface-400
|
||||
uppercase tracking-[0.06em] px-2.5 pb-1.5 whitespace-nowrap overflow-hidden
|
||||
.nav-item → flex items-center gap-3 py-[var(--nav-y,9px)] px-2.5 rounded-[var(--radius,6px)]
|
||||
.nav-item → flex items-center gap-3 py-[9px] px-2.5 rounded-md
|
||||
text-surface-600 dark:text-surface-300 text-[13.5px] font-medium
|
||||
transition-[background,color] duration-150 whitespace-nowrap relative
|
||||
cursor-pointer border-0 bg-transparent w-full text-left min-w-0
|
||||
cursor-pointer w-full text-start min-w-0
|
||||
.nav-item:hover → hover:bg-surface-100 dark:hover:bg-surface-800 hover:text-surface-900 dark:hover:text-surface-0
|
||||
.nav-item.active → bg-primary-50 dark:bg-primary-950 text-primary-600 dark:text-primary-400 font-semibold
|
||||
.nav-item.active::before → the left accent bar — inexpressible in Tailwind without a plugin
|
||||
@@ -58,47 +61,55 @@ function checkActive(item: V2NavItem): boolean {
|
||||
{{ group.label }}
|
||||
</div>
|
||||
|
||||
<button
|
||||
<RouterLink
|
||||
v-for="item in group.items"
|
||||
:key="item.id"
|
||||
class="flex items-center gap-3 py-[9px] rounded-md text-[13.5px] font-medium transition-[background,color] duration-150 whitespace-nowrap relative cursor-pointer border-0 bg-transparent w-full text-left min-w-0"
|
||||
:class="[
|
||||
collapsed ? 'justify-center px-0' : 'px-2.5',
|
||||
checkActive(item)
|
||||
? 'nav-item-active bg-primary-50 dark:bg-primary-950 text-primary-600 dark:text-primary-400 font-semibold'
|
||||
: 'text-surface-600 dark:text-surface-300 hover:bg-surface-100 dark:hover:bg-surface-800 hover:text-surface-900 dark:hover:text-surface-0',
|
||||
]"
|
||||
:title="collapsed ? item.label : undefined"
|
||||
:aria-label="item.label"
|
||||
@click="navigate(item)"
|
||||
v-slot="{ href, navigate }"
|
||||
:to="itemTo(item)"
|
||||
custom
|
||||
>
|
||||
<Icon
|
||||
:name="item.icon"
|
||||
:size="18"
|
||||
class="flex-shrink-0"
|
||||
/>
|
||||
|
||||
<!-- Text label: hidden in collapsed mode -->
|
||||
<span
|
||||
v-if="!collapsed"
|
||||
class="overflow-hidden text-ellipsis min-w-0"
|
||||
>
|
||||
{{ item.label }}
|
||||
</span>
|
||||
|
||||
<!-- Count badge: hidden in collapsed mode -->
|
||||
<span
|
||||
v-if="item.count != null && !collapsed"
|
||||
class="ms-auto text-[11px] font-semibold px-1.5 py-px rounded-full"
|
||||
<a
|
||||
:href="href"
|
||||
class="flex items-center gap-3 py-[9px] rounded-md text-[13.5px] font-medium transition-[background,color] duration-150 whitespace-nowrap relative cursor-pointer w-full text-start min-w-0"
|
||||
:class="[
|
||||
collapsed ? 'justify-center px-0' : 'px-2.5',
|
||||
checkActive(item)
|
||||
? 'bg-primary-600 dark:bg-primary-500 text-white'
|
||||
: 'bg-surface-100 dark:bg-surface-800 text-surface-500 dark:text-surface-400',
|
||||
? 'nav-item-active bg-primary-50 dark:bg-primary-950 text-primary-600 dark:text-primary-400 font-semibold'
|
||||
: 'text-surface-600 dark:text-surface-300 hover:bg-surface-100 dark:hover:bg-surface-800 hover:text-surface-900 dark:hover:text-surface-0',
|
||||
]"
|
||||
:aria-current="checkActive(item) ? 'page' : undefined"
|
||||
:aria-label="item.label"
|
||||
:title="collapsed ? item.label : undefined"
|
||||
@click="navigate"
|
||||
>
|
||||
{{ item.count }}
|
||||
</span>
|
||||
</button>
|
||||
<Icon
|
||||
:name="item.icon"
|
||||
:size="18"
|
||||
class="flex-shrink-0"
|
||||
/>
|
||||
|
||||
<!-- Text label: hidden in collapsed mode -->
|
||||
<span
|
||||
v-if="!collapsed"
|
||||
class="overflow-hidden text-ellipsis min-w-0"
|
||||
>
|
||||
{{ item.label }}
|
||||
</span>
|
||||
|
||||
<!-- Count badge: hidden in collapsed mode -->
|
||||
<span
|
||||
v-if="item.count != null && !collapsed"
|
||||
class="ms-auto text-[11px] font-semibold px-1.5 py-px rounded-full"
|
||||
:class="[
|
||||
checkActive(item)
|
||||
? 'bg-primary-600 dark:bg-primary-500 text-white'
|
||||
: 'bg-surface-100 dark:bg-surface-800 text-surface-500 dark:text-surface-400',
|
||||
]"
|
||||
>
|
||||
{{ item.count }}
|
||||
</span>
|
||||
</a>
|
||||
</RouterLink>
|
||||
</div>
|
||||
</nav>
|
||||
</template>
|
||||
@@ -106,12 +117,10 @@ function checkActive(item: V2NavItem): boolean {
|
||||
<style scoped>
|
||||
/*
|
||||
* The active left-accent bar (.nav-item.active::before in crewli-starter main.css).
|
||||
* position:absolute with left:-10px, specific width/height, and transform:translateY(-50%)
|
||||
* cannot be expressed with Tailwind utilities alone — no utility maps to a
|
||||
* pseudo-element with these exact values, and arbitrary-value pseudo variants
|
||||
* like before:content-[''] with left-[-10px] do not compose with transform/height
|
||||
* reliably across Tailwind v3/v4. A <style scoped> pseudo-element is the
|
||||
* correct Tailwind-escape-hatch per RFC (customization order: Tailwind →
|
||||
* The bar sits at left:-10px — outside the row boundary. The parent <nav> is an
|
||||
* overflow-y-auto scroll container which clips cross-axis overflow, so a Tailwind
|
||||
* before: utility would be clipped by that parent overflow context. A <style scoped>
|
||||
* pseudo-element keeps the exception self-contained (customization order: Tailwind →
|
||||
* pt API → Aura → <style scoped> last resort with comment).
|
||||
*/
|
||||
.nav-item-active::before {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { toV2NavGroups } from '@/composables/useV2Nav'
|
||||
import { toV2NavGroups, useV2Nav } from '@/composables/useV2Nav'
|
||||
import type { V2NavItem } from '@/types/v2/nav'
|
||||
|
||||
// Hand-built fixture — does NOT depend on the real orgNavItems contents.
|
||||
@@ -52,7 +52,7 @@ describe('toV2NavGroups', () => {
|
||||
expect(alpha.count).toBeUndefined()
|
||||
})
|
||||
|
||||
it('id is derived from the route name (kebab already for dotted names)', () => {
|
||||
it('id equals the v1 route name (already kebab-case; no normalisation applied)', () => {
|
||||
const groups = toV2NavGroups(fixture)
|
||||
const delta = groups[2].items[0] as V2NavItem
|
||||
|
||||
@@ -75,3 +75,27 @@ describe('toV2NavGroups', () => {
|
||||
expect(groups[0].items).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('useV2Nav', () => {
|
||||
it('returns a computed whose .value equals toV2NavGroups(items)', () => {
|
||||
const { groups } = useV2Nav(fixture)
|
||||
|
||||
expect(groups.value).toEqual(toV2NavGroups(fixture))
|
||||
})
|
||||
|
||||
it('consecutive headings produce an empty-items group then the next group', () => {
|
||||
const consecutiveHeadings = [
|
||||
{ heading: 'First' },
|
||||
{ heading: 'Second' },
|
||||
{ title: 'Alpha', to: { name: 'alpha' }, icon: { icon: 'tabler-home' } },
|
||||
] as const
|
||||
|
||||
const groups = toV2NavGroups(consecutiveHeadings)
|
||||
|
||||
expect(groups).toHaveLength(2)
|
||||
expect(groups[0].label).toBe('First')
|
||||
expect(groups[0].items).toHaveLength(0)
|
||||
expect(groups[1].label).toBe('Second')
|
||||
expect(groups[1].items).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -55,7 +55,7 @@ export function toV2NavGroups(items: readonly V1NavEntry[]): V2NavGroup[] {
|
||||
current = { label: '', items: [] }
|
||||
|
||||
const navItem: V2NavItem = {
|
||||
id: entry.to.name,
|
||||
id: entry.to.name, // v1 route names are already kebab-case; no normalisation needed
|
||||
label: entry.title,
|
||||
icon: entry.icon.icon,
|
||||
to: { name: entry.to.name },
|
||||
|
||||
Reference in New Issue
Block a user