fix(skills): select files on click, add Edit to the row menu (MUL-5654)

packages/views/locales/en/skills.json

@@ -206,6 +206,7 @@
       "add_file": "New file",
       "mode_aria": "File view",
       "mode_preview": "Preview",
+      "mode_edit": "Edit",
       "mode_raw": "Plain text"
     }
   },
@@ -328,6 +329,7 @@
     "no_files": "No files",
     "actions": {
       "label": "Actions for {{path}}",
+      "edit": "Edit",
       "rename": "Rename",
       "delete": "Delete"
     }

packages/views/locales/ja/skills.json

@@ -194,6 +194,7 @@
       "add_file": "新規ファイル",
       "mode_aria": "ファイル表示",
       "mode_preview": "プレビュー",
+      "mode_edit": "編集",
       "mode_raw": "プレーンテキスト"
     }
   },
@@ -315,6 +316,7 @@
     "no_files": "ファイルなし",
     "actions": {
       "label": "{{path}} の操作",
+      "edit": "編集",
       "rename": "名前を変更",
       "delete": "削除"
     }

packages/views/locales/ko/skills.json

@@ -194,6 +194,7 @@
       "add_file": "새 파일",
       "mode_aria": "파일 보기",
       "mode_preview": "미리보기",
+      "mode_edit": "편집",
       "mode_raw": "일반 텍스트"
     }
   },
@@ -315,6 +316,7 @@
     "no_files": "파일 없음",
     "actions": {
       "label": "{{path}} 작업",
+      "edit": "편집",
       "rename": "이름 변경",
       "delete": "삭제"
     }

packages/views/locales/zh-Hans/skills.json

@@ -194,6 +194,7 @@
       "add_file": "新建文件",
       "mode_aria": "文件视图",
       "mode_preview": "预览",
+      "mode_edit": "编辑",
       "mode_raw": "纯文本"
     }
   },
@@ -315,6 +316,7 @@
     "no_files": "无文件",
     "actions": {
       "label": "{{path}} 的操作",
+      "edit": "编辑",
       "rename": "重命名",
       "delete": "删除"
     }

packages/views/skills/components/file-tree.test.tsx

