Address code review feedback: use design system colors, constants, and custom dialogs

Co-authored-by: ankurrera <186232326+ankurrera@users.noreply.github.com>

Author: Copilot

src/components/skills/CharacteristicCard.tsx

@@ -4,6 +4,7 @@ import { Button } from "@/components/ui/button";
 import { Input } from "@/components/ui/input";
 import { calculateLevelProgress } from "@/lib/levelCalculation";
 import { Edit2, Trash2, Save, X, Star } from "lucide-react";
+import ConfirmDialog from "./ConfirmDialog";
 
 interface CharacteristicCardProps {
   characteristic: Characteristic;
@@ -12,6 +13,7 @@ interface CharacteristicCardProps {
 const CharacteristicCard = ({ characteristic }: CharacteristicCardProps) => {
   const { updateCharacteristic, deleteCharacteristic } = useCharacteristics();
   const [isEditing, setIsEditing] = useState(false);
+  const [showDeleteDialog, setShowDeleteDialog] = useState(false);
   const [editName, setEditName] = useState(characteristic.name);
   const [editIcon, setEditIcon] = useState(characteristic.icon);
   const [editXP, setEditXP] = useState(characteristic.xp.toString());
@@ -36,13 +38,17 @@ const CharacteristicCard = ({ characteristic }: CharacteristicCardProps) => {
   };
 
   const handleDelete = () => {
-    if (window.confirm(`Delete characteristic "${characteristic.name}"?`)) {
-      deleteCharacteristic.mutate(characteristic.id);
-    }
+    setShowDeleteDialog(true);
+  };
+
+  const confirmDelete = () => {
+    deleteCharacteristic.mutate(characteristic.id);
+    setShowDeleteDialog(false);
   };
 
   return (
-    <div className="system-panel p-4 space-y-3">
+    <>
+      <div className="system-panel p-4 space-y-3">
       {/* Header */}
       <div className="flex items-start justify-between">
         <div className="flex items-center gap-3 flex-1">
@@ -86,7 +92,7 @@ const CharacteristicCard = ({ characteristic }: CharacteristicCardProps) => {
               <Button
                 variant="ghost"
                 size="icon"
-                className="h-8 w-8 text-green-600 hover:text-green-700 hover:bg-green-50"
+                className="h-8 w-8 text-primary hover:text-primary/80 hover:bg-primary/10"
                 onClick={handleSave}
               >
                 <Save className="w-4 h-4" />
@@ -153,6 +159,16 @@ const CharacteristicCard = ({ characteristic }: CharacteristicCardProps) => {
         </div>
       </div>
     </div>
+
+      {/* Delete Confirmation Dialog */}
+      <ConfirmDialog
+        open={showDeleteDialog}
+        onOpenChange={setShowDeleteDialog}
+        title="Delete Characteristic"
+        description={`Are you sure you want to delete "${characteristic.name}"? This action cannot be undone.`}
+        onConfirm={confirmDelete}
+      />
+    </>
   );
 };
 

src/components/skills/ConfirmDialog.tsx

@@ -0,0 +1,58 @@
+import {
+  AlertDialog,
+  AlertDialogAction,
+  AlertDialogCancel,
+  AlertDialogContent,
+  AlertDialogDescription,
+  AlertDialogFooter,
+  AlertDialogHeader,
+  AlertDialogTitle,
+} from "@/components/ui/alert-dialog";
+
+interface ConfirmDialogProps {
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+  title: string;
+  description: string;
+  onConfirm: () => void;
+  confirmText?: string;
+  cancelText?: string;
+}
+
+const ConfirmDialog = ({
+  open,
+  onOpenChange,
+  title,
+  description,
+  onConfirm,
+  confirmText = "Delete",
+  cancelText = "Cancel",
+}: ConfirmDialogProps) => {
+  return (
+    <AlertDialog open={open} onOpenChange={onOpenChange}>
+      <AlertDialogContent className="system-panel">
+        <AlertDialogHeader>
+          <AlertDialogTitle className="text-foreground">
+            {title}
+          </AlertDialogTitle>
+          <AlertDialogDescription className="text-muted-foreground">
+            {description}
+          </AlertDialogDescription>
+        </AlertDialogHeader>
+        <AlertDialogFooter>
+          <AlertDialogCancel className="border-border hover:bg-muted">
+            {cancelText}
+          </AlertDialogCancel>
+          <AlertDialogAction
+            onClick={onConfirm}
+            className="bg-destructive hover:bg-destructive/90 text-destructive-foreground"
+          >
+            {confirmText}
+          </AlertDialogAction>
+        </AlertDialogFooter>
+      </AlertDialogContent>
+    </AlertDialog>
+  );
+};
+
+export default ConfirmDialog;

src/components/skills/CreateCharacteristicForm.tsx

@@ -3,17 +3,16 @@ import { useCharacteristics } from "@/hooks/useCharacteristics";
 import { Button } from "@/components/ui/button";
 import { Input } from "@/components/ui/input";
 import { X } from "lucide-react";
+import { CHARACTERISTIC_ICONS } from "@/lib/skillsConstants";
 
 interface CreateCharacteristicFormProps {
   onClose: () => void;
 }
 
-const iconOptions = ["⭐", "💪", "🧠", "❤️", "⚡", "🎯", "🛡️", "⚔️", "🏃", "🎨"];
-
 const CreateCharacteristicForm = ({ onClose }: CreateCharacteristicFormProps) => {
   const { createCharacteristic } = useCharacteristics();
   const [name, setName] = useState("");
-  const [icon, setIcon] = useState("⭐");
+  const [icon, setIcon] = useState(CHARACTERISTIC_ICONS[0]);
   const [xp, setXP] = useState("0");
 
   const handleSubmit = (e: React.FormEvent) => {
@@ -57,7 +56,7 @@ const CreateCharacteristicForm = ({ onClose }: CreateCharacteristicFormProps) =>
             Icon
           </label>
           <div className="grid grid-cols-5 gap-2">
-            {iconOptions.map((iconOption) => (
+            {CHARACTERISTIC_ICONS.map((iconOption) => (
               <button
                 key={iconOption}
                 type="button"

src/components/skills/SkillCard.tsx

@@ -4,6 +4,7 @@ import { Button } from "@/components/ui/button";
 import { calculateLevelProgress } from "@/lib/levelCalculation";
 import { Edit2, Trash2, Star, Power, PowerOff } from "lucide-react";
 import EditSkillDialog from "./EditSkillDialog";
+import ConfirmDialog from "./ConfirmDialog";
 
 interface SkillCardProps {
   skill: Skill;
@@ -12,6 +13,7 @@ interface SkillCardProps {
 const SkillCard = ({ skill }: SkillCardProps) => {
   const { updateSkill, deleteSkill } = useSkills();
   const [isEditDialogOpen, setIsEditDialogOpen] = useState(false);
+  const [showDeleteDialog, setShowDeleteDialog] = useState(false);
 
   const progress = calculateLevelProgress(skill.xp);
 
@@ -23,9 +25,12 @@ const SkillCard = ({ skill }: SkillCardProps) => {
   };
 
   const handleDelete = () => {
-    if (window.confirm(`Delete skill "${skill.name}"?`)) {
-      deleteSkill.mutate(skill.id);
-    }
+    setShowDeleteDialog(true);
+  };
+
+  const confirmDelete = () => {
+    deleteSkill.mutate(skill.id);
+    setShowDeleteDialog(false);
   };
 
   return (
@@ -135,7 +140,7 @@ const SkillCard = ({ skill }: SkillCardProps) => {
               onClick={handleToggleActive}
               className={`h-7 px-3 text-xs ${
                 skill.is_active
-                  ? "text-green-600 hover:text-green-700"
+                  ? "text-primary hover:text-primary/80"
                   : "text-muted-foreground hover:text-foreground"
               }`}
             >
@@ -156,6 +161,15 @@ const SkillCard = ({ skill }: SkillCardProps) => {
         open={isEditDialogOpen}
         onOpenChange={setIsEditDialogOpen}
       />
+
+      {/* Delete Confirmation Dialog */}
+      <ConfirmDialog
+        open={showDeleteDialog}
+        onOpenChange={setShowDeleteDialog}
+        title="Delete Skill"
+        description={`Are you sure you want to delete "${skill.name}"? This action cannot be undone.`}
+        onConfirm={confirmDelete}
+      />
     </>
   );
 };

src/lib/skillsConstants.ts

@@ -0,0 +1,34 @@
+/**
+ * Constants for the Skills system
+ */
+
+export const CHARACTERISTIC_ICONS = [
+  "⭐", "💪", "🧠", "❤️", "⚡", 
+  "🎯", "🛡️", "⚔️", "🏃", "🎨"
+] as const;
+
+export const SKILL_AREAS = [
+  "Programming",
+  "Music",
+  "Fitness",
+  "Art",
+  "Languages",
+  "Business",
+  "Science",
+  "Writing",
+] as const;
+
+/**
+ * Level progression formula constants
+ * Level = floor(sqrt(XP / 100)) + 1
+ * 
+ * XP Thresholds:
+ * Level 1: 0-99 XP
+ * Level 2: 100-399 XP
+ * Level 3: 400-899 XP
+ * Level 4: 900-1599 XP
+ * Level 5: 1600-2499 XP
+ * Level 10: 8100-9999 XP
+ * Level 20: 36100-40099 XP
+ */
+export const XP_LEVEL_MULTIPLIER = 100;

supabase/migrations/20260127000000_add_skills_system.sql

@@ -90,7 +90,14 @@ IMMUTABLE
 AS $$
 BEGIN
   -- Level calculation: Level = floor(sqrt(XP / 100)) + 1
-  -- This means: Level 1 = 0-99 XP, Level 2 = 100-399 XP, Level 3 = 400-899 XP, etc.
+  -- XP Thresholds:
+  -- Level 1: 0-99 XP
+  -- Level 2: 100-399 XP
+  -- Level 3: 400-899 XP
+  -- Level 4: 900-1599 XP
+  -- Level 5: 1600-2499 XP
+  -- Level 10: 8100-9999 XP
+  -- Level 20: 36100-40099 XP
   RETURN FLOOR(SQRT(xp_amount / 100.0)) + 1;
 END;
 $$;