diff --git a/CHANGELOG.md b/CHANGELOG.md index 3269fb9..61a2fa3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Leafdown uses lightweight [Keep a Changelog](https://keepachangelog.com/en/1.1.0 - Announce the article navigator as a tree, with the nesting depth, sibling position, and expanded state of every row. - Keep empty folders in the article navigator reachable instead of skipping them. - Open the editor context popup with `Shift+F10` or the `Menu` key and operate every command in it from the keyboard. +- Open the editor context popup from a selection made with the keyboard, `Select all` included, instead of only from a pointer selection. - Keep the editor context popup beside the text it acts on while the document scrolls, and inside a selection too tall to sit beside. - Hide the editor context popup while its selection is scrolled out of view instead of closing it, and bring it back with the selection. - Announce the editor context popup as a named toolbar instead of an unnamed dialog. diff --git a/docs/reference.md b/docs/reference.md index d49809d..dda88f7 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -235,7 +235,7 @@ For diagnostic log format and ownership, see [Architecture](./architecture.md#ba ### Context Popup -The context popup is a contextual menu triggered by selection, right-click, or `Shift+F10` and the `Menu` key within the editor. +The context popup is a contextual menu triggered by a pointer or keyboard 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. diff --git a/docs/specification.md b/docs/specification.md index 98378e2..f4c0792 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -167,7 +167,9 @@ 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. +- Making a selection opens the context popup, whether it was made with the pointer, extended with `Shift+Arrow` or `Mod+Shift+Arrow`, or made whole by `Select all`. A pointer selection opens it on release, a keyboard one as the selection changes; extending further keeps the open popup rather than reopening it. +- `Escape` dismisses a popup that does not hold focus, leaving the selection standing, and it stays dismissed until the selection collapses. +- `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 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. diff --git a/src/features/editor/plugins/contextPopup.test.tsx b/src/features/editor/plugins/contextPopup.test.tsx index ee62256..b14dd84 100644 --- a/src/features/editor/plugins/contextPopup.test.tsx +++ b/src/features/editor/plugins/contextPopup.test.tsx @@ -2,7 +2,12 @@ import { describe, expect, it, vi } from "vitest"; import type { ContextPopupRequest, ContextPopupSource } from "@/features/editor"; import { HELLO_WORLD_TEXT } from "@/test/fixtures/editorMarkdown"; -import { dispatchContextMenu, dispatchMouseUp } from "@/test/utils/events"; +import { + dispatchContextMenu, + dispatchMouseDown, + dispatchMouseUp, + type TestKeyboardEventOptions, +} from "@/test/utils/events"; import { setupMilkdownEditorMount, type MountedMilkdownEditor } from "@/test/utils/milkdown"; import { runKeyDownHandlers, setTextSelection, typeText } from "@/test/utils/prosemirror"; import { waitFor } from "@/test/utils/react"; @@ -17,6 +22,34 @@ const mockCoordinates = (mounted: MountedMilkdownEditor) => top: 30 + pos, })); +// The browser moves the selection for these keys, so the movement is dispatched beside the +// keydown rather than produced by it. +const extendSelection = ( + mounted: MountedMilkdownEditor, + head: number, + modifiers: TestKeyboardEventOptions = {}, +) => { + runKeyDownHandlers(mounted.view, "ArrowRight", { shift: true, ...modifiers }); + setTextSelection(mounted.view, 1, head); +}; + +const selectAll = (mounted: MountedMilkdownEditor) => + runKeyDownHandlers(mounted.view, "a", { ctrl: true, keyCode: 65 }); + +const trackPopupOpenState = () => { + let popupOpen = false; + + return { + getContextPopupOpen: () => popupOpen, + onContextPopupClosed: vi.fn(() => { + popupOpen = false; + }), + onContextPopupRequested: vi.fn(() => { + popupOpen = true; + }), + }; +}; + const popupRequest = (source: ContextPopupSource) => ({ anchor: expect.objectContaining({ getRect: expect.any(Function) }), source, @@ -152,6 +185,8 @@ describe("context popup plugin", () => { const mounted = await mountEditor(HELLO_WORLD_TEXT, { onContextPopupRequested }); setTextSelection(mounted.view, 1, 6); + // The selection opens the popup on its own, so only what F10 adds is under test here. + onContextPopupRequested.mockClear(); const { event, handled } = runKeyDownHandlers(mounted.view, "F10"); expect(handled).toBe(false); @@ -173,6 +208,99 @@ describe("context popup plugin", () => { expect(onContextPopupRequested).not.toHaveBeenCalled(); }); + it.each([ + ["Shift+Arrow", {}], + ["Mod+Shift+Arrow", { ctrl: true }], + ])( + "opens from a selection extended with %s, leaving focus in the editor", + async (_label, modifiers) => { + const onContextPopupRequested = vi.fn(); + const mounted = await mountEditor(HELLO_WORLD_TEXT, { onContextPopupRequested }); + + mockCoordinates(mounted); + mounted.view.focus(); + extendSelection(mounted, 6, modifiers); + + expect(onContextPopupRequested).toHaveBeenCalledWith(popupRequest("pointer")); + expect(document.activeElement).toBe(mounted.view.dom); + }, + ); + + it("opens from Select all, leaving focus in the editor", async () => { + const onContextPopupRequested = vi.fn(); + const mounted = await mountEditor(HELLO_WORLD_TEXT, { onContextPopupRequested }); + + mockCoordinates(mounted); + mounted.view.focus(); + selectAll(mounted); + + expect(mounted.view.state.selection.empty).toBe(false); + expect(onContextPopupRequested).toHaveBeenCalledWith(popupRequest("pointer")); + expect(document.activeElement).toBe(mounted.view.dom); + }); + + it("keeps one popup open while the selection grows", async () => { + const popupState = trackPopupOpenState(); + const mounted = await mountEditor(HELLO_WORLD_TEXT, popupState); + + mockCoordinates(mounted); + mounted.view.focus(); + extendSelection(mounted, 6); + extendSelection(mounted, 7); + + expect(popupState.onContextPopupRequested).toHaveBeenLastCalledWith(popupRequest("pointer")); + expect(popupState.onContextPopupClosed).not.toHaveBeenCalled(); + expect(document.activeElement).toBe(mounted.view.dom); + }); + + it("stays dismissed for the rest of the selection gesture", async () => { + const popupState = trackPopupOpenState(); + const mounted = await mountEditor(HELLO_WORLD_TEXT, popupState); + + mockCoordinates(mounted); + extendSelection(mounted, 6); + runKeyDownHandlers(mounted.view, "Escape"); + popupState.onContextPopupRequested.mockClear(); + extendSelection(mounted, 7); + + expect(popupState.onContextPopupRequested).not.toHaveBeenCalled(); + }); + + it("stays dismissed after Select all until the selection collapses", async () => { + const popupState = trackPopupOpenState(); + const mounted = await mountEditor(HELLO_WORLD_TEXT, popupState); + + mockCoordinates(mounted); + selectAll(mounted); + runKeyDownHandlers(mounted.view, "Escape"); + popupState.onContextPopupRequested.mockClear(); + extendSelection(mounted, 7); + + expect(popupState.onContextPopupRequested).not.toHaveBeenCalled(); + + setTextSelection(mounted.view, 3); + extendSelection(mounted, 6); + + expect(popupState.onContextPopupRequested).toHaveBeenCalledWith(popupRequest("pointer")); + }); + + it("holds a pointer selection back until the button is released", async () => { + const onContextPopupRequested = vi.fn(); + const mounted = await mountEditor(HELLO_WORLD_TEXT, { onContextPopupRequested }); + + mockCoordinates(mounted); + dispatchMouseDown(mounted.view.dom, { button: 0 }); + setTextSelection(mounted.view, 1, 6); + + expect(onContextPopupRequested).not.toHaveBeenCalled(); + + dispatchMouseUp(mounted.view.dom, { button: 0 }); + + await waitFor(() => { + expect(onContextPopupRequested).toHaveBeenCalledWith(popupRequest("pointer")); + }); + }); + it("closes on Escape and typing", async () => { let popupOpen = true; const onContextPopupClosed = vi.fn(() => { diff --git a/src/features/editor/plugins/contextPopup.ts b/src/features/editor/plugins/contextPopup.ts index 2d96884..095542d 100644 --- a/src/features/editor/plugins/contextPopup.ts +++ b/src/features/editor/plugins/contextPopup.ts @@ -45,6 +45,11 @@ export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPl let openSource: ContextPopupSource = "pointer"; // The anchor measures the live selection, so one per editor serves every request. let anchor: ContextPopupAnchor | null = null; + // `Escape` leaves the selection standing, so without this the next keystroke that extends it + // would reopen what was just dismissed. + let dismissed = false; + // The pointer path opens on release, so the selection a drag builds must not open the popup. + let pointerSelecting = false; const requestSelectionPopup = (view: EditorView, source: ContextPopupSource) => { if (!canMeasureSelection(view)) { @@ -53,16 +58,13 @@ export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPl anchor ??= createContextPopupAnchor(view); openSource = source; + dismissed = false; options.onRequest?.({ anchor, source }); return true; }; const syncPopupToSelection = (view: EditorView, previousState: EditorView["state"]) => { - if (!options.isOpen?.()) { - return; - } - const selectionChanged = !view.state.selection.eq(previousState.selection); const documentChanged = view.state.doc !== previousState.doc; @@ -70,8 +72,30 @@ export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPl return; } - if (view.state.selection.empty || !requestSelectionPopup(view, openSource)) { - options.onClose?.(); + if (view.state.selection.empty) { + // The only release: an extension never collapses, and `Select all` has no gesture whose + // end could serve instead. + dismissed = false; + + if (options.isOpen?.()) { + options.onClose?.(); + } + + return; + } + + if (options.isOpen?.()) { + if (!requestSelectionPopup(view, openSource)) { + options.onClose?.(); + } + + return; + } + + // A keyboard request would move focus into the popup and end the gesture that opened it, + // so this stays a pointer one. An edit that leaves a selection standing is not one made. + if (!documentChanged && !dismissed && !pointerSelecting) { + requestSelectionPopup(view, "pointer"); } }; @@ -98,12 +122,21 @@ export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPl return true; }, + mousedown: (_view, event) => { + if (event instanceof MouseEvent && event.button === 0) { + pointerSelecting = true; + } + + return false; + }, mouseup: (view, event) => { if (!(event instanceof MouseEvent) || event.button !== 0) { return false; } window.requestAnimationFrame(() => { + pointerSelecting = false; + if (view.isDestroyed) { return; } @@ -122,6 +155,10 @@ export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPl }, }, handleKeyDown: (view, event) => { + // A keystroke means the drag is over, including one whose release the handler above + // never saw because it landed outside the editor. + pointerSelecting = false; + if (isContextMenuKey(event)) { // Also suppresses the contextmenu event the key would produce, which would reopen // this popup through the pointer path. @@ -142,6 +179,7 @@ export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPl return false; } + dismissed = true; event.preventDefault(); return true;