Skip to content

feat(demos/custom-ui): consume new SD-2936 controller surfaces#3152

Merged
caio-pizzol merged 15 commits intomainfrom
caio/sd-2936-demo-update
May 5, 2026
Merged

feat(demos/custom-ui): consume new SD-2936 controller surfaces#3152
caio-pizzol merged 15 commits intomainfrom
caio/sd-2936-demo-update

Conversation

@caio-pizzol
Copy link
Copy Markdown
Contributor

Updates the demos/custom-ui reference 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 into superdoc.activeEditor.presentationEditor and data-* attributes to ship a custom UI).

What lands in the demo:

  • SelectionPopover — floating bubble menu over the user's selection, anchored via ui.selection.getAnchorRect({ placement: 'start' }). No more window.getSelection().getBoundingClientRect().
  • ContextMenu + ContextMenuRegistrations — right-click handler calls ui.viewport.entityAt({ x, y }) for the entities under the click and ui.commands.getContextMenuItems({ entities }) for the contributed items. Three example registrations (Accept suggestion, Reject suggestion, Resolve comment) declare contextMenu: { when: ({ entities }) => ... } so each item only shows up when the click target matches. <SuperDocEditor disableContextMenu /> suppresses the built-in.
  • InsertClauseButton — registration carries shortcut: 'Mod-Shift-C'. Pressing it opens the menu; clicking a clause from the menu still inserts directly. No separate keydown wiring.
  • CommentComposer — calls ui.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.
  • README — adds a "custom-UI recipe" section documenting the four patterns above with file pointers, and drops the old "no floating bubble menu" caveat.

Stacks on PR #3149. Once that merges this rebases onto main directly.

Verified: pnpm --filter custom-ui run build → clean (after the prerequisite pnpm --filter @superdoc/super-editor run build and pnpm --filter superdoc run build:es).

@caio-pizzol caio-pizzol requested a review from a team as a code owner May 5, 2026 11:36
@linear
Copy link
Copy Markdown

linear Bot commented May 5, 2026

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread demos/custom-ui/src/components/InsertClauseButton.tsx
Base automatically changed from caio/sd-2936-shortcuts to main May 5, 2026 11:49
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

caio-pizzol added 11 commits May 5, 2026 16:48
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.
@caio-pizzol caio-pizzol force-pushed the caio/sd-2936-demo-update branch from 275dc05 to ef78262 Compare May 5, 2026 19:49
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.
@caio-pizzol caio-pizzol merged commit c895165 into main May 5, 2026
27 checks passed
@caio-pizzol caio-pizzol deleted the caio/sd-2936-demo-update branch May 5, 2026 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants