Feat/file tree row menu - #648
Merged
Merged
Conversation
The three ContextMenuTrigger wrappers around FileTreeFile, FileTreeFolder, and the RootDropFolder root-drop menu rendered as bare <span> elements. HTML parser rules disallow <span> from containing <div> children, so the row div ended up as the span's sibling and Radix's pointerdown / contextmenu handlers — and its WebkitTouchCallout: none style — never reached the actual row. Effect on touch: * iOS Safari showed its native long-press callout (Copy / Share / Save Image), eating the gesture before Radix's 700ms long-press timer could open the menu. * Android Chrome and desktop touchscreens likewise had no path to open the per-row menu. Desktop right-click worked because onContextMenu propagates by capture and bubble independently of the parent-child DOM relationship. Fix: pass asChild to each of the three ContextMenuTrigger instances so Radix's Slot mechanism merges the trigger props onto the row's own div. This matches every other ContextMenuTrigger usage in the codebase (tabs/tab-item, automations-page, settings/skills-settings, settings/mcp-settings, conversations/sidebar-conversation-list, etc.).
Bring the mobile virtual key bar for the terminal into main.
Touch devices don't get a free way to open the file-tree context menu now that the long-press gesture is reserved for drag. Add a small horizontal three-dots button (MoreHorizontal) on the right of every file, folder, and workspace-root row. Clicking it dispatches a synthetic contextmenu MouseEvent on the row so Radix's existing ContextMenuTrigger opens the same menu the right-click and long-press paths open — no menu items duplicated. - FileTreeFile / FileTreeFolder gain an optional actions?: ReactNode prop rendered inside the existing FileTreeActions wrapper (which already stops click bubbling). - RowMoreButton (new component) walks up to the nearest [data-tree-row-path] ancestor and dispatches the contextmenu event. - Folder.fileTreeTab.moreActions added to all 10 locales. - useLongPressToOpenMenu is kept (and its tests restored) for reuse elsewhere; the file tree now relies on the more button + native right-click instead of the long-press hook.
Four defects in the row-menu button, three of them regressions the PR introduces. The workspace-root row lost its context menu entirely. `asChild` was added to a `ContextMenuTrigger` whose child is a `DesktopDropDirContext. Provider`: Radix's Slot clones the child ELEMENT and hands it the trigger's props, and a Context.Provider drops every prop it doesn't know — so the trigger rendered NO element at all, and right-click, long-press and the new ⋯ all did nothing on the root row. `RootDropFolder` swallowed its props too. The provider now sits outside the menu and `RootDropFolder` spreads `...props` onto the row. The folder ⋯ was a `<button>` inside the header `<button>`. HTML forbids that and React reports it as a hydration error on every render; the header's accessible name also absorbed the action's label, so the row announced as "src More actions". The action is now a sibling overlaid on the header, inside a wrapper that owns the row highlight — hovering a sibling would otherwise switch the tint off, since :hover only propagates to ancestors. The header keeps spanning the whole row so the folder's drop area and its `data-tree-drop-dir` hit-test marker are unchanged, and a reserved right padding keeps the longest name out from under the ⋯. The aux tree is a roving-focus widget: the container is its single tab stop and owns the arrow keys. A native button per row put every row back in the tab sequence, and `FileTreeActions` swallows keydown, so the arrow keys died wherever focus landed. The button now reads the tree's mode and drops out of the tab order there. The menu was anchored at the click's clientX/clientY, which a keyboard or programmatic activation reports as 0 — parking the menu in the viewport's top-left corner. It now anchors at the button's own box, like the sidebar conversation row's ⋯ that this pattern comes from. Also adopted from that prior art: hidden at rest on pointer devices (right-click is the primary affordance and one ⋯ per row is a lot of ink), pinned visible where there is no hover, `aria-haspopup` and a title. Dropped the unused `i18nNamespace` prop, whose other allowed value named a key that does not exist. The tests now fire at a real Radix trigger around a real row instead of a bare div — the old ones could not tell a working button from one whose trigger never reached the DOM.
…ested long-press Two defects a Codex review found in the previous commit and in PR xintaofei#648. The folder's drop zone stopped at the action. Once the ⋯ became a sibling of the header, `data-tree-drop-dir` and the drag handlers were still on the header, and `resolveFileTreeDropZone` walks UP from whatever the pointer hit — so dragging an entry over the ⋯ strip of a destination folder found no zone on desktop, and on the web the dragover/drop handlers never fired there either. The move just silently did nothing. When a row has actions the wrapper IS the row now: it carries the drop marker, `rowProps`, and the highlight, and the header keeps only what belongs to the header. That also lets the drag source's `opacity-70` dim the action along with the rest of the row. `rowProps` is typed against HTMLElement because the row element is a button or a div depending on whether the row has actions. Long-press on a nested row opened the wrong menu. A folder's trigger encloses its expanded descendants' triggers, and PR xintaofei#648 spread `useLongPressToOpenMenu` onto every one of them — so a single touch pointerdown armed a timer per ancestor, and each fired its own contextmenu from its OWN element. Long-pressing a file two levels deep opened three menus, the outermost winning the screen. Radix's built-in long-press survives nesting because all the triggers share one bubbling event that the innermost preventDefaults; separate dispatches carry no such interlock. The hook is unwired — which is what the PR description said the tree does anyway — leaving the ⋯ button as the touch entry point and Radix's own long-press underneath it. The hook and its tests stay, now carrying the nesting warning. Also corrected the asChild rationale in the two row comments: React builds this DOM with appendChild, so the old wrapper span really did contain the row div — the reason to merge onto the row is the inline span's effect on the row's w-max/min-w-full sizing, not a parser split. Geometry verified against the built CSS in headless Chrome: the drop marker resolves from the ⋯ for both folder rows and spans the full 525.4px row; the ⋯ aligns at the same x on file and folder rows and never overlaps a name, including the longest one.
… its action Moving `rowProps` to the wrapper also took `min-w-full` off the header button, so an action-bearing folder row shrank its trigger to the label: clicking anywhere between the name and the ⋯ hit the wrapper, which owns no click handler and cannot bubble into its sibling button, and the folder neither selected nor expanded. `grow` restores the full-width target. Not `flex-1` — its zero basis would drop the label out of the row's intrinsic width, and this tree sizes itself to its longest row and scrolls horizontally. Measured in a browser against the built CSS: the header is 493px of a 525px row (the remaining 32px is the action strip), and the tree's total content width is unchanged at 537px.
Owner
|
codeg work task |
Clears the PR conflict GitHub reported. main has since taken PR xintaofei#647, so the terminal key bar this branch was carrying its own copy of arrives from main instead and drops out of the PR diff entirely — what is left is just the file-tree row menu. Nothing needed a hand-resolution: git merges all 67 files cleanly (the row-menu additions to aux-panel-file-tree-tab-source.test.ts sit beside the markdown-preview assertions main moved into their own describe). Merged tree verified: eslint 0, tsc 0, 409 test files / 5892 tests, next build clean. src-tauri is byte-identical to main.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
文件树在桌面端可以右键打开菜单,触屏端之前依赖长按。长按接下来要留给拖拽用,所以触屏端需要另一条入口。
改动
每个文件/文件夹/工作区根目录行的右侧加一个横向三点的
RowMoreButton。点击时阻止冒泡,向最近的[data-tree-row-path]祖先派发一个合成的contextmenuMouseEvent。Radix 已有的ContextMenuTrigger(asChild包裹整行)收到这个事件就打开和右键、长按完全同一个菜单——零菜单项重复。FileTreeFile/FileTreeFolder新增actions?: ReactNode,渲染在已有的FileTreeActions包裹里(自动stopPropagation)。RowMoreButton(新组件 + 3 个单测)处理按钮的渲染和派发逻辑。Folder.fileTreeTab.moreActions文案 10 个语言全量新增。useLongPressToOpenMenu保留(测试一起恢复)备用,文件树不再依赖它。验证
eslint .→ exit 0vitest run→ 5786 个测试通过(406 个文件;比之前 +13:恢复的 10 个 hook 测试 + 新增的 3 个RowMoreButton测试)next build→ 成功