@@ -13,6 +13,7 @@ const PATHS = ["SKILL.md", "references/api.md", "notes.txt"];
 
 function makeActions(overrides: Partial<FileTreeActions> = {}): FileTreeActions {
   return {
+    onEdit: vi.fn(),
     validatePath: () => "",
     onRename: vi.fn(),
     onDelete: vi.fn(),
@@ -30,7 +31,7 @@ describe("FileTree row actions", () => {
     expect(screen.queryByRole("button", { name: /notes\.txt/ })).toBeNull();
   });
 
-  it("withholds them from the reserved file, which owns the skill's content", async () => {
+  it("withholds rename and delete from the reserved file, but not edit", async () => {
     await renderWithI18n(
       <FileTree
         filePaths={PATHS}
@@ -40,11 +41,57 @@ describe("FileTree row actions", () => {
       />,
     );
 
+    await userEvent.click(screen.getByRole("button", { name: /SKILL\.md/ }));
+
     // Deleting or renaming SKILL.md is refused by the server, so the row must
-    // not present it as available.
-    const rows = screen.getAllByRole("tab");
-    const skillMd = rows.find((row) => row.textContent === "SKILL.md")!;
-    expect(skillMd.parentElement!.querySelector("[aria-label]")).toBeNull();
+    // not present either as available. Editing it is ordinary, so the row
+    // keeps that one rather than losing the menu wholesale.
+    expect(
+      await screen.findByRole("menuitem", { name: /编辑|Edit/ }),
+    ).toBeInTheDocument();
+    expect(screen.queryByRole("menuitem", { name: /重命名|Rename/ })).toBeNull();
+    expect(screen.queryByRole("menuitem", { name: /删除|Delete/ })).toBeNull();
+  });
+
+  it("edits the row acted on", async () => {
+    const onEdit = vi.fn();
+    await renderWithI18n(
+      <FileTree
+        filePaths={PATHS}
+        selectedPath="SKILL.md"
+        onSelect={vi.fn()}
+        actions={makeActions({ onEdit })}
+      />,
+    );
+
+    await userEvent.click(screen.getByRole("button", { name: /notes\.txt/ }));
+    await userEvent.click(
+      await screen.findByRole("menuitem", { name: /编辑|Edit/ }),
+    );
+
+    expect(onEdit).toHaveBeenCalledWith("notes.txt");
+  });
+
+  it("keeps the menu trigger's widened hit area inside the trigger", async () => {
+    await renderWithI18n(
+      <FileTree
+        filePaths={PATHS}
+        selectedPath="SKILL.md"
+        onSelect={vi.fn()}
+        actions={makeActions()}
+      />,
+    );
+
+    // MUL-5654: the trigger widens its hit area with an absolutely positioned
+    // `after:-inset-1`. Drop `relative` and that pseudo-element resolves
+    // against the row instead, covering it end to end and swallowing every
+    // click on the file name — the row stops selecting and opens this menu.
+    // jsdom has no layout or hit testing, so the class pairing is the part a
+    // unit test can hold; the behaviour itself needs a real browser.
+    // The name button carries role="tab", so a "button" match is the trigger.
+    const trigger = screen.getByRole("button", { name: /notes\.txt/ });
+    expect(trigger.className).toContain("after:absolute");
+    expect(trigger.className).toContain("relative");
   });
 
   it("deletes the row acted on, not whichever file happens to be open", async () => {

packages/views/skills/components/file-tree.tsx

@@ -10,6 +10,7 @@ import {
   FolderOpen,
   MoreHorizontal,
   Pencil,
+  PencilLine,
   Trash2,
 } from "lucide-react";
 import { Input } from "@multica/ui/components/ui/input";
@@ -23,11 +24,16 @@ import { cn } from "@multica/ui/lib/utils";
 import { useT } from "../../i18n";
 
 /**
- * What a row may offer beyond selection. Absent for read-only viewers and for
- * the reserved primary file, so the tree never shows an action it would then
- * have to refuse.
+ * What a row may offer beyond selection. Absent for read-only viewers, so the
+ * tree never shows an action it would then have to refuse.
+ *
+ * `reservedPath` narrows the menu rather than removing it: the primary file
+ * cannot be renamed or deleted, but it is edited like any other row, so it
+ * keeps the edit entry and loses the other two.
  */
 export interface FileTreeActions {
+  /** Open the row's file in the editor, caret already in it. */
+  onEdit: (path: string) => void;
   /** Returns an error message, or "" when the path is free to use. */
   validatePath: (path: string, existing: string[]) => string;
   onRename: (from: string, to: string) => void;
@@ -159,7 +165,11 @@ function TreeNodeItem({
   }
 
   const Icon = getFileIcon(node.name);
-  const editable = !!actions && node.path !== actions.reservedPath;
+  // Two different questions: whether the row has a menu at all (any row does,
+  // once the viewer may edit), and whether that menu may offer rename/delete
+  // (the reserved primary file may not).
+  const hasMenu = !!actions;
+  const canModify = !!actions && node.path !== actions.reservedPath;
 
   if (renaming && actions) {
     return (
@@ -185,7 +195,7 @@ function TreeNodeItem({
       // anchors to that button rather than the cursor, which keeps one menu
       // per row instead of a context-menu root beside a dropdown root.
       onContextMenu={
-        editable
+        hasMenu
           ? (event) => {
               event.preventDefault();
               menuButtonRef.current?.click();
@@ -209,7 +219,7 @@ function TreeNodeItem({
         <Icon className="h-3.5 w-3.5 shrink-0" />
         <span className="truncate">{node.name}</span>
       </button>
-      {editable && (
+      {actions && (
         <DropdownMenu>
           <DropdownMenuTrigger
             render={
@@ -222,25 +232,43 @@ function TreeNodeItem({
                 // Hidden until the row is hovered or something inside it holds
                 // focus, so a rail of ten files is not a rail of ten buttons.
                 // after:-inset-1 widens the hit area past the 20px glyph.
-                className="mr-1 shrink-0 rounded p-0.5 text-faint-foreground opacity-0 transition-opacity after:absolute after:-inset-1 hover:text-foreground group-hover/row:opacity-100 focus-visible:opacity-100 aria-expanded:opacity-100"
+                //
+                // `relative` is load-bearing, not decoration: it makes THIS
+                // button the containing block for that ::after. Without it the
+                // pseudo-element resolves against the row (the nearest
+                // positioned ancestor), so the invisible hit area covered the
+                // whole row and — being absolutely positioned, hence painted
+                // above the in-flow name button — swallowed every click on the
+                // file name. Selecting a supporting file opened this menu
+                // instead (MUL-5654). opacity-0 does not opt out of hit
+                // testing, so the row was unclickable even before hover.
+                className="relative mr-1 shrink-0 rounded p-0.5 text-faint-foreground opacity-0 transition-opacity after:absolute after:-inset-1 hover:text-foreground group-hover/row:opacity-100 focus-visible:opacity-100 aria-expanded:opacity-100"
                 onClick={(event) => event.stopPropagation()}
               >
                 <MoreHorizontal className="h-3.5 w-3.5" />
               </button>
             }
           />
           <DropdownMenuContent align="end" className="w-36">
-            <DropdownMenuItem onClick={() => setRenaming(true)}>
-              <Pencil />
-              {t(($) => $.file_tree.actions.rename)}
-            </DropdownMenuItem>
-            <DropdownMenuItem
-              variant="destructive"
-              onClick={() => actions.onDelete(node.path)}
-            >
-              <Trash2 />
-              {t(($) => $.file_tree.actions.delete)}
+            <DropdownMenuItem onClick={() => actions.onEdit(node.path)}>
+              <PencilLine />
+              {t(($) => $.file_tree.actions.edit)}
             </DropdownMenuItem>
+            {canModify && (
+              <>
+                <DropdownMenuItem onClick={() => setRenaming(true)}>
+                  <Pencil />
+                  {t(($) => $.file_tree.actions.rename)}
+                </DropdownMenuItem>
+                <DropdownMenuItem
+                  variant="destructive"
+                  onClick={() => actions.onDelete(node.path)}
+                >
+                  <Trash2 />
+                  {t(($) => $.file_tree.actions.delete)}
+                </DropdownMenuItem>
+              </>
+            )}
           </DropdownMenuContent>
         </DropdownMenu>
       )}

packages/views/skills/components/file-viewer.tsx

@@ -1,6 +1,6 @@
 "use client";
 
-import { useMemo } from "react";
+import { useEffect, useMemo, useRef } from "react";
 import { Textarea } from "@multica/ui/components/ui/textarea";
 import { parseFrontmatter } from "@multica/core/skills/frontmatter";
 import { RichContent } from "../../rich-content";
@@ -30,22 +30,45 @@ export function FileViewer({
   content,
   mode,
   readOnly,
+  autoFocus,
   onChange,
+  onFocusHandled,
 }: {
   path: string;
   content: string;
   mode: FileMode;
   readOnly: boolean;
+  /** Set by "Edit": put the caret in this file's editor once it renders. */
+  autoFocus?: boolean;
   onChange: (content: string) => void;
+  /** Called once the caret has landed, so the request is not replayed. */
+  onFocusHandled?: () => void;
 }) {
   const { t } = useT("skills");
   const isMd = isMarkdownPath(path);
+  const editorRef = useRef<HTMLTextAreaElement>(null);
 
   const body = useMemo(
     () => (isMd ? parseFrontmatter(content).body : content),
     [content, isMd],
   );
 
+  // The caller flips the mode to raw in the same update that raises this flag,
+  // so by the time the effect runs the textarea below is mounted. Focusing
+  // through a request the page owns — rather than `autoFocus` on the element —
+  // is what makes "Edit" work on the file that is ALREADY open: that path
+  // changes no key, mounts nothing, and would never re-run a mount-only focus.
+  useEffect(() => {
+    if (autoFocus !== true) return;
+    const el = editorRef.current;
+    if (!el) return;
+    el.focus();
+    // Caret at the top. Jumping to the end of a multi-thousand-character
+    // reference file would scroll away the content the user just clicked.
+    el.setSelectionRange(0, 0);
+    onFocusHandled?.();
+  }, [autoFocus, onFocusHandled]);
+
   // Non-markdown files have nothing to preview; they are always raw.
   if (isMd && mode === "preview") {
     return (
@@ -64,6 +87,7 @@ export function FileViewer({
 
   return (
     <Textarea
+      ref={editorRef}
       value={content}
       readOnly={readOnly}
       onChange={(e) => onChange(e.target.value)}