refactor(board): rebuild canvas context menu on Radix menu primitives - #247
Merged
Conversation
The context menu was a hand-rolled fixed-position div: it placed itself at the raw click point with no collision handling (so it clipped off-screen near edges) and rendered AI/Translate as inline accordions rather than submenus. Rebuild it on the real Radix menu (ui/dropdown-menu): the canvas-aware right-click detection (selection-gated, skips editor inputs) now sets an anchor point, and a controlled menu opens against a zero-size trigger there — so positioning collides-and-flips against the viewport, and AI and Translate become proper hover submenus. Also promotes the transparent-bg toggle to a checkbox item. No change to the transform/export behavior.
- Re-key the menu content on the anchor point so right-clicking a new location while the menu is already open re-anchors it there (moving the zero-size anchor's CSS position alone doesn't trigger a re-measure). - Suppress the native browser menu inside the portaled content + submenus (they live outside the canvas wrap, so its contextmenu guard misses them). - Drop aria-hidden from the trigger: Radix projects focusable button semantics onto it, and a focusable aria-hidden node is an a11y violation.
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
1 participant
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.
Problem
The canvas right-click menu was a hand-rolled fixed-position
<div>:style={{ top, left }}) with no collision handling, so right-clicking near the right/bottom edge left the menu partly off-screen.It also hand-rolled outside-click, Escape, and
role="menu".Change
Rebuild it on the real Radix menu already in the kit (
ui/dropdown-menu, backed by@radix-ui/react-dropdown-menu, used in several other places).The canvas owns its pointer events, so a Radix
ContextMenuTriggercan't overlay it. Instead I kept the canvas-aware right-click detection (selection-gated, skips editor inputs) and used it to set an anchor point; a controlled menu opens against a zero-size trigger placed there. Radix then handles:Also: the "Transparent background" On/Off toggle is now a proper checkbox item; the Translate custom-language input lives in the Translate submenu with keydown/pointer guards so Radix's menu typeahead doesn't eat typing.
Unchanged
All behavior: selection gating, input-skip, the four position actions, PNG/SVG export, and every AI/Translate action (local transforms + backend path) are byte-for-byte the same handlers — this is a presentation refactor.
Testing
check-allclean. No automated test: the component pulls in react-query, the router, and several stores, so a render test would need heavy, brittle mocking for a purely presentational change (the original had no test either). Verified manually: menu opens at the cursor, flips near edges, AI/Translate open as submenus, actions still fire.Related (separate, not in this PR)
The transform "works but sometimes not" is a prompt/thin-input matter plus the empty-content guard flagged earlier — to be handled in its own change.