From f3ec3fd3518c15dae76918b8e9809a1ac92aae91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ademir=20Jos=C3=A9=20Ferreira=20J=C3=BAnior?= Date: Mon, 3 Aug 2026 13:32:10 -0300 Subject: [PATCH 1/2] Open the context popup from a selection made with the keyboard Opening from the plugin update path rather than a new key binding covers `Select all` too, which arrives as an ordinary selection transaction with no gesture of its own. --- CHANGELOG.md | 1 + docs/reference.md | 2 +- docs/specification.md | 4 +- .../editor/plugins/contextPopup.test.tsx | 130 +++++++++++++++++- src/features/editor/plugins/contextPopup.ts | 85 ++++++++++-- 5 files changed, 206 insertions(+), 16 deletions(-) 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..775ee36 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 and leaves the selection standing. It stays dismissed for the rest of that selection gesture, and until the selection collapses when there is no gesture to end. +- `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..a10db21 100644 --- a/src/features/editor/plugins/contextPopup.ts +++ b/src/features/editor/plugins/contextPopup.ts @@ -39,12 +39,35 @@ const isEditablePopupTarget = (event: MouseEvent) => const isContextMenuKey = (event: KeyboardEvent) => event.key === "ContextMenu" || (event.key === "F10" && event.shiftKey); +const SELECTION_MOVEMENT_KEYS = new Set([ + "ArrowDown", + "ArrowLeft", + "ArrowRight", + "ArrowUp", + "End", + "Home", + "PageDown", + "PageUp", +]); + +const MODIFIER_KEYS = new Set(["Alt", "Control", "Meta", "Shift"]); + +// A shifted movement key extends the selection, and a modifier pressed before one belongs to the +// same gesture rather than ending it. +const continuesSelectionGesture = (event: KeyboardEvent) => + MODIFIER_KEYS.has(event.key) || (event.shiftKey && SELECTION_MOVEMENT_KEYS.has(event.key)); + export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPluginOptions = {}) => $prose(() => { // Held so that refreshing an open popup's anchor cannot downgrade it to a pointer open. let openSource: ContextPopupSource = "pointer"; // The anchor measures the live selection, so one per editor serves every request. let anchor: ContextPopupAnchor | null = null; + // Latched by an explicit dismissal: `Escape` leaves the selection standing, so without this + // the next keystroke of the same gesture 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 +76,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 +90,29 @@ export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPl return; } - if (view.state.selection.empty || !requestSelectionPopup(view, openSource)) { - options.onClose?.(); + if (view.state.selection.empty) { + // A collapsed selection outlives no gesture, so it releases the dismissal too. + 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 +139,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 +172,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. @@ -134,17 +188,22 @@ export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPl return true; } - if (event.key !== "Escape") { - return false; - } + if (event.key === "Escape") { + if (!closePopup(options)) { + return false; + } - if (!closePopup(options)) { - return false; + dismissed = true; + event.preventDefault(); + + return true; } - event.preventDefault(); + if (!continuesSelectionGesture(event)) { + dismissed = false; + } - return true; + return false; }, handleTextInput: () => { closePopup(options); From 0c75a7af8ad67acd833b6445a77f8590b893d34a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ademir=20Jos=C3=A9=20Ferreira=20J=C3=BAnior?= Date: Mon, 3 Aug 2026 14:08:08 -0300 Subject: [PATCH 2/2] Release the popup dismissal on collapse alone Deriving the end of a selection gesture from the keystroke cannot serve `Select all`: extending afterwards re-anchors the selection at the text start, so the dismissal would lift on the first `Shift+Arrow`. Collapsing is the one thing no selection survives, and holding the dismissal until then is conservative in the direction the user asked for. --- docs/specification.md | 2 +- src/features/editor/plugins/contextPopup.ts | 45 ++++++--------------- 2 files changed, 13 insertions(+), 34 deletions(-) diff --git a/docs/specification.md b/docs/specification.md index 775ee36..f4c0792 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -168,7 +168,7 @@ The editor is a unified hybrid Markdown surface. Behavior is governed by renderi - `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). - 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 and leaves the selection standing. It stays dismissed for the rest of that selection gesture, and until the selection collapses when there is no gesture to end. +- `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. diff --git a/src/features/editor/plugins/contextPopup.ts b/src/features/editor/plugins/contextPopup.ts index a10db21..095542d 100644 --- a/src/features/editor/plugins/contextPopup.ts +++ b/src/features/editor/plugins/contextPopup.ts @@ -39,32 +39,14 @@ const isEditablePopupTarget = (event: MouseEvent) => const isContextMenuKey = (event: KeyboardEvent) => event.key === "ContextMenu" || (event.key === "F10" && event.shiftKey); -const SELECTION_MOVEMENT_KEYS = new Set([ - "ArrowDown", - "ArrowLeft", - "ArrowRight", - "ArrowUp", - "End", - "Home", - "PageDown", - "PageUp", -]); - -const MODIFIER_KEYS = new Set(["Alt", "Control", "Meta", "Shift"]); - -// A shifted movement key extends the selection, and a modifier pressed before one belongs to the -// same gesture rather than ending it. -const continuesSelectionGesture = (event: KeyboardEvent) => - MODIFIER_KEYS.has(event.key) || (event.shiftKey && SELECTION_MOVEMENT_KEYS.has(event.key)); - export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPluginOptions = {}) => $prose(() => { // Held so that refreshing an open popup's anchor cannot downgrade it to a pointer open. let openSource: ContextPopupSource = "pointer"; // The anchor measures the live selection, so one per editor serves every request. let anchor: ContextPopupAnchor | null = null; - // Latched by an explicit dismissal: `Escape` leaves the selection standing, so without this - // the next keystroke of the same gesture would reopen what was just dismissed. + // `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; @@ -91,7 +73,8 @@ export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPl } if (view.state.selection.empty) { - // A collapsed selection outlives no gesture, so it releases the dismissal too. + // The only release: an extension never collapses, and `Select all` has no gesture whose + // end could serve instead. dismissed = false; if (options.isOpen?.()) { @@ -188,22 +171,18 @@ export const createLeafdownContextPopupPlugin = (options: LeafdownContextPopupPl return true; } - if (event.key === "Escape") { - if (!closePopup(options)) { - return false; - } - - dismissed = true; - event.preventDefault(); - - return true; + if (event.key !== "Escape") { + return false; } - if (!continuesSelectionGesture(event)) { - dismissed = false; + if (!closePopup(options)) { + return false; } - return false; + dismissed = true; + event.preventDefault(); + + return true; }, handleTextInput: () => { closePopup(options);