From 8444ea74433926da32821bd3c3c013659ed4483f Mon Sep 17 00:00:00 2001
From: "bert.hausmans" <bert@hausmans.nl>
Date: Sat, 16 May 2026 19:39:56 +0200
Subject: [PATCH] fix(gui-v2): SidebarNav uses RouterLink (a11y) + review-nit
 cleanup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- 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>
---
 .../src/components-v2/layout/SidebarNav.vue   | 105 ++++++++++--------
 .../composables/__tests__/useV2Nav.spec.ts    |  28 ++++-
 apps/app/src/composables/useV2Nav.ts          |   2 +-
 3 files changed, 84 insertions(+), 51 deletions(-)

diff --git a/apps/app/src/components-v2/layout/SidebarNav.vue b/apps/app/src/components-v2/layout/SidebarNav.vue
index adbb276d..7fac0a39 100644
--- a/apps/app/src/components-v2/layout/SidebarNav.vue
+++ b/apps/app/src/components-v2/layout/SidebarNav.vue
@@ -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 {
diff --git a/apps/app/src/composables/__tests__/useV2Nav.spec.ts b/apps/app/src/composables/__tests__/useV2Nav.spec.ts
index dd7a4e9b..81e74c6e 100644
--- a/apps/app/src/composables/__tests__/useV2Nav.spec.ts
+++ b/apps/app/src/composables/__tests__/useV2Nav.spec.ts
@@ -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)
+  })
+})
diff --git a/apps/app/src/composables/useV2Nav.ts b/apps/app/src/composables/useV2Nav.ts
index ec1aa57a..3d398c5e 100644
--- a/apps/app/src/composables/useV2Nav.ts
+++ b/apps/app/src/composables/useV2Nav.ts
@@ -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 },