From 80c6b4b47e6437ce04597d76118352855081c2bf Mon Sep 17 00:00:00 2001
From: Vadym Samoilenko <vadymsamoilenko@oliver.agency>
Date: Wed, 13 May 2026 11:34:18 +0100
Subject: [PATCH] fix(ui): address code review findings

- ConfirmDialog: guard handleConfirm against AlertDialogAction :disabled bypass
- useDebounce: add { deep: true } so object (filter record) mutations trigger debounce
- useFocusTrap: defer focus restore by 150ms to not interrupt close animation
- KpiRow: migrate to ui/KpiCard (proper interface, tone, lucide icons, animated counter)
- ui/KpiCard: add optional `to` prop with RouterLink support
- Remove legacy dashboard/KpiCard.vue (replaced by ui/KpiCard)
- Fix pre-existing lint errors in TopBar and color.ts

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 web/src/components/dashboard/KpiCard.vue | 112 -----------------------
 web/src/components/dashboard/KpiRow.vue  |  17 ++--
 web/src/components/shared/TopBar.vue     |   1 -
 web/src/components/ui/ConfirmDialog.vue  |   4 +-
 web/src/components/ui/KpiCard.vue        |  11 ++-
 web/src/composables/useDebounce.ts       |   2 +-
 web/src/composables/useFocusTrap.ts      |   8 +-
 web/src/lib/color.ts                     |   1 +
 8 files changed, 27 insertions(+), 129 deletions(-)
 delete mode 100644 web/src/components/dashboard/KpiCard.vue

