From 4f8e4ad15d01e5c438f8fd4cc16a3141119a4259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ademir=20Jos=C3=A9=20Ferreira=20J=C3=BAnior?= Date: Mon, 3 Aug 2026 15:08:32 -0300 Subject: [PATCH] Move the context popup onto its selection when the selection changes Radix registers a virtual anchor once per object identity, so the single anchor the popup held could never be re-measured: nothing short of a scroll or a resize moved it. A fresh identity per request is what asks floating-ui to measure again, and it lets the anchor read the request it was built for rather than a ref that existed to keep one object current. --- CHANGELOG.md | 1 + docs/specification.md | 2 +- .../components/EditorContextPopup.test.tsx | 72 +++++++++++++++++-- .../editor/components/EditorContextPopup.tsx | 56 +++++++-------- 4 files changed, 98 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61a2fa3..e95720e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Leafdown uses lightweight [Keep a Changelog](https://keepachangelog.com/en/1.1.0 - 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. +- Move the editor context popup onto the selection as it changes, instead of leaving it where it opened. - 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. - Announce recent files and recent folders under their own headings in the `Open recent` menu. diff --git a/docs/specification.md b/docs/specification.md index f4c0792..a89588c 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -178,7 +178,7 @@ The editor is a unified hybrid Markdown surface. Behavior is governed by renderi - `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. -- The popup anchors to the part of its selection that is visible in the document surface and follows that text as the document scrolls. Scrolling does not close the popup. +- The popup anchors to the part of its selection that is visible in the document surface and follows that text as the selection changes and as the document scrolls. Scrolling does not close the popup. - A selection taller than the visible area, or one that fills it, has no room beside it, so the popup sits inside the selection at its first visible line. - While no part of the selection is visible the popup is hidden rather than closed, and it returns when the selection scrolls back into view. - A popup opened from the keyboard, or holding focus for any other reason, stays visible and stays where it is. diff --git a/src/features/editor/components/EditorContextPopup.test.tsx b/src/features/editor/components/EditorContextPopup.test.tsx index 77cbb6b..4bd7169 100644 --- a/src/features/editor/components/EditorContextPopup.test.tsx +++ b/src/features/editor/components/EditorContextPopup.test.tsx @@ -3,7 +3,7 @@ import { describe, expect, it, vi, type Mock } from "vitest"; import { createActiveEditorCommandState, createEditorCommandState } from "@/test/factories/editor"; import { dispatchDOMEvent } from "@/test/utils/events"; -import { render, renderWithUser, screen, waitFor } from "@/test/utils/react"; +import { act, render, renderWithUser, screen, waitFor } from "@/test/utils/react"; import type { ContextPopupRequest } from "../plugins/contextPopup"; import type { ContextPopupAnchorMode } from "../utils/contextPopupAnchor"; @@ -20,6 +20,27 @@ const createAnchorRect = (top = 60): DOMRect => { const popperWrapper = () => document.querySelector("[data-radix-popper-content-wrapper]"); +// Radix parks the wrapper at a percentage translate until Floating UI has placed it, so a pixel +// offset is also the signal that a placement has happened. +const wrapperTranslateY = () => { + const transform = popperWrapper()?.style.transform ?? ""; + const placed = /translate\([^,]+,\s*(-?[\d.]+)px\)/u.exec(transform); + + if (!placed) { + throw new Error(`Expected a placed popper wrapper, got: ${transform || "no transform"}`); + } + + return Number(placed[1]); +}; + +const flushPlacement = () => + act( + () => + new Promise((resolve) => { + window.requestAnimationFrame(() => resolve()); + }), + ); + const ANCHOR = { contextElement: document.body, getRect: () => createAnchorRect() }; const POINTER_REQUEST: ContextPopupRequest = { anchor: ANCHOR, source: "pointer" }; const KEYBOARD_REQUEST: ContextPopupRequest = { anchor: ANCHOR, source: "keyboard" }; @@ -555,8 +576,11 @@ describe("EditorContextPopup", () => { }); describe("anchor mode", () => { - const renderWithSpiedAnchor = (source: ContextPopupRequest["source"]) => { - const getRect = vi.fn((_mode: ContextPopupAnchorMode) => createAnchorRect()); + const renderWithSpiedAnchor = ( + source: ContextPopupRequest["source"], + measure: () => DOMRect = () => createAnchorRect(), + ) => { + const getRect = vi.fn((_mode: ContextPopupAnchorMode) => measure()); const request = { anchor: { contextElement: document.body, getRect }, source }; const view = render( { expect(modesUsed(getRect)).toEqual(new Set(["pinned"])); }); + it("moves onto the selection when a fresh request measures it elsewhere", async () => { + const openedAt = 60; + const movedTo = 400; + let rect = createAnchorRect(openedAt); + const request: ContextPopupRequest = { + anchor: { contextElement: document.body, getRect: () => rect }, + source: "pointer", + }; + const renderPopup = () => ( + + ); + const view = render(renderPopup()); + + await waitFor(() => { + expect(wrapperTranslateY()).toEqual(expect.any(Number)); + }); + + const placedAt = wrapperTranslateY(); + + rect = createAnchorRect(movedTo); + view.rerender(renderPopup()); + + await waitFor(() => { + expect(wrapperTranslateY()).toBe(placedAt + movedTo - openedAt); + }); + }); + it("holds one rect for as long as focus stays inside the popup", async () => { - const { getRect, request, view } = renderWithSpiedAnchor("keyboard"); + let rect = createAnchorRect(60); + const { getRect, request, view } = renderWithSpiedAnchor("keyboard", () => rect); await waitFor(() => { expect(screen.getByLabelText("Cut")).toHaveFocus(); }); + const placedAt = wrapperTranslateY(); + + rect = createAnchorRect(400); + // A fresh request would otherwise re-measure; a popup being worked in must not move. view.rerender( { onReturnFocus={vi.fn()} />, ); + await flushPlacement(); expect(modesUsed(getRect)).toEqual(new Set(["pinned"])); expect(getRect).toHaveBeenCalledTimes(1); + expect(wrapperTranslateY()).toBe(placedAt); }); }); diff --git a/src/features/editor/components/EditorContextPopup.tsx b/src/features/editor/components/EditorContextPopup.tsx index 9e8a1ef..8d72462 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, useLayoutEffect, useRef, type KeyboardEvent } from "react"; +import { useEffect, useLayoutEffect, useMemo, useRef, type KeyboardEvent } from "react"; import { Button } from "@/components/ui/Button"; import { @@ -172,32 +172,34 @@ export function EditorContextPopup({ 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 requestRef = useRef(null); const pinnedRectRef = useRef(null); - // Radix reads the virtual anchor on every render and re-registers it whenever its identity - // changes, which would re-render this component in turn. It has to be created once. - const virtualRef = useRef({ - get contextElement() { - return requestRef.current?.anchor.contextElement; - }, - getBoundingClientRect: () => { - const currentRequest = requestRef.current; - - if (!currentRequest) { - return new DOMRect(); - } - - // A keyboard popup pins from the start rather than from the focus it is about to take, - // so it cannot hide in the moment between the two. - if (!hasHeldFocusRef.current && currentRequest.source !== "keyboard") { - return currentRequest.anchor.getRect("live"); - } - - pinnedRectRef.current ??= currentRequest.anchor.getRect("pinned"); - - return pinnedRectRef.current; - }, - }); + // Radix registers the anchor once per object identity and Floating UI measures only when it + // does, so scroll and resize aside, a fresh identity is the one thing that moves the popup + // onto a selection that has changed. Held against the render rather than created during one, + // or every unrelated render would re-register it. + const virtualRef = useMemo<{ current: VirtualAnchor }>( + () => ({ + current: { + contextElement: request?.anchor.contextElement, + getBoundingClientRect: () => { + if (!request) { + return new DOMRect(); + } + + // A keyboard popup pins from the start rather than from the focus it is about to take, + // so it cannot hide in the moment between the two. + if (!hasHeldFocusRef.current && request.source !== "keyboard") { + return request.anchor.getRect("live"); + } + + pinnedRectRef.current ??= request.anchor.getRect("pinned"); + + return pinnedRectRef.current; + }, + }, + }), + [request], + ); const canExecute = (commandId: EditorCommandId) => isCommandEnabled(commandId, commandState); const releaseHeldFocus = () => { hasHeldFocusRef.current = false; @@ -207,8 +209,6 @@ export function EditorContextPopup({ // Layout is early enough: Radix registers the anchor from a passive effect, and Floating UI // measures later still. useLayoutEffect(() => { - requestRef.current = request; - // A popup the user is working in keeps the rect it was pinned to. if (!hasHeldFocusRef.current) { pinnedRectRef.current = null;