From adab1399d4e48a0e2600b38a6bb1c1045df4fc30 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:15:46 +0800 Subject: [PATCH] fix(skills): select files on click, add Edit to the row menu (MUL-5654) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The row's action-menu trigger widens its hit area with `after:absolute after:-inset-1` but was not itself positioned, so that pseudo-element resolved against the row — the nearest positioned ancestor — and covered it end to end. Being absolutely positioned it painted above the in-flow name button, so every click on a supporting file's name opened the menu instead of selecting the file, and `opacity-0` never opted the overlay out of hit testing. Only SKILL.md, rendered without actions, stayed clickable, which read as "only SKILL.md can be edited". `relative` puts the ::after back inside the trigger. Alongside the fix, so the editor is reachable in one gesture: - the raw segment reads "Edit" for viewers who may edit, and stays "Plain text" for read-only viewers rather than offering an edit the page refuses; - the row menu gains an Edit entry that opens the file, switches the pane to the editor and puts the caret at its top — including for the file that is already open, which mounts nothing and so cannot rely on mount-time focus; - SKILL.md keeps that entry and still withholds rename/delete. Verified in Chromium against the compiled classes: clicking a file name or empty row space now hits the name button, while the glyph and 4px around it still hit the menu. Co-authored-by: multica-agent --- packages/views/locales/en/skills.json | 2 + packages/views/locales/ja/skills.json | 2 + packages/views/locales/ko/skills.json | 2 + packages/views/locales/zh-Hans/skills.json | 2 + .../skills/components/file-tree.test.tsx | 57 ++++++++++++++-- .../views/skills/components/file-tree.tsx | 62 ++++++++++++----- .../views/skills/components/file-viewer.tsx | 26 ++++++- .../components/skill-detail-page.test.tsx | 68 ++++++++++++++++++- .../skills/components/skill-detail-page.tsx | 45 +++++++++++- 9 files changed, 239 insertions(+), 27 deletions(-) diff --git a/packages/views/locales/en/skills.json b/packages/views/locales/en/skills.json index a972dce0c13..65972377f4a 100644 --- a/packages/views/locales/en/skills.json +++ b/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" } diff --git a/packages/views/locales/ja/skills.json b/packages/views/locales/ja/skills.json index 5e8d0e66fc8..4206a8ae8c2 100644 --- a/packages/views/locales/ja/skills.json +++ b/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": "削除" } diff --git a/packages/views/locales/ko/skills.json b/packages/views/locales/ko/skills.json index da238c07382..e6f0ebdd4cf 100644 --- a/packages/views/locales/ko/skills.json +++ b/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": "삭제" } diff --git a/packages/views/locales/zh-Hans/skills.json b/packages/views/locales/zh-Hans/skills.json index 1b0db5f8102..c4cbed3b5fe 100644 --- a/packages/views/locales/zh-Hans/skills.json +++ b/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": "删除" } diff --git a/packages/views/skills/components/file-tree.test.tsx b/packages/views/skills/components/file-tree.test.tsx index 02b17bb8a22..3ff6a6dc617 100644 --- a/packages/views/skills/components/file-tree.test.tsx +++ b/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 { 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( { />, ); + 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( + , + ); + + 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( + , + ); + + // 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 () => { diff --git a/packages/views/skills/components/file-tree.tsx b/packages/views/skills/components/file-tree.tsx index 11987b78d43..839360f02c3 100644 --- a/packages/views/skills/components/file-tree.tsx +++ b/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({ {node.name} - {editable && ( + {actions && ( )} diff --git a/packages/views/skills/components/file-viewer.tsx b/packages/views/skills/components/file-viewer.tsx index 09f070fa026..92d0e468fff 100644 --- a/packages/views/skills/components/file-viewer.tsx +++ b/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(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 (