diff --git a/web/src/components/dashboard/KpiCard.vue b/web/src/components/dashboard/KpiCard.vue
deleted file mode 100644
index 20f366d..0000000
--- a/web/src/components/dashboard/KpiCard.vue
+++ /dev/null
@@ -1,112 +0,0 @@
-<script setup lang="ts">
-import { computed } from 'vue'
-import { RouterLink } from 'vue-router'
-import Card from '@/components/ui/Card.vue'
-import CardContent from '@/components/ui/CardContent.vue'
-
-const props = defineProps<{
-  label: string
-  value: string | number
-  icon?: string
-  trend?: number
-  description?: string
-  loading?: boolean
-  hero?: boolean
-  to?: string
-}>()
-
-const cardClass = computed(() =>
-  props.hero
-    ? 'relative overflow-hidden transition-all duration-200 border-primary/20 bg-primary/5 ring-1 ring-primary/15 panel-glow-hover'
-    : 'relative overflow-hidden transition-all duration-200 border-border/60 panel-glow-hover'
-)
-</script>
-
-<template>
-  <component
-    :is="to ? RouterLink : 'div'"
-    :to="to"
-    :class="to ? 'block hover:opacity-90 transition-opacity' : ''"
-  >
-    <Card :class="cardClass">
-      <!-- Subtle corner accent -->
-      <span class="pointer-events-none absolute -right-4 -top-4 h-14 w-14 rounded-full"
-        :class="hero ? 'bg-primary/10' : 'bg-primary/5'" />
-      <span class="pointer-events-none absolute -right-1 -top-1 h-6 w-6 rounded-full"
-        :class="hero ? 'bg-primary/15' : 'bg-primary/8'" />
-
-      <CardContent class="p-5">
-        <div class="flex items-start justify-between gap-2">
-          <div class="flex-1 min-w-0">
-            <p class="text-[10px] font-semibold uppercase tracking-[0.1em] truncate"
-              :class="hero ? 'text-primary/80' : 'text-muted-foreground'">
-              {{ label }}
-            </p>
-            <div class="mt-2">
-              <div v-if="loading" class="h-7 w-20 bg-muted animate-pulse rounded" />
-              <p v-else
-                class="kpi-value font-bold tracking-tight leading-none"
-                :class="hero ? 'text-3xl text-primary' : 'text-2xl text-foreground'">
-                {{ value }}
-              </p>
-            </div>
-            <p v-if="description" class="text-xs text-muted-foreground mt-1.5 truncate">
-              {{ description }}
-            </p>
-          </div>
-
-          <!-- Icon -->
-          <div
-            v-if="icon"
-            class="rounded-xl flex items-center justify-center shrink-0"
-            :class="[
-              hero ? 'h-11 w-11 bg-primary/15 ring-1 ring-primary/25' : 'h-9 w-9 bg-muted ring-1 ring-border',
-            ]"
-          >
-            <svg v-if="icon === 'clock'" :class="['shrink-0', hero ? 'h-5 w-5 text-primary' : 'h-4 w-4 text-muted-foreground']" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-              <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 8v4l3 3m6-3a9 9 0 11-18 0 9 9 0 0118 0z" />
-            </svg>
-            <svg v-else-if="icon === 'calendar'" :class="['shrink-0', hero ? 'h-5 w-5 text-primary' : 'h-4 w-4 text-muted-foreground']" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-              <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M8 7V3m8 4V3m-9 8h10M5 21h14a2 2 0 002-2V7a2 2 0 00-2-2H5a2 2 0 00-2 2v12a2 2 0 002 2z" />
-            </svg>
-            <svg v-else-if="icon === 'folder'" :class="['shrink-0', hero ? 'h-5 w-5 text-primary' : 'h-4 w-4 text-muted-foreground']" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-              <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M3 7a2 2 0 012-2h3.586a1 1 0 01.707.293l1.414 1.414A1 1 0 0011.414 7H19a2 2 0 012 2v9a2 2 0 01-2 2H5a2 2 0 01-2-2V7z" />
-            </svg>
-            <svg v-else-if="icon === 'trending-up'" :class="['shrink-0', hero ? 'h-5 w-5 text-primary' : 'h-4 w-4 text-muted-foreground']" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-              <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M13 7h8m0 0v8m0-8l-8 8-4-4-6 6" />
-            </svg>
-            <svg v-else-if="icon === 'git'" :class="['shrink-0', hero ? 'h-5 w-5 text-primary' : 'h-4 w-4 text-muted-foreground']" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-              <circle cx="12" cy="12" r="4" stroke="currentColor" stroke-width="2" />
-              <path stroke-linecap="round" stroke-width="2" d="M2 12h6M16 12h6" />
-            </svg>
-            <svg v-else :class="['shrink-0', hero ? 'h-5 w-5 text-primary' : 'h-4 w-4 text-muted-foreground']" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-              <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M11.049 2.927c.3-.921 1.603-.921 1.902 0l1.519 4.674a1 1 0 00.95.69h4.915c.969 0 1.371 1.24.588 1.81l-3.976 2.888a1 1 0 00-.363 1.118l1.518 4.674c.3.922-.755 1.688-1.538 1.118l-3.976-2.888a1 1 0 00-1.176 0l-3.976 2.888c-.783.57-1.838-.197-1.538-1.118l1.518-4.674a1 1 0 00-.363-1.118l-3.976-2.888c-.784-.57-.38-1.81.588-1.81h4.914a1 1 0 00.951-.69l1.519-4.674z" />
-            </svg>
-          </div>
-        </div>
-
-        <!-- Trend indicator -->
-        <div v-if="trend !== undefined" class="mt-3 flex items-center gap-1.5 text-xs">
-          <div
-            :class="[
-              'flex items-center gap-1 font-semibold tabular-nums',
-              trend > 0 ? 'text-[hsl(var(--success))]' : trend < 0 ? 'text-destructive' : 'text-muted-foreground',
-            ]"
-          >
-            <svg class="h-3 w-3" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-              <path v-if="trend > 0" stroke-linecap="round" stroke-linejoin="round" stroke-width="2.5" d="M5 10l7-7m0 0l7 7m-7-7v18" />
-              <path v-else-if="trend < 0" stroke-linecap="round" stroke-linejoin="round" stroke-width="2.5" d="M19 14l-7 7m0 0l-7-7m7 7V3" />
-              <path v-else stroke-linecap="round" stroke-linejoin="round" stroke-width="2.5" d="M5 12h14" />
-            </svg>
-            {{ trend > 0 ? '+' : '' }}{{ Math.abs(trend) }}%
-          </div>
-          <span class="text-muted-foreground">vs last period</span>
-        </div>
-
-        <!-- Bottom accent bar -->
-        <div class="mt-3 h-px rounded-full"
-          :class="hero ? 'w-full bg-primary/20' : 'w-10 bg-primary/20'" />
-      </CardContent>
-    </Card>
-  </component>
-</template>
diff --git a/web/src/components/dashboard/KpiRow.vue b/web/src/components/dashboard/KpiRow.vue
index a5d1f78..c5e2ca9 100644
--- a/web/src/components/dashboard/KpiRow.vue
+++ b/web/src/components/dashboard/KpiRow.vue
@@ -1,5 +1,6 @@
 <script setup lang="ts">
-import KpiCard from '@/components/dashboard/KpiCard.vue'
+import { Clock, CalendarDays, FolderOpen, TrendingUp, Star, GitCommitHorizontal } from 'lucide-vue-next'
+import KpiCard from '@/components/ui/KpiCard.vue'
 import { formatDuration } from '@/lib/utils'
 import type { KpiSummary } from '@/types'
 
