diff --git a/CHANGELOG.md b/CHANGELOG.md index 81a5726..96ee8ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ Leafdown uses lightweight [Keep a Changelog](https://keepachangelog.com/en/1.1.0 ### Fixed +- Open the editor context popup with `Shift+F10` or the `Menu` key and operate every command in it from the keyboard. +- Announce the editor context popup as a named toolbar instead of an unnamed dialog. - Announce recent files and recent folders under their own headings in the `Open recent` menu. - Disable a submenu instead of opening it empty when every command inside it is unavailable. - Keep the window controls out of the keyboard tab order, matching native title bar buttons. diff --git a/docs/architecture.md b/docs/architecture.md index 5e65f58..e06a60c 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -72,6 +72,8 @@ Leafdown's editor integration uses Milkdown Kit directly through a Leafdown-owne Shortcut execution follows the layer that owns the interaction. The window-level application listener routes only application command IDs and reserved webview suppression. Leafdown's editor keymap routes semantic editor commands and projection-aware history while the editor has focus. Milkdown, ProseMirror, and the browser retain structural editing and native clipboard gesture ownership. The shared command metadata describes labels and displayed shortcuts across these surfaces; it is not itself a global executable shortcut registry. +Focus ownership follows the same layering. A pointer-opened context popup leaves focus with the editor, which still owns the selection the popup acts on; a keyboard-opened popup takes focus, having no other route in, and returns it to the editor on close rather than leaving it on the document body. ProseMirror keeps its selection across a blur, so restoring the editor's focus restores the selection with it, and the popup holds no selection state of its own. + Syntax highlighting uses bundled Shiki assets through Milkdown highlighting plugins. Raw Markdown HTML is preserved as text-like editor content instead of being rendered as browser DOM. ### Clipboard Ownership diff --git a/docs/reference.md b/docs/reference.md index 95db8ba..dd5cc8e 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -235,11 +235,11 @@ For diagnostic log format and ownership, see [Architecture](./architecture.md#ba ### Context Popup -The context popup is a contextual menu triggered by selection or right-click within the editor. +The context popup is a contextual menu triggered by selection, right-click, or `Shift+F10` and the `Menu` key within the editor. - Right-click inside an existing selection keeps the selection. - Right-click outside a selection uses the editor's normal pointer handling to place the caret at the clicked location; the popup does not perform a second coordinate-based caret move. -- `Escape`, typing, clicking outside, or scrolling the popup out of view closes it. +- `Escape`, typing, or clicking outside closes it, as does `Tab` while focus is inside it. Scrolling the popup out of view closes it only while focus is in the editor; a popup holding focus stays open. #### Popup Command Groups diff --git a/docs/specification.md b/docs/specification.md index ff75050..1ec49b3 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -95,6 +95,7 @@ These state axes compose. A document session, for example, can have a folder con - **Closed:** no popup is visible. - **Open from selection:** commands act on the selected text or blocks. - **Open from right-click:** commands act on the editor selection established by the right-click. +- **Open from keyboard:** commands act on the caret or selection the request was made from, and the popup holds focus. ## Editor Model @@ -151,6 +152,16 @@ The editor is a unified hybrid Markdown surface. Behavior is governed by renderi - `Shift+Tab`: Moves focus to the cell to the left. - `Enter`: Moves focus to the cell directly below. If pressed in the bottom row, inserts a new row below and focuses it. - `ArrowDown` (in the bottom row of a table): Exits the table downwards and moves the caret to the block below (creating a new empty paragraph block if none exists). +- `Shift+F10` and the `Menu` key open the context popup around the caret or selection and move focus into it. A popup opened by right-click or by a mouse selection leaves focus in the editor. +- The popup is one command toolbar, and focus enters it on its first available command: + - `ArrowLeft` and `ArrowRight`: Move between commands in order, wrapping at either end. + - `ArrowUp` and `ArrowDown`: Move between rows at the nearest available column, wrapping at either end and skipping a row whose commands are all unavailable. On a submenu, `ArrowDown` opens it instead. + - `Home` and `End`: Move to the first or last available command. + - `Enter` and `Space`: Run the focused command, or open the focused submenu. + - `Escape`: Closes the popup, or an open submenu first, returning focus to the command that opened it. + - `Tab`: Closes the popup as well, rather than moving to another control. +- Closing a popup that holds focus returns focus to the editor with its selection intact, whichever path closed it. +- A scroll closes the popup while focus is in the editor. While focus is inside it the popup stays open and may drift from the text it anchors to. - Structural editing and native text gestures retain their normal editor behavior. Leafdown commands provide the same semantic operations across menus, keyboard shortcuts, and the context popup. - The app intercepts and disables default webview reload and navigation shortcuts, including `Mod+R` and `Mod+Shift+R`, to prevent accidental state resets. diff --git a/src/components/ui/Toolbar.tsx b/src/components/ui/Toolbar.tsx new file mode 100644 index 0000000..8046676 --- /dev/null +++ b/src/components/ui/Toolbar.tsx @@ -0,0 +1,12 @@ +import { Toolbar as ToolbarPrimitive } from "radix-ui"; +import type { ComponentProps } from "react"; + +function Toolbar({ ...props }: ComponentProps) { + return ; +} + +function ToolbarButton({ ...props }: ComponentProps) { + return ; +} + +export { Toolbar, ToolbarButton }; diff --git a/src/features/editor/components/EditorContextPopup.test.tsx b/src/features/editor/components/EditorContextPopup.test.tsx index 70eeafb..7576d77 100644 --- a/src/features/editor/components/EditorContextPopup.test.tsx +++ b/src/features/editor/components/EditorContextPopup.test.tsx @@ -1,10 +1,41 @@ +import { useState } from "react"; import { describe, expect, it, vi } from "vitest"; import { createActiveEditorCommandState, createEditorCommandState } from "@/test/factories/editor"; -import { render, renderWithUser, screen } from "@/test/utils/react"; +import { dispatchDOMEvent } from "@/test/utils/events"; +import { render, renderWithUser, screen, waitFor } from "@/test/utils/react"; +import type { ContextPopupRequest } from "../plugins/contextPopup"; import { EditorContextPopup } from "./EditorContextPopup"; +const noop = () => {}; + +const ANCHOR = { x: 40, top: 60, bottom: 80 }; +const POINTER_REQUEST: ContextPopupRequest = { anchor: ANCHOR, source: "pointer" }; +const KEYBOARD_REQUEST: ContextPopupRequest = { anchor: ANCHOR, source: "keyboard" }; + +interface ClosingPopupHostProps { + onReturnFocus?: () => void; +} + +// A mock `onClose` leaves the popup mounted, so close paths asserted against one pass either way. +function ClosingPopupHost({ onReturnFocus = noop }: ClosingPopupHostProps) { + const [request, setRequest] = useState(KEYBOARD_REQUEST); + + return ( + <> + + setRequest(null)} + onExecute={noop} + onReturnFocus={onReturnFocus} + /> + + ); +} + const enabledPopupCommandState = createActiveEditorCommandState({ enabledCommandIds: [ "edit.cut", @@ -29,10 +60,11 @@ describe("EditorContextPopup", () => { it("uses the selection range as the collision-aware popup anchor", () => { render( , ); @@ -44,10 +76,11 @@ describe("EditorContextPopup", () => { it("renders the initial five-row context UI", () => { render( , ); @@ -77,7 +110,7 @@ describe("EditorContextPopup", () => { const { user } = renderWithUser( { }} onClose={vi.fn()} onExecute={onExecute} + onReturnFocus={vi.fn()} />, ); @@ -101,12 +135,13 @@ describe("EditorContextPopup", () => { const { user } = renderWithUser( , ); @@ -115,4 +150,435 @@ describe("EditorContextPopup", () => { expect(onExecute).not.toHaveBeenCalled(); }); + + describe("focus ownership", () => { + it("moves focus into the popup when the keyboard opened it", async () => { + render( + , + ); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + }); + + it("skips a leading unavailable command when taking focus", async () => { + render( + , + ); + + await waitFor(() => { + expect(screen.getByLabelText("Copy")).toHaveFocus(); + }); + }); + + it("leaves focus in the editor when a pointer opened it", async () => { + render( + , + ); + + await waitFor(() => { + expect(screen.getByTestId("editor-context-popup")).toBeInTheDocument(); + }); + expect(screen.getByLabelText("Cut")).not.toHaveFocus(); + }); + + it("takes focus when the keyboard reopens a popup a pointer had opened", async () => { + const { rerender } = render( + , + ); + + expect(screen.getByLabelText("Cut")).not.toHaveFocus(); + + rerender( + , + ); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + }); + + it("returns focus to the editor when it closes while holding focus", async () => { + const onReturnFocus = vi.fn(); + const { rerender } = render( + , + ); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + rerender( + , + ); + + await waitFor(() => { + expect(onReturnFocus).toHaveBeenCalledTimes(1); + }); + expect(document.body).toHaveFocus(); + }); + + it("leaves focus where a click outside put it", async () => { + const onReturnFocus = vi.fn(); + const { user } = renderWithUser(); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + const outside = screen.getByRole("button", { name: "Outside" }); + await user.click(outside); + + await waitFor(() => { + expect(screen.queryByTestId("editor-context-popup")).not.toBeInTheDocument(); + }); + expect(onReturnFocus).not.toHaveBeenCalled(); + expect(outside).toHaveFocus(); + }); + + it("returns focus to the editor when Escape closes it from the toolbar", async () => { + const onReturnFocus = vi.fn(); + const { user } = renderWithUser(); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + await user.keyboard("{Escape}"); + + await waitFor(() => { + expect(screen.queryByTestId("editor-context-popup")).not.toBeInTheDocument(); + }); + expect(onReturnFocus).toHaveBeenCalledTimes(1); + }); + + it("leaves focus alone when it closes without ever holding it", async () => { + const onReturnFocus = vi.fn(); + const { rerender } = render( + , + ); + + rerender( + , + ); + + await waitFor(() => { + expect(screen.queryByTestId("editor-context-popup")).not.toBeInTheDocument(); + }); + expect(onReturnFocus).not.toHaveBeenCalled(); + }); + }); + + describe("toolbar semantics", () => { + const renderToolbar = ( + request: ContextPopupRequest, + overrides: Partial[0]> = {}, + ) => + renderWithUser( + , + ); + + it("exposes a labeled toolbar rather than an unnamed dialog", () => { + renderToolbar(POINTER_REQUEST); + + expect(screen.getByRole("toolbar", { name: "Context actions" })).toBeInTheDocument(); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(screen.queryByRole("group")).not.toBeInTheDocument(); + }); + + it("holds every control outside the tab sequence", () => { + renderToolbar(POINTER_REQUEST); + + const controls = screen.getAllByRole("button"); + + expect(controls).toHaveLength(14); + controls.forEach((control) => { + expect(control).toHaveAttribute("tabindex", "-1"); + }); + }); + + it("moves along a row with the horizontal arrows", async () => { + const { user } = renderToolbar(KEYBOARD_REQUEST); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + await user.keyboard("{ArrowRight}"); + await waitFor(() => { + expect(screen.getByLabelText("Copy")).toHaveFocus(); + }); + + await user.keyboard("{ArrowLeft}"); + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + }); + + it("moves between rows with the vertical arrows, keeping the column", async () => { + const { user } = renderToolbar(KEYBOARD_REQUEST); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + await user.keyboard("{ArrowRight}{ArrowRight}"); + await waitFor(() => { + expect(screen.getByLabelText("Paste")).toHaveFocus(); + }); + + await user.keyboard("{ArrowDown}"); + expect(screen.getByLabelText("Inline code")).toHaveFocus(); + + await user.keyboard("{ArrowUp}"); + expect(screen.getByLabelText("Paste")).toHaveFocus(); + }); + + it("clamps to the nearest column when the next row is shorter", async () => { + const { user } = renderToolbar(KEYBOARD_REQUEST); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + await user.keyboard("{ArrowRight}{ArrowDown}{ArrowDown}"); + expect(screen.getByLabelText("Ordered list")).toHaveFocus(); + + await user.keyboard("{ArrowDown}"); + expect(screen.getByRole("button", { name: "Block type" })).toHaveFocus(); + }); + + it("wraps from the first row back to the last", async () => { + const { user } = renderToolbar(KEYBOARD_REQUEST); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + await user.keyboard("{ArrowUp}"); + expect(screen.getByRole("button", { name: "Insert" })).toHaveFocus(); + + await user.keyboard("{ArrowUp}"); + expect(screen.getByRole("button", { name: "Block type" })).toHaveFocus(); + }); + + it("opens a submenu with ArrowDown instead of leaving its row", async () => { + const { user } = renderToolbar(KEYBOARD_REQUEST); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + await user.keyboard("{ArrowUp}{ArrowUp}{ArrowDown}"); + + await waitFor(() => { + expect(screen.getByRole("menu")).toBeInTheDocument(); + }); + expect(screen.getByRole("menuitem", { name: "Paragraph" })).toBeInTheDocument(); + }); + + it("skips a row whose commands are all unavailable", async () => { + const { user } = renderToolbar(KEYBOARD_REQUEST, { + commandState: { + ...enabledPopupCommandState, + enabledCommands: { + ...enabledPopupCommandState.enabledCommands, + "format.strong": false, + "format.emphasis": false, + "format.inlineCode": false, + "insert.link": false, + }, + }, + }); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + await user.keyboard("{ArrowDown}"); + expect(screen.getByLabelText("Blockquote")).toHaveFocus(); + }); + + it("leaves the toolbar to the editor on Tab", async () => { + const onClose = vi.fn(); + const { user } = renderToolbar(KEYBOARD_REQUEST, { onClose }); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + await user.tab(); + + expect(onClose).toHaveBeenCalledTimes(1); + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + it("runs a focused command with Enter", async () => { + const onExecute = vi.fn(); + const { user } = renderToolbar(KEYBOARD_REQUEST, { onExecute }); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + await user.keyboard("{ArrowRight}{Enter}"); + + expect(onExecute).toHaveBeenCalledWith("edit.copy"); + }); + + it("runs a submenu command from the keyboard", async () => { + const onExecute = vi.fn(); + const { user } = renderToolbar(KEYBOARD_REQUEST, { onExecute }); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + await user.keyboard("{ArrowUp}{ArrowUp}{Enter}"); + await waitFor(() => { + expect(screen.getByRole("menuitem", { name: "Paragraph" })).toHaveFocus(); + }); + + await user.keyboard("{Enter}"); + + expect(onExecute).toHaveBeenCalledWith("format.paragraph"); + }); + + it("closes on Escape from inside the toolbar", async () => { + const onClose = vi.fn(); + const { user } = renderToolbar(KEYBOARD_REQUEST, { onClose }); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + + await user.keyboard("{Escape}"); + + expect(onClose).toHaveBeenCalled(); + }); + + it("closes only the submenu when Escape comes from inside it", async () => { + const { user } = renderWithUser(); + const trigger = screen.getByRole("button", { name: "Block type" }); + + await user.click(trigger); + await waitFor(() => { + expect(screen.getByRole("menu")).toBeInTheDocument(); + }); + + await user.keyboard("{Escape}"); + + await waitFor(() => { + expect(screen.queryByRole("menu")).not.toBeInTheDocument(); + }); + expect(screen.getByTestId("editor-context-popup")).toBeInTheDocument(); + expect(trigger).toHaveFocus(); + }); + }); + + describe("scroll", () => { + it("closes on a scroll while focus is still in the editor", () => { + const onClose = vi.fn(); + + render( + , + ); + + dispatchDOMEvent(document, "scroll"); + + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("stays open on a scroll while focus is inside it", async () => { + const onClose = vi.fn(); + + render( + , + ); + + await waitFor(() => { + expect(screen.getByLabelText("Cut")).toHaveFocus(); + }); + dispatchDOMEvent(document, "scroll"); + + expect(onClose).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/features/editor/components/EditorContextPopup.tsx b/src/features/editor/components/EditorContextPopup.tsx index 568c88f..36b0d8a 100644 --- a/src/features/editor/components/EditorContextPopup.tsx +++ b/src/features/editor/components/EditorContextPopup.tsx @@ -23,7 +23,7 @@ import { Trash2Icon, type LucideIcon, } from "lucide-react"; -import { useEffect } from "react"; +import { useEffect, useRef, type KeyboardEvent } from "react"; import { Button } from "@/components/ui/Button"; import { @@ -33,12 +33,13 @@ import { DropdownMenuTrigger, } from "@/components/ui/DropdownMenu"; import { Popover, PopoverAnchor, PopoverContent } from "@/components/ui/Popover"; +import { Toolbar, ToolbarButton } from "@/components/ui/Toolbar"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/Tooltip"; import { cn } from "@/lib/cn"; import type { EditorCommandId, EditorCommandState } from "../commands"; import { EDITOR_COMMAND_LABELS } from "../commands/metadata"; -import type { ContextPopupAnchor } from "../plugins/contextPopup"; +import type { ContextPopupRequest } from "../plugins/contextPopup"; interface ContextButtonCommand { commandId: EditorCommandId; @@ -101,20 +102,69 @@ const INSERT_COMMANDS = [ const isCommandEnabled = (commandId: EditorCommandId, commandState: EditorCommandState) => commandState.status === "ready" && commandState.enabledCommands[commandId]; +const CONTEXT_POPUP_LABEL = "Context actions"; + +// Radix's roving focus only walks controls in document order, so vertical movement across the +// wrapped rows is worked out from these. +const toolbarPosition = (row: number, column: number) => ({ + "data-toolbar-row": row, + "data-toolbar-column": column, +}); + +const ENABLED_CONTROL_SELECTOR = "[data-toolbar-row]:not([disabled])"; + +const readPosition = (control: HTMLElement, axis: "toolbarRow" | "toolbarColumn") => + Number(control.dataset[axis]); + +const focusFirstControl = (toolbar: HTMLElement | null) => { + toolbar?.querySelector(ENABLED_CONTROL_SELECTOR)?.focus(); +}; + +const focusAdjacentRow = (toolbar: HTMLElement, control: HTMLElement, step: 1 | -1) => { + const controls = [...toolbar.querySelectorAll(ENABLED_CONTROL_SELECTOR)]; + // A row with nothing available drops out here, which is what skips it. + const rows = [...new Set(controls.map((candidate) => readPosition(candidate, "toolbarRow")))]; + const rowIndex = rows.indexOf(readPosition(control, "toolbarRow")); + + if (rowIndex === -1 || rows.length < 2) { + return false; + } + + const column = readPosition(control, "toolbarColumn"); + const nextRow = rows[(rowIndex + step + rows.length) % rows.length]; + const distanceToColumn = (candidate: HTMLElement) => + Math.abs(readPosition(candidate, "toolbarColumn") - column); + + controls + .filter((candidate) => readPosition(candidate, "toolbarRow") === nextRow) + .reduce((closest, candidate) => + distanceToColumn(candidate) < distanceToColumn(closest) ? candidate : closest, + ) + .focus(); + + return true; +}; + interface EditorContextPopupProps { - anchor: ContextPopupAnchor | null; commandState: EditorCommandState; onClose: () => void; onExecute: (commandId: EditorCommandId) => void; + onReturnFocus: () => void; + request: ContextPopupRequest | null; } export function EditorContextPopup({ - anchor, commandState, onClose, onExecute, + onReturnFocus, + request, }: EditorContextPopupProps) { - const isOpen = anchor !== null; + const isOpen = request !== null; + const source = request?.source; + const contentRef = useRef(null); + // Sticky for one open popup, so that focus moving into a portalled submenu does not clear it. + const hasHeldFocusRef = useRef(false); const canExecute = (commandId: EditorCommandId) => isCommandEnabled(commandId, commandState); useEffect(() => { @@ -122,7 +172,11 @@ export function EditorContextPopup({ return undefined; } - const handleScroll = () => onClose(); + const handleScroll = () => { + if (!hasHeldFocusRef.current) { + onClose(); + } + }; document.addEventListener("scroll", handleScroll, true); return () => { @@ -130,10 +184,61 @@ export function EditorContextPopup({ }; }, [onClose, isOpen]); - if (!anchor) { + // Only for a keyboard request landing on an already open popup. A fresh open cannot be served + // here, because the content ref fills a microtask after this runs. + useEffect(() => { + if (isOpen && source === "keyboard") { + focusFirstControl(contentRef.current); + } + }, [isOpen, source]); + + const handleKeyDown = (event: KeyboardEvent) => { + // A submenu keeps its keys despite reaching here through its portal: its Escape closes it. + if (!(event.target instanceof Node) || !event.currentTarget.contains(event.target)) { + return; + } + + // The focused control's tooltip is the layer Radix offers Escape to first, so leaving this + // to the popup's own layer would cost a second press. Closing twice asks nothing of a closed + // popup, and the two cases cannot be told apart: both mark the event as handled. + if (event.key === "Escape") { + onClose(); + return; + } + + if (event.key === "Tab") { + // Tabbing on would land after the portal rather than back in the text. + event.preventDefault(); + onClose(); + return; + } + + // Left to Radix: the horizontal arrows to roving focus, ArrowDown to a submenu trigger. + if (event.defaultPrevented) { + return; + } + + const step = event.key === "ArrowDown" ? 1 : event.key === "ArrowUp" ? -1 : null; + const control = document.activeElement; + + if ( + step === null || + !(control instanceof HTMLElement) || + !event.currentTarget.contains(control) + ) + return; + + if (focusAdjacentRow(event.currentTarget, control, step)) { + event.preventDefault(); + } + }; + + if (!request) { return null; } + const { anchor } = request; + return ( !nextIsOpen && onClose()}> @@ -149,40 +254,79 @@ export function EditorContextPopup({ event.preventDefault()} - onOpenAutoFocus={(event) => event.preventDefault()} + onCloseAutoFocus={(event) => { + // Radix restores focus to a trigger, and this popup only has an anchor, so its restore + // is a no-op that leaves focus on the body. + event.preventDefault(); + + if (hasHeldFocusRef.current) { + hasHeldFocusRef.current = false; + onReturnFocus(); + } + }} + onFocus={() => { + hasHeldFocusRef.current = true; + }} + onInteractOutside={() => { + // Radix defers this dismissal past the click, so returning focus would take it back + // from whatever was just clicked. + hasHeldFocusRef.current = false; + }} + onOpenAutoFocus={(event) => { + // Radix would take focus on every open, including the pointer ones that must not. + event.preventDefault(); + hasHeldFocusRef.current = false; + + if (source === "keyboard" && event.currentTarget instanceof HTMLElement) { + focusFirstControl(event.currentTarget); + } + }} + ref={contentRef} side="bottom" sideOffset={8} > - - - - - + + + + + + + ); @@ -192,30 +336,33 @@ interface ContextCommandRowProps { commands: readonly ContextButtonCommand[]; onExecute: (commandId: EditorCommandId) => void; canExecute: (commandId: EditorCommandId) => boolean; + row: number; } -function ContextCommandRow({ commands, onExecute, canExecute }: ContextCommandRowProps) { +function ContextCommandRow({ commands, onExecute, canExecute, row }: ContextCommandRowProps) { return ( -
- {commands.map(({ commandId, icon: Icon }) => { +
+ {commands.map(({ commandId, icon: Icon }, column) => { const label = EDITOR_COMMAND_LABELS[commandId]; const enabled = canExecute(commandId); return ( - - - + + + + + {label} @@ -231,6 +378,7 @@ interface ContextCommandSubmenuProps { commands: readonly ContextSubmenuCommand[]; onExecute: (commandId: EditorCommandId) => void; canExecute: (commandId: EditorCommandId) => boolean; + row: number; } function ContextCommandSubmenu({ @@ -238,26 +386,23 @@ function ContextCommandSubmenu({ commands, onExecute, canExecute, + row, }: ContextCommandSubmenuProps) { return ( - - - - event.preventDefault()} - > + + + + + + {commands.map(({ commandId, icon: CommandIcon }) => { const label = EDITOR_COMMAND_LABELS[commandId]; const enabled = canExecute(commandId); diff --git a/src/features/editor/components/MilkdownEditor.tsx b/src/features/editor/components/MilkdownEditor.tsx index 307abe9..1a189ed 100644 --- a/src/features/editor/components/MilkdownEditor.tsx +++ b/src/features/editor/components/MilkdownEditor.tsx @@ -41,18 +41,24 @@ export function MilkdownEditor({ autoPairBracketsAndQuotes = true, softWrapCodeBlocks = false, }: MilkdownEditorProps) { - const { closeContextPopup, commandState, contextPopupAnchor, executeContextCommand, rootRef } = - useMilkdownEditorInstance({ - autoPairBracketsAndQuotes, - documentPath, - folderContextPath, - initialMarkdown, - onMarkdownUpdated, - onContentChanged, - onCommandStateChanged, - onOpenMarkdownPath, - ref, - }); + const { + closeContextPopup, + commandState, + contextPopupRequest, + executeContextCommand, + focusEditor, + rootRef, + } = useMilkdownEditorInstance({ + autoPairBracketsAndQuotes, + documentPath, + folderContextPath, + initialMarkdown, + onMarkdownUpdated, + onContentChanged, + onCommandStateChanged, + onOpenMarkdownPath, + ref, + }); return (
); diff --git a/src/features/editor/hooks/useMilkdownEditorInstance.ts b/src/features/editor/hooks/useMilkdownEditorInstance.ts index 55d79a8..2ef7995 100644 --- a/src/features/editor/hooks/useMilkdownEditorInstance.ts +++ b/src/features/editor/hooks/useMilkdownEditorInstance.ts @@ -19,7 +19,7 @@ import { type EditorCommandId, type EditorCommandState, } from "../commands"; -import type { ContextPopupAnchor } from "../plugins/contextPopup"; +import type { ContextPopupRequest } from "../plugins/contextPopup"; import { createMilkdownEditor, getMilkdownEditorMarkdown, @@ -63,7 +63,7 @@ export const useMilkdownEditorInstance = ({ const [commandState, setCommandState] = useState( INACTIVE_EDITOR_COMMAND_STATE, ); - const [contextPopupAnchor, setContextPopupAnchor] = useState(null); + const [contextPopupRequest, setContextPopupRequest] = useState(null); const commandStateRef = useRef(INACTIVE_EDITOR_COMMAND_STATE); const liveOptionsRef = useRef({ @@ -122,12 +122,26 @@ export const useMilkdownEditorInstance = ({ const closeContextPopup = useCallback(() => { contextPopupOpenRef.current = false; - setContextPopupAnchor(null); + setContextPopupRequest(null); }, []); - const requestContextPopup = useCallback((anchor: ContextPopupAnchor) => { + const requestContextPopup = useCallback((request: ContextPopupRequest) => { contextPopupOpenRef.current = true; - setContextPopupAnchor(anchor); + setContextPopupRequest(request); + }, []); + + const focusEditor = useCallback(() => { + const editor = editorRef.current; + + if (!editor?.ctx) { + return; + } + + try { + editor.ctx.get(editorViewCtx).focus(); + } catch (error) { + handleUnexpectedError(error, "focusEditor"); + } }, []); const updateCommandState = useCallback((nextCommandState: EditorCommandState) => { @@ -239,8 +253,9 @@ export const useMilkdownEditorInstance = ({ return { closeContextPopup, commandState, - contextPopupAnchor, + contextPopupRequest, executeContextCommand, + focusEditor, rootRef, }; }; diff --git a/src/features/editor/index.ts b/src/features/editor/index.ts index 04afd76..8d68ea9 100644 --- a/src/features/editor/index.ts +++ b/src/features/editor/index.ts @@ -14,7 +14,11 @@ export { type MilkdownEditorBridge, type MilkdownEditorProps, } from "./components/MilkdownEditor"; -export type { ContextPopupAnchor } from "./plugins/contextPopup"; +export type { + ContextPopupAnchor, + ContextPopupRequest, + ContextPopupSource, +} from "./plugins/contextPopup"; export { createMilkdownEditor, getMilkdownEditorMarkdown, diff --git a/src/features/editor/plugins/contextPopup.test.tsx b/src/features/editor/plugins/contextPopup.test.tsx index eaf8fbc..1855dbd 100644 --- a/src/features/editor/plugins/contextPopup.test.tsx +++ b/src/features/editor/plugins/contextPopup.test.tsx @@ -26,7 +26,10 @@ describe("context popup plugin", () => { dispatchMouseUp(mounted.view.dom, { button: 0 }); await waitFor(() => { - expect(onContextPopupRequested).toHaveBeenCalledWith({ x: 19, top: 31, bottom: 46 }); + expect(onContextPopupRequested).toHaveBeenCalledWith({ + anchor: { x: 19, top: 31, bottom: 46 }, + source: "pointer", + }); }); expect(coordsAtPos).toHaveBeenNthCalledWith(1, 1, 1); expect(coordsAtPos).toHaveBeenNthCalledWith(2, 6, -1); @@ -41,7 +44,10 @@ describe("context popup plugin", () => { setTextSelection(mounted.view, 1, 6); dispatchContextMenu(mounted.view.dom, { clientX: 80, clientY: 42 }); - expect(onContextPopupRequested).toHaveBeenCalledWith({ x: 19, top: 31, bottom: 46 }); + expect(onContextPopupRequested).toHaveBeenCalledWith({ + anchor: { x: 19, top: 31, bottom: 46 }, + source: "pointer", + }); expect(posAtCoords).not.toHaveBeenCalled(); expect(mounted.view.state.selection.empty).toBe(false); expect(mounted.view.state.selection.from).toBe(1); @@ -56,11 +62,59 @@ describe("context popup plugin", () => { setTextSelection(mounted.view, 8); dispatchContextMenu(mounted.view.dom, { clientX: 80, clientY: 42 }); - expect(onContextPopupRequested).toHaveBeenCalledWith({ x: 23, top: 38, bottom: 48 }); + expect(onContextPopupRequested).toHaveBeenCalledWith({ + anchor: { x: 23, top: 38, bottom: 48 }, + source: "pointer", + }); expect(mounted.view.state.selection.empty).toBe(true); expect(mounted.view.state.selection.from).toBe(8); }); + it.each([ + ["the Menu key", "ContextMenu", {}], + ["Shift+F10", "F10", { shift: true }], + ])("opens from %s as a keyboard request", async (_label, key, modifiers) => { + const onContextPopupRequested = vi.fn(); + const mounted = await mountEditor(HELLO_WORLD_TEXT, { onContextPopupRequested }); + + mockCoordinates(mounted); + setTextSelection(mounted.view, 1, 6); + const { event, handled } = runKeyDownHandlers(mounted.view, key, modifiers); + + expect(handled).toBe(true); + expect(event.defaultPrevented).toBe(true); + expect(onContextPopupRequested).toHaveBeenCalledWith({ + anchor: { x: 19, top: 31, bottom: 46 }, + source: "keyboard", + }); + }); + + it("opens from the keyboard around a caret with no selection", async () => { + const onContextPopupRequested = vi.fn(); + const mounted = await mountEditor(HELLO_WORLD_TEXT, { onContextPopupRequested }); + + mockCoordinates(mounted); + setTextSelection(mounted.view, 8); + runKeyDownHandlers(mounted.view, "ContextMenu"); + + expect(onContextPopupRequested).toHaveBeenCalledWith({ + anchor: { x: 23, top: 38, bottom: 48 }, + source: "keyboard", + }); + }); + + it("leaves an unmodified F10 to the rest of the editor", async () => { + const onContextPopupRequested = vi.fn(); + const mounted = await mountEditor(HELLO_WORLD_TEXT, { onContextPopupRequested }); + + setTextSelection(mounted.view, 1, 6); + const { event, handled } = runKeyDownHandlers(mounted.view, "F10"); + + expect(handled).toBe(false); + expect(event.defaultPrevented).toBe(false); + expect(onContextPopupRequested).not.toHaveBeenCalled(); + }); + it("does not open for an ordinary caret placement", async () => { const onContextPopupRequested = vi.fn(); const mounted = await mountEditor(HELLO_WORLD_TEXT, { onContextPopupRequested }); @@ -114,11 +168,73 @@ describe("context popup plugin", () => { popupOpen = true; setTextSelection(mounted.view, 1, 6); - expect(onContextPopupRequested).toHaveBeenCalledWith({ x: 19, top: 31, bottom: 46 }); + expect(onContextPopupRequested).toHaveBeenCalledWith({ + anchor: { x: 19, top: 31, bottom: 46 }, + source: "pointer", + }); expect(onContextPopupClosed).not.toHaveBeenCalled(); setTextSelection(mounted.view, 3); expect(onContextPopupClosed).toHaveBeenCalledTimes(1); }); + + it("keeps the popup open and the selection intact while focus leaves the editor", async () => { + let popupOpen = false; + const onContextPopupClosed = vi.fn(() => { + popupOpen = false; + }); + const onContextPopupRequested = vi.fn(() => { + popupOpen = true; + }); + const mounted = await mountEditor(HELLO_WORLD_TEXT, { + getContextPopupOpen: () => popupOpen, + onContextPopupClosed, + onContextPopupRequested, + }); + + mockCoordinates(mounted); + setTextSelection(mounted.view, 1, 6); + runKeyDownHandlers(mounted.view, "ContextMenu"); + + expect(popupOpen).toBe(true); + + // Stands in for the popup taking focus. + const elsewhere = document.createElement("button"); + document.body.append(elsewhere); + + try { + elsewhere.focus(); + + expect(onContextPopupClosed).not.toHaveBeenCalled(); + + mounted.view.focus(); + } finally { + elsewhere.remove(); + } + + expect(mounted.view.state.selection.from).toBe(1); + expect(mounted.view.state.selection.to).toBe(6); + }); + + it("keeps a keyboard-opened popup keyboard-sourced when its selection moves", async () => { + let popupOpen = false; + const onContextPopupRequested = vi.fn(() => { + popupOpen = true; + }); + const mounted = await mountEditor(HELLO_WORLD_TEXT, { + getContextPopupOpen: () => popupOpen, + onContextPopupRequested, + }); + + mockCoordinates(mounted); + setTextSelection(mounted.view, 1, 6); + runKeyDownHandlers(mounted.view, "ContextMenu"); + setTextSelection(mounted.view, 2, 7); + + expect(onContextPopupRequested).toHaveBeenLastCalledWith({ + anchor: { x: 20, top: 32, bottom: 47 }, + source: "keyboard", + }); + }); }); diff --git a/src/features/editor/plugins/contextPopup.ts b/src/features/editor/plugins/contextPopup.ts index c330a72..403ee64 100644 --- a/src/features/editor/plugins/contextPopup.ts +++ b/src/features/editor/plugins/contextPopup.ts @@ -10,10 +10,17 @@ export interface ContextPopupAnchor { bottom: number; } +export type ContextPopupSource = "keyboard" | "pointer"; + +export interface ContextPopupRequest { + anchor: ContextPopupAnchor; + source: ContextPopupSource; +} + export interface LeafdownContextPopupPluginOptions { isOpen?: () => boolean; onClose?: () => void; - onRequest?: (anchor: ContextPopupAnchor) => void; + onRequest?: (request: ContextPopupRequest) => void; } const getSelectionAnchor = (view: EditorView): ContextPopupAnchor | null => { @@ -33,21 +40,6 @@ const getSelectionAnchor = (view: EditorView): ContextPopupAnchor | null => { } }; -const requestSelectionPopup = ( - view: EditorView, - onRequest: LeafdownContextPopupPluginOptions["onRequest"], -) => { - const anchor = getSelectionAnchor(view); - - if (!anchor) { - return false; - } - - onRequest?.(anchor); - - return true; -}; - const closePopup = ({ isOpen, onClose }: LeafdownContextPopupPluginOptions) => { if (!isOpen?.()) { return false; @@ -58,97 +50,123 @@ const closePopup = ({ isOpen, onClose }: LeafdownContextPopupPluginOptions) => { return true; }; -const syncPopupToSelection = ( - view: EditorView, - previousState: EditorView["state"], - options: LeafdownContextPopupPluginOptions, -) => { - if (!options.isOpen?.()) { - return; - } - - const selectionChanged = !view.state.selection.eq(previousState.selection); - const documentChanged = view.state.doc !== previousState.doc; - - if (!selectionChanged && !documentChanged) { - return; - } - - if (view.state.selection.empty || !requestSelectionPopup(view, options.onRequest)) { - options.onClose?.(); - } -}; - const isEditablePopupTarget = (event: MouseEvent) => event.target instanceof HTMLElement && event.target.closest("input, textarea, select") !== null; +const isContextMenuKey = (event: KeyboardEvent) => + event.key === "ContextMenu" || (event.key === "F10" && event.shiftKey); + export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPluginOptions = {}) => - $prose( - () => - new Plugin({ - key: leafdownContextPopupPluginKey, - view: () => ({ - update: (view, previousState) => { - syncPopupToSelection(view, previousState, options); - }, - }), - props: { - handleDOMEvents: { - contextmenu: (view, event) => { - if (!(event instanceof MouseEvent) || isEditablePopupTarget(event)) { - return false; - } + $prose(() => { + // Held so that refreshing an open popup's anchor cannot downgrade it to a pointer open. + let openSource: ContextPopupSource = "pointer"; - event.preventDefault(); - view.focus(); + const requestSelectionPopup = (view: EditorView, source: ContextPopupSource) => { + const anchor = getSelectionAnchor(view); - if (!requestSelectionPopup(view, options.onRequest)) { - options.onClose?.(); - } + if (!anchor) { + return false; + } - return true; - }, - mouseup: (view, event) => { - if (!(event instanceof MouseEvent) || event.button !== 0) { - return false; - } + openSource = source; + options.onRequest?.({ anchor, source }); - window.requestAnimationFrame(() => { - if (view.isDestroyed) { - return; - } + return true; + }; - if (view.state.selection.empty) { - closePopup(options); - return; - } + const syncPopupToSelection = (view: EditorView, previousState: EditorView["state"]) => { + if (!options.isOpen?.()) { + return; + } - if (!requestSelectionPopup(view, options.onRequest)) { - options.onClose?.(); - } - }); + const selectionChanged = !view.state.selection.eq(previousState.selection); + const documentChanged = view.state.doc !== previousState.doc; - return false; - }, - }, - handleKeyDown: (_view, event) => { - if (event.key !== "Escape") { - return false; - } + if (!selectionChanged && !documentChanged) { + return; + } - if (!closePopup(options)) { + if (view.state.selection.empty || !requestSelectionPopup(view, openSource)) { + options.onClose?.(); + } + }; + + return new Plugin({ + key: leafdownContextPopupPluginKey, + view: () => ({ + update: (view, previousState) => { + syncPopupToSelection(view, previousState); + }, + }), + props: { + handleDOMEvents: { + contextmenu: (view, event) => { + if (!(event instanceof MouseEvent) || isEditablePopupTarget(event)) { return false; } event.preventDefault(); + view.focus(); + + if (!requestSelectionPopup(view, "pointer")) { + options.onClose?.(); + } return true; }, - handleTextInput: () => { - closePopup(options); + mouseup: (view, event) => { + if (!(event instanceof MouseEvent) || event.button !== 0) { + return false; + } + + window.requestAnimationFrame(() => { + if (view.isDestroyed) { + return; + } + + if (view.state.selection.empty) { + closePopup(options); + return; + } + + if (!requestSelectionPopup(view, "pointer")) { + options.onClose?.(); + } + }); return false; }, }, - }), - ); + handleKeyDown: (view, event) => { + if (isContextMenuKey(event)) { + // Also suppresses the contextmenu event the key would produce, which would reopen + // this popup through the pointer path. + event.preventDefault(); + + if (!requestSelectionPopup(view, "keyboard")) { + options.onClose?.(); + } + + return true; + } + + if (event.key !== "Escape") { + return false; + } + + if (!closePopup(options)) { + return false; + } + + event.preventDefault(); + + return true; + }, + handleTextInput: () => { + closePopup(options); + + return false; + }, + }, + }); + }); diff --git a/src/test/utils/milkdown.ts b/src/test/utils/milkdown.ts index 8d9c13f..71c13cd 100644 --- a/src/test/utils/milkdown.ts +++ b/src/test/utils/milkdown.ts @@ -3,7 +3,7 @@ import type { EditorView } from "@milkdown/kit/prose/view"; import { afterEach } from "vitest"; import { - type ContextPopupAnchor, + type ContextPopupRequest, createMilkdownEditor, type EditorCommandState, getMilkdownEditorMarkdown, @@ -28,7 +28,7 @@ export interface MountMilkdownEditorOptions extends Partial void; onOpenMarkdownPath?: (path: string) => boolean | Promise; onContextPopupClosed?: () => void; - onContextPopupRequested?: (anchor: ContextPopupAnchor) => void; + onContextPopupRequested?: (request: ContextPopupRequest) => void; getContextPopupOpen?: () => boolean; }