feat(skills): add skill deletion and improve UI/UX

- Add skill deletion functionality with confirmation dialog - Protect builtin skills from deletion - Optimize skill card layout (badge and enable button near menu) - Change enable button to icon-only with theme color - Improve card selection and hover states - Fix skill hub installation state tracking (per-skill) - Add proper i18n for delete feature (en/zh) - Add loading states to delete confirmation dialog - Remove manual refetch calls (use query invalidation)
2026-02-28 09:23:08 +08:00 · 2026-02-24 21:06:34 +08:00
parent 6c9ad9a9f3
commit 33e12a31ac
6 changed files with 219 additions and 24 deletions
--- a/ccw/frontend/src/components/shared/SkillCard.tsx
+++ b/ccw/frontend/src/components/shared/SkillCard.tsx
@@ -13,6 +13,7 @@ import {
  Power,
  PowerOff,
  User,
+  Trash2,
 } from 'lucide-react';
 import { cn } from '@/lib/utils';
 import { Card } from '@/components/ui/Card';
@@ -28,6 +29,7 @@ export interface SkillCardProps {
  onToggle?: (skill: Skill, enabled: boolean) => void;
  onClick?: (skill: Skill) => void;
  onConfigure?: (skill: Skill) => void;
+  onDelete?: (skill: Skill) => void;
  className?: string;
  compact?: boolean;
  showActions?: boolean;
@@ -70,6 +72,7 @@ export function SkillCard({
  onToggle,
  onClick,
  onConfigure,
+  onDelete,
  className,
  compact = false,
  showActions = true,
@@ -95,6 +98,12 @@ export function SkillCard({
    onConfigure?.(skill);
  };

+  const handleDelete = (e: React.MouseEvent) => {
+    e.stopPropagation();
+    setIsMenuOpen(false);
+    onDelete?.(skill);
+  };
+
  if (compact) {
    return (
      <div
@@ -172,20 +181,15 @@ export function SkillCard({
          <Button
            variant="ghost"
            size="sm"
-            className={cn(
-              "h-8 w-8 p-0",
-              skill.enabled
-                ? "bg-primary hover:bg-primary/90"
-                : "hover:bg-muted"
-            )}
+            className="h-8 w-8 p-0 hover:bg-primary/10"
            onClick={handleToggle}
            disabled={isToggling}
            title={skill.enabled ? formatMessage({ id: 'skills.state.enabled' }) : formatMessage({ id: 'skills.state.disabled' })}
          >
            {skill.enabled ? (
-              <Power className="w-4 h-4 text-white" />
+              <Power className="w-4 h-4 text-primary" />
            ) : (
-              <PowerOff className="w-4 h-4 text-foreground" />
+              <PowerOff className="w-4 h-4 text-muted-foreground" />
            )}
          </Button>
          {showActions && (
@@ -222,6 +226,12 @@ export function SkillCard({
                    </>
                  )}
                </DropdownMenuItem>
+                {onDelete && skill.source !== 'builtin' && (
+                  <DropdownMenuItem onClick={handleDelete} className="text-destructive focus:text-destructive">
+                    <Trash2 className="w-4 h-4 mr-2" />
+                    {formatMessage({ id: 'skills.actions.delete' })}
+                  </DropdownMenuItem>
+                )}
              </DropdownMenuContent>
            </DropdownMenu>
          )}
--- a/ccw/frontend/src/hooks/useSkills.ts
+++ b/ccw/frontend/src/hooks/useSkills.ts
@@ -215,15 +215,53 @@ export function useToggleSkill(): UseToggleSkillReturn {
  };
 }

+/**
+ * Hook for deleting a skill
+ */
+export function useDeleteSkill() {
+  const queryClient = useQueryClient();
+
+  const mutation = useMutation({
+    mutationFn: async ({
+      skillName,
+      location,
+      projectPath,
+      cliType,
+    }: {
+      skillName: string;
+      location: 'project' | 'user';
+      projectPath?: string;
+      cliType: 'claude' | 'codex';
+    }) => {
+      const { deleteSkill } = await import('@/lib/api');
+      return deleteSkill(skillName, location, projectPath, cliType);
+    },
+    onSuccess: () => {
+      // Invalidate skills queries to refresh the list
+      queryClient.invalidateQueries({ queryKey: skillsKeys.all });
+    },
+  });
+
+  return {
+    deleteSkill: (skillName: string, location: 'project' | 'user', projectPath?: string, cliType: 'claude' | 'codex' = 'claude') =>
+      mutation.mutateAsync({ skillName, location, projectPath, cliType }),
+    isDeleting: mutation.isPending,
+    error: mutation.error,
+  };
+}
+
 /**
 * Combined hook for all skill mutations
 */
 export function useSkillMutations() {
  const toggle = useToggleSkill();
+  const deleteSkillHook = useDeleteSkill();

  return {
    toggleSkill: toggle.toggleSkill,
    isToggling: toggle.isToggling,
-    isMutating: toggle.isToggling,
+    deleteSkill: deleteSkillHook.deleteSkill,
+    isDeleting: deleteSkillHook.isDeleting,
+    isMutating: toggle.isToggling || deleteSkillHook.isDeleting,
  };
 }
--- a/ccw/frontend/src/lib/api.ts
+++ b/ccw/frontend/src/lib/api.ts
@@ -1265,6 +1265,25 @@ export async function fetchSkillDetail(
  return fetchApi<{ skill: Skill }>(url);
 }

+/**
+ * Delete a skill
+ * @param skillName - Name of the skill to delete
+ * @param location - Location of the skill (project or user)
+ * @param projectPath - Optional project path
+ * @param cliType - CLI type (claude or codex)
+ */
+export async function deleteSkill(
+  skillName: string,
+  location: 'project' | 'user',
+  projectPath?: string,
+  cliType: 'claude' | 'codex' = 'claude'
+): Promise<{ success: boolean }> {
+  return fetchApi<{ success: boolean }>(`/api/skills/${encodeURIComponent(skillName)}`, {
+    method: 'DELETE',
+    body: JSON.stringify({ location, projectPath, cliType }),
+  });
+}
+
 /**
 * Validate a skill folder for import
 */
@@ -1298,6 +1317,42 @@ export async function createSkill(params: {
  });
 }

+/**
+ * Read a skill file content
+ */
+export async function readSkillFile(params: {
+  skillName: string;
+  fileName: string;
+  location: 'project' | 'user';
+  projectPath?: string;
+  cliType?: 'claude' | 'codex';
+}): Promise<{ content: string; fileName: string; path: string }> {
+  const { skillName, fileName, location, projectPath, cliType = 'claude' } = params;
+  const encodedSkillName = encodeURIComponent(skillName);
+  const url = `/api/skills/${encodedSkillName}/file?filename=${encodeURIComponent(fileName)}&location=${location}&cliType=${cliType}${projectPath ? `&path=${encodeURIComponent(projectPath)}` : ''}`;
+  return fetchApi(url);
+}
+
+/**
+ * Write a skill file content
+ */
+export async function writeSkillFile(params: {
+  skillName: string;
+  fileName: string;
+  content: string;
+  location: 'project' | 'user';
+  projectPath?: string;
+  cliType?: 'claude' | 'codex';
+}): Promise<{ success: boolean; fileName: string; path: string }> {
+  const { skillName, fileName, content, location, projectPath, cliType = 'claude' } = params;
+  const encodedSkillName = encodeURIComponent(skillName);
+  const url = `/api/skills/${encodedSkillName}/file`;
+  return fetchApi(url, {
+    method: 'POST',
+    body: JSON.stringify({ content, fileName, location, projectPath, cliType }),
+  });
+}
+
 // ========== Commands API ==========

 export interface Command {
--- a/ccw/frontend/src/locales/en/skills.json
+++ b/ccw/frontend/src/locales/en/skills.json
@@ -8,6 +8,14 @@
    "title": "Disable Skill?",
    "message": "Are you sure you want to disable \"{name}\"?"
  },
+  "deleteConfirm": {
+    "title": "Delete Skill?",
+    "message": "Are you sure you want to delete \"{name}\"? This action cannot be undone."
+  },
+  "delete": {
+    "success": "Skill \"{name}\" has been deleted",
+    "error": "Failed to delete skill: {error}"
+  },
  "location": {
    "project": "Project",
    "user": "Global",
@@ -25,8 +33,10 @@
    "disable": "Disable",
    "toggle": "Toggle",
    "install": "Install Skill",
+    "delete": "Delete",
    "cancel": "Cancel",
-    "confirmDisable": "Disable"
+    "confirmDisable": "Disable",
+    "confirmDelete": "Delete"
  },
  "state": {
    "enabled": "Enabled",
--- a/ccw/frontend/src/locales/zh/skills.json
+++ b/ccw/frontend/src/locales/zh/skills.json
@@ -8,6 +8,14 @@
    "title": "禁用技能?",
    "message": "确定要禁用 \"{name}\" 吗?"
  },
+  "deleteConfirm": {
+    "title": "删除技能?",
+    "message": "确定要删除 \"{name}\" 吗? 此操作无法撤销。"
+  },
+  "delete": {
+    "success": "技能 \"{name}\" 已删除",
+    "error": "删除技能失败: {error}"
+  },
  "location": {
    "project": "项目",
    "user": "全局",
@@ -25,8 +33,10 @@
    "disable": "禁用",
    "toggle": "切换",
    "install": "安装技能",
+    "delete": "删除",
    "cancel": "取消",
-    "confirmDisable": "禁用"
+    "confirmDisable": "禁用",
+    "confirmDelete": "删除"
  },
  "state": {
    "enabled": "已启用",
--- a/ccw/frontend/src/pages/SkillsManagerPage.tsx
+++ b/ccw/frontend/src/pages/SkillsManagerPage.tsx
@@ -6,6 +6,7 @@
 import { useState, useMemo, useCallback } from 'react';
 import { useIntl } from 'react-intl';
 import { useSearchParams } from 'react-router-dom';
+import { toast } from 'sonner';
 import {
  Sparkles,
  Search,
@@ -70,11 +71,12 @@ interface SkillGridProps {
  isLoading: boolean;
  onToggle: (skill: Skill, enabled: boolean) => void;
  onClick: (skill: Skill) => void;
+  onDelete?: (skill: Skill) => void;
  isToggling: boolean;
  compact?: boolean;
 }

-function SkillGrid({ skills, isLoading, onToggle, onClick, isToggling, compact }: SkillGridProps) {
+function SkillGrid({ skills, isLoading, onToggle, onClick, onDelete, isToggling, compact }: SkillGridProps) {
  const { formatMessage } = useIntl();

  if (isLoading) {
@@ -113,6 +115,7 @@ function SkillGrid({ skills, isLoading, onToggle, onClick, isToggling, compact }
          skill={skill}
          onToggle={onToggle}
          onClick={onClick}
+          onDelete={onDelete}
          isToggling={isToggling}
          compact={compact}
        />
@@ -140,12 +143,14 @@ export function SkillsManagerPage() {
  const [viewMode, setViewMode] = useState<'grid' | 'compact'>('grid');
  const [showDisabledSection, setShowDisabledSection] = useState(false);
  const [confirmDisable, setConfirmDisable] = useState<{ skill: Skill; enable: boolean } | null>(null);
+  const [confirmDelete, setConfirmDelete] = useState<Skill | null>(null);
  const [locationFilter, setLocationFilter] = useState<'project' | 'user' | 'hub'>(initialLocationFilter);

  // Skill Hub state
  const [hubTab, setHubTab] = useState<'remote' | 'local' | 'installed'>('remote');
  const [hubSearchQuery, setHubSearchQuery] = useState('');
  const [hubCategoryFilter, setHubCategoryFilter] = useState<string | null>(null);
+  const [installingSkillId, setInstallingSkillId] = useState<string | null>(null);

  // Skill create dialog state
  const [isCreateDialogOpen, setIsCreateDialogOpen] = useState(false);
@@ -179,7 +184,7 @@ export function SkillsManagerPage() {
    cliType: cliMode,
  });

-  const { toggleSkill, isToggling } = useSkillMutations();
+  const { toggleSkill, deleteSkill, isToggling, isDeleting } = useSkillMutations();

  // Skill Hub hooks
  const {
@@ -245,21 +250,40 @@ export function SkillsManagerPage() {
  // Hub skill handlers
  const handleHubInstall = async (skill: RemoteSkill | LocalSkill, cliType: CliType) => {
    const source: SkillSource = 'downloadUrl' in skill ? 'remote' : 'local';
-    await installSkillMutation.mutateAsync({
-      skillId: skill.id,
-      cliType,
-      source,
-      downloadUrl: 'downloadUrl' in skill ? skill.downloadUrl : undefined,
-    });
+    setInstallingSkillId(skill.id);
+    try {
+      await installSkillMutation.mutateAsync({
+        skillId: skill.id,
+        cliType,
+        source,
+        downloadUrl: 'downloadUrl' in skill ? skill.downloadUrl : undefined,
+      });
+      // Show success toast
+      toast.success(formatMessage({ id: 'skill-hub.install.success' }, { name: skill.name }));
+    } catch (error) {
+      // Show error toast
+      const errorMessage = error instanceof Error ? error.message : String(error);
+      toast.error(formatMessage({ id: 'skill-hub.install.error' }, { error: errorMessage }));
+    } finally {
+      setInstallingSkillId(null);
+    }
  };

  const handleHubUninstall = async (skill: RemoteSkill | LocalSkill, cliType: CliType) => {
    const installedInfo = installedMap.get(skill.id);
    if (installedInfo) {
-      await uninstallSkillMutation.mutateAsync({
-        skillId: installedInfo.id,
-        cliType,
-      });
+      try {
+        await uninstallSkillMutation.mutateAsync({
+          skillId: installedInfo.id,
+          cliType,
+        });
+        // Show success toast
+        toast.success(formatMessage({ id: 'skill-hub.uninstall.success' }, { name: skill.name }));
+      } catch (error) {
+        // Show error toast
+        const errorMessage = error instanceof Error ? error.message : String(error);
+        toast.error(formatMessage({ id: 'skill-hub.uninstall.error' }, { error: errorMessage }));
+      }
    }
  };

@@ -317,6 +341,27 @@ export function SkillsManagerPage() {
    }
  };

+  const handleDelete = (skill: Skill) => {
+    setConfirmDelete(skill);
+  };
+
+  const handleConfirmDelete = async () => {
+    if (confirmDelete) {
+      const location = confirmDelete.location || 'project';
+      const skillIdentifier = confirmDelete.folderName || confirmDelete.name;
+
+      try {
+        await deleteSkill(skillIdentifier, location, projectPath, cliMode);
+        toast.success(formatMessage({ id: 'skills.delete.success' }, { name: confirmDelete.name }));
+      } catch (error) {
+        const errorMessage = error instanceof Error ? error.message : String(error);
+        toast.error(formatMessage({ id: 'skills.delete.error' }, { error: errorMessage }));
+      } finally {
+        setConfirmDelete(null);
+      }
+    }
+  };
+
  // Skill detail panel handlers
  const handleSkillClick = useCallback(async (skill: Skill) => {
    setIsDetailLoading(true);
@@ -610,7 +655,7 @@ export function SkillsManagerPage() {
                    source={skillSource}
                    onInstall={handleHubInstall}
                    onUninstall={handleHubUninstall}
-                    isInstalling={installSkillMutation.isPending}
+                    isInstalling={installingSkillId === skill.id}
                  />
                );
              })}
@@ -712,6 +757,7 @@ export function SkillsManagerPage() {
            isLoading={isLoading}
            onToggle={handleToggleWithConfirm}
            onClick={handleSkillClick}
+            onDelete={handleDelete}
            isToggling={isToggling || !!confirmDisable}
            compact={viewMode === 'compact'}
          />
@@ -735,6 +781,7 @@ export function SkillsManagerPage() {
                  isLoading={false}
                  onToggle={handleToggleWithConfirm}
                  onClick={handleSkillClick}
+                  onDelete={handleDelete}
                  isToggling={isToggling || !!confirmDisable}
                  compact={true}
                />
@@ -765,6 +812,31 @@ export function SkillsManagerPage() {
        </AlertDialogContent>
      </AlertDialog>

+      {/* Delete Confirmation Dialog */}
+      <AlertDialog open={!!confirmDelete} onOpenChange={(open) => !open && setConfirmDelete(null)}>
+        <AlertDialogContent>
+          <AlertDialogHeader>
+            <AlertDialogTitle>{formatMessage({ id: 'skills.deleteConfirm.title' })}</AlertDialogTitle>
+            <AlertDialogDescription>
+              {formatMessage(
+                { id: 'skills.deleteConfirm.message' },
+                { name: confirmDelete?.name || '' }
+              )}
+            </AlertDialogDescription>
+          </AlertDialogHeader>
+          <AlertDialogFooter>
+            <AlertDialogCancel disabled={isDeleting}>{formatMessage({ id: 'skills.actions.cancel' })}</AlertDialogCancel>
+            <AlertDialogAction
+              onClick={handleConfirmDelete}
+              disabled={isDeleting}
+              className="bg-destructive text-destructive-foreground hover:bg-destructive/90"
+            >
+              {isDeleting ? formatMessage({ id: 'common.deleting' }) : formatMessage({ id: 'skills.actions.confirmDelete' })}
+            </AlertDialogAction>
+          </AlertDialogFooter>
+        </AlertDialogContent>
+      </AlertDialog>
+
      {/* Skill Detail Panel */}
      <SkillDetailPanel
        skill={selectedSkill}