feat(demos/custom-ui): consume new SD-2936 controller surfaces#3152
Merged
caio-pizzol merged 15 commits intomainfrom May 5, 2026
Merged
feat(demos/custom-ui): consume new SD-2936 controller surfaces#3152caio-pizzol merged 15 commits intomainfrom
caio-pizzol merged 15 commits intomainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f4f9792be
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
c79f86a to
275dc05
Compare
…d state as the button
Replaces the manual entityAt + selection-rect hit-test + payload-
threading stitching with the SD-2945 right-click context bundle:
- ContextMenu opens against `ui.viewport.contextAt({ x, y })` and
dispatches each pick via `item.invoke()`. Drops the local
isPointInsideSelectionRects helper, the `commands.get(id).execute({
entities })` payload threading, and the legacy
`getContextMenuItems({ entities })` shape.
- Listener scope swaps the `.editor-shell` closest() check for
`ui.viewport.getHost()`.
- ContextMenuRegistrations move the "click inside selection" gate from
the menu component into each `when` predicate via
`insideSelection === true`. Handlers read the click subject from
`context.entities` / `context.selection` instead of unpacking a
payload, so predicate and handler always see the same shape.
README updates strip the SD-2936 follow-up caveat (positionAt landed
in SD-2943, contextAt in SD-2945) and walk consumers through the new
single-call recipe. The "deliberately empty for plain text" warning
is gone; the menu now stays honest because contextAt's bundle carries
the click point through to the handler.
Registers demo.insertClauseHere as the canonical SD-2945 demonstration: the predicate gates on position !== null && insideSelection !== true (plain caret-only clicks) and the handler reads context.position.target straight from the bundle, so the insert lands at the click point regardless of the user's prior selection somewhere else in the doc. Without the bundle, the same registration would have to dispatch against state.selection.selectionTarget and silently misroute. README updates: - "right-click anywhere to see the menu" is now actually true; the three-surfaces table lists the new item. - Notes that empty getContextMenuItems(context) results fall through to the browser native menu via SD-2944's disableContextMenu fix rather than producing a dead right-click. - Links the canonical SD-2945 example to the new registration.
275dc05 to
ef78262
Compare
ContextMenuRegistrations re-registered all six contributed commands
on every AppInner render because the props it received were unstable:
useDecidedChanges returned a fresh { decidedChanges, decideChange }
object literal each call, and App.tsx threaded inline arrow
() => setComposeOpen(true) through three children. Every track-change
update or composer toggle re-fired the effect and churned the
registry. The SD-2945 invoke-identity guard kept correctness intact,
but a demo that's the canonical example for consumers to copy
shouldn't teach "register inside an effect with unstable deps."
useMemo the useDecidedChanges return value so consumers passing it
into effect deps see a stable reference; useCallback the open/close
composer handlers in App.tsx for the same reason.
ContextMenu.tsx JSDoc and inline scope comment also drifted out of
sync with the SD-2944 / SD-2945 modernization. JSDoc now describes
the empty-result fall-through to the browser native menu, and the
scope comment talks about contextAt + getHost instead of the removed
entityAt-based scope check.
…SD-2945) The predicate gated only on position !== null && insideSelection !== true, but a right-click on a tracked change or comment also has position !== null (the click is over document text) and insideSelection === false. The item leaked into the entity menus alongside Accept/Reject/Resolve, and selecting it would insert into the entity's range instead of acting on the entity. Add entities.length === 0 to the predicate so the item only shows on plain caret-only text, matching the README's "click target = subject" rule. Each menu surface stays scoped to its intended subject: entity-scoped items on entity hits, selection-scoped items inside the selection, point-anchored insert only when neither matches.
The bundled sample-review.docx embedded a real human's name and an internal-looking AD identifier across word/comments.xml, word/document.xml, word/people.xml, and docProps/core.xml: w:author="Caio Pizzol" w:initials="CP" w15:author="Caio Pizzol" w15:userId="S::caio@harbourclients.com::6fd3240b-..." dc:creator / cp:lastModifiedBy Re-zipped with neutral metadata so the public demo doesn't ship employee identity or internal domain references when a reader opens the sample in Word: w:author="Reviewer" w:initials="R" w15:author="Reviewer" w15:userId="S::reviewer@example.com::00000000-..." dc:creator="Reviewer", cp:lastModifiedBy="Reviewer"
Tightens the demo before making it the canonical custom-UI example. Five threads in one pass: 1. Viewport clamping + a11y on the floating menus. ContextMenu and SelectionPopover both placed themselves at raw mouse / rect coords and could render off-screen near viewport edges. Both now measure their rendered size in useLayoutEffect, clamp to the viewport, and in the popover's case flip below the selection when there's no room above. Visibility is hidden for the single frame between layout and clamp so neither flashes at the unclamped coords. ContextMenu also gains role="menu" + role="menuitem", initial focus on open, and Up/Down/Home/End keyboard navigation. 2. Remove ticket-ID references from public-facing code and prose. Linear IDs (SD-2816, SD-2839, SD-2944, SD-2945, SD-2954, SD-2936) appeared in JSDoc and the README. Public readers can't open them. Replaced with behavior-only descriptions. 3. Stable callback identity in useDecidedChanges. decideChange's useCallback deps included trackChanges.items, so the wrapper object's identity churned on every doc edit. Mirror the items in a ref instead, so decideChange and the memoized wrapper only change when decidedChanges does. 4. Naming consistency. Custom-command id 'company.insertClause' renamed to 'demo.insertClause' to match the rest of the demo's 'demo.*' namespace. 5. Dead code + small fixes. Stray empty fragment in App.tsx wrapping a single child removed. README rewritten: - Drops em-dashes per the writing-style rule. - Strips the SD-* ticket references throughout. - Drops the forward-reference to an `examples/` folder that doesn't exist. - Fixes overstated claims: the toolbar / bubble-menu / right-click table now lists the items actually in the demo (no nonexistent "review nav", no "Link / Copy" in the bubble menu, no SD-2944 / SD-2945 / SD-2936 cross-references). - Trims the merged-activity-feed code block to a one-paragraph pointer at ActivitySidebar.tsx instead of an inline 18-line code sample. - Adds a prerequisites line above the run commands. - Folds the Telemetry footer into "What this demo deliberately doesn't do". Net README is ~30% shorter (109 -> 87 lines) and accurate against the current source.
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
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.
Updates the
demos/custom-uireference workspace to consume every new controller surface from the SD-2936 stack. End-to-end proof that the platform changes actually solve the original problem (consumers reaching intosuperdoc.activeEditor.presentationEditoranddata-*attributes to ship a custom UI).What lands in the demo:
SelectionPopover— floating bubble menu over the user's selection, anchored viaui.selection.getAnchorRect({ placement: 'start' }). No morewindow.getSelection().getBoundingClientRect().ContextMenu+ContextMenuRegistrations— right-click handler callsui.viewport.entityAt({ x, y })for the entities under the click andui.commands.getContextMenuItems({ entities })for the contributed items. Three example registrations (Accept suggestion,Reject suggestion,Resolve comment) declarecontextMenu: { when: ({ entities }) => ... }so each item only shows up when the click target matches.<SuperDocEditor disableContextMenu />suppresses the built-in.InsertClauseButton— registration carriesshortcut: 'Mod-Shift-C'. Pressing it opens the menu; clicking a clause from the menu still inserts directly. No separate keydown wiring.CommentComposer— callsui.selection.restore(captured)on both successful post and cancel, so the user sees the editor's visible selection rejoin where it was when they opened the composer.Stacks on PR #3149. Once that merges this rebases onto main directly.
Verified:
pnpm --filter custom-ui run build→ clean (after the prerequisitepnpm --filter @superdoc/super-editor run buildandpnpm --filter superdoc run build:es).