@@ -14,40 +15,40 @@ defineProps<{
     <KpiCard
       label="Total Hours"
       :value="summary ? formatDuration(summary.total_hours) : '—'"
-      icon="clock"
+      :icon="Clock"
+      tone="primary"
       :loading="loading"
-      :hero="true"
     />
     <KpiCard
       label="Working Days"
       :value="summary?.working_days ?? '—'"
-      icon="calendar"
+      :icon="CalendarDays"
       :loading="loading"
     />
     <KpiCard
       label="Projects"
       :value="summary?.total_projects ?? '—'"
-      icon="folder"
+      :icon="FolderOpen"
       :loading="loading"
       to="/projects"
     />
     <KpiCard
       label="Avg / Day"
       :value="summary ? formatDuration(summary.avg_hours_per_day) : '—'"
-      icon="trending-up"
+      :icon="TrendingUp"
       :loading="loading"
     />
     <KpiCard
       label="Top Project"
       :value="summary?.top_project ?? '—'"
-      icon="star"
+      :icon="Star"
       :loading="loading"
       to="/projects"
     />
     <KpiCard
       label="Commits"
       :value="summary?.total_commits ?? '—'"
-      icon="git"
+      :icon="GitCommitHorizontal"
       :loading="loading"
     />
   </div>
diff --git a/web/src/components/shared/TopBar.vue b/web/src/components/shared/TopBar.vue
index 93d5375..789f34a 100644
--- a/web/src/components/shared/TopBar.vue
+++ b/web/src/components/shared/TopBar.vue
@@ -1,7 +1,6 @@
 <script setup lang="ts">
 import { useAuthStore } from '@/stores/auth'
 import { useRouter } from 'vue-router'
-import Button from '@/components/ui/Button.vue'
 import { toast } from 'vue-sonner'
 
 defineProps<{
diff --git a/web/src/components/ui/ConfirmDialog.vue b/web/src/components/ui/ConfirmDialog.vue
index 8174cf2..0d6b9f9 100644
--- a/web/src/components/ui/ConfirmDialog.vue
+++ b/web/src/components/ui/ConfirmDialog.vue
@@ -61,7 +61,7 @@ import { CircleAlert } from 'lucide-vue-next'
 import Button from './Button.vue'
 import Input from './Input.vue'
 
-withDefaults(defineProps<{
+const props = withDefaults(defineProps<{
   open: boolean
   title: string
   description: string
@@ -84,6 +84,8 @@ const emit = defineEmits<{
 const typedText = ref('')
 
 function handleConfirm() {
+  // Guard: AlertDialogAction bypasses :disabled, so enforce the check here too
+  if (props.confirmText && typedText.value !== props.confirmText) return
   emit('confirm')
   emit('update:open', false)
   typedText.value = ''
diff --git a/web/src/components/ui/KpiCard.vue b/web/src/components/ui/KpiCard.vue
index 50ebabd..0e21fc0 100644
--- a/web/src/components/ui/KpiCard.vue
+++ b/web/src/components/ui/KpiCard.vue
@@ -1,8 +1,11 @@
 <template>
-  <div
+  <component
+    :is="to ? RouterLink : 'div'"
+    :to="to"
     :class="cn(
       'relative overflow-hidden rounded-xl p-5 shadow-sm',
-      toneClasses[tone].card
+      toneClasses[tone].card,
+      to ? 'block hover:opacity-90 transition-opacity cursor-pointer' : ''
     )"
   >
     <!-- Corner pulse decoration -->
@@ -46,12 +49,13 @@
 
     <!-- Bottom accent bar -->
     <div :class="cn('mt-4 h-0.5 w-16 rounded opacity-60', toneClasses[tone].bar)" />
-  </div>
+  </component>
 </template>
 
 <script setup lang="ts">
 import { computed } from 'vue'
 import type { Component } from 'vue'
+import { RouterLink } from 'vue-router'
 import { TrendingUp, TrendingDown, Minus } from 'lucide-vue-next'
 import { cn } from '@/lib/utils'
 import Skeleton from './Skeleton.vue'
@@ -69,6 +73,7 @@ const props = withDefaults(defineProps<{
   icon?: Component
   tone?: Tone
   loading?: boolean
+  to?: string
 }>(), {
   tone: 'default',
   trend: 'flat',
diff --git a/web/src/composables/useDebounce.ts b/web/src/composables/useDebounce.ts
index d5c33c4..7cfc38e 100644
--- a/web/src/composables/useDebounce.ts
+++ b/web/src/composables/useDebounce.ts
@@ -10,7 +10,7 @@ export function useDebouncedRef<T>(source: Ref<T>, ms: number): Ref<T> {
     timeout = setTimeout(() => {
       debounced.value = val
     }, ms)
-  })
+  }, { deep: true })
 
   return debounced
 }
diff --git a/web/src/composables/useFocusTrap.ts b/web/src/composables/useFocusTrap.ts
index 845c860..e945ee8 100644
--- a/web/src/composables/useFocusTrap.ts
+++ b/web/src/composables/useFocusTrap.ts
@@ -44,9 +44,11 @@ export function useFocusTrap(containerRef: Ref<HTMLElement | null>, isOpen: Ref<
       }, 50)
     } else {
       document.removeEventListener('keydown', onKeydown)
-      if (previousFocus && 'focus' in previousFocus) {
-        (previousFocus as HTMLElement).focus()
-      }
+      // Defer past close animation so focus restore doesn't interrupt it
+      const saved = previousFocus
+      setTimeout(() => {
+        if (saved && 'focus' in saved) (saved as HTMLElement).focus()
+      }, 150)
     }
   })
 
diff --git a/web/src/lib/color.ts b/web/src/lib/color.ts
index 5e51712..0a33caf 100644
--- a/web/src/lib/color.ts
+++ b/web/src/lib/color.ts
@@ -10,6 +10,7 @@ export function hslBorderFromHue(hue: number): string {
   return `hsla(${hue}, 65%, 55%, 1)`
 }
 
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
 export function contrastColor(_hue: number): string {
   return 'white'
 }