Skip to content

improvement(rich-md-editor): stabilize bubble-menu plugin key + comment cleanup - #5158

Merged
waleedlatif1 merged 2 commits into
stagingfrom
improvement/rich-md-editor-cleanup
Jun 20, 2026
Merged

improvement(rich-md-editor): stabilize bubble-menu plugin key + comment cleanup#5158
waleedlatif1 merged 2 commits into
stagingfrom
improvement/rich-md-editor-cleanup

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Stabilize the bubble menu's PluginKey (useState lazy-init instead of useMemo, which React may recompute and break setMeta-based reveal)
  • Move inline rationale into TSDoc and drop leftover // comments across the rich-markdown-editor files (per style guide: TSDoc only)

Follow-up polish on top of the rich markdown editor work merged in #5148. No behavior change beyond the plugin-key stability hardening.

Type of Change

  • Improvement / refactor

Testing

  • Typecheck clean, biome clean
  • 303 vitest + 28 bubble/streaming/hover e2e green (incl. link-via-bubble setMeta reveal and the agent-lock editability test)

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercelBot commented Jun 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
docsSkippedSkippedJun 20, 2026 10:44pm

Request Review

@cursor

cursorBot commented Jun 20, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Editor UX hardening and documentation-only edits; the plugin-key change reduces risk of a wedged toolbar rather than introducing new logic paths.

Overview
Polish on the rich markdown editor bubble menu and related files: stabilizes the TipTap bubble menu PluginKey by switching from useMemo to lazy useState init so React cannot recreate the key and break setMeta-based reveal after drag-select (including link-via-bubble flows).

Also extracts revealBubbleMenu (show then updatePosition) and moves scattered // rationale into TSDoc in extensions.ts (pipe-only table escaping / CodeQL note), bubble-menu.tsx, and drops redundant comments in rich-markdown-editor.tsx and global-commands-provider.tsx. No other intentional behavior changes.

Reviewed by Cursor Bugbot for commit 27901e4. Configure here.

@waleedlatif1

Copy link
Copy Markdown
CollaboratorAuthor

@cursor review

@waleedlatif1

Copy link
Copy Markdown
CollaboratorAuthor

@greptile

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit e3e702a. Configure here.

@greptile-apps

greptile-appsBot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hardens the bubble menu's PluginKey stability by switching from useMemo (which React is permitted to discard and recompute, breaking setMeta-based reveal) to useState lazy initialization, and does comment cleanup by converting inline // rationale to TSDoc across the rich-markdown-editor files.

  • bubble-menu.tsx: useMemo(() => new PluginKey(...), [])useState(() => new PluginKey(...)) to guarantee a single stable instance; the two-step reveal is extracted into a revealBubbleMenu helper whose TSDoc documents the required showupdatePosition ordering.
  • extensions.ts: CodeQL false-positive explanation moved from an inline // comment into the block TSDoc on PipeSafeTable.
  • rich-markdown-editor.tsx / global-commands-provider.tsx: Single leftover // comments removed; no behavioral change.

Confidence Score: 5/5

Safe to merge — the only behavioral change is the PluginKey stabilization, which is a correct fix, and the rest is comment migration with no logic touched.

The useState lazy-init swap is the right approach for a stable singleton that must survive React's ability to discard and recreate useMemo values. The revealBubbleMenu extraction preserves the existing two-step sequencing intact. All other edits are comment moves or deletions with zero logic change.

No files require special attention.

Important Files Changed

FilenameOverview
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/menus/bubble-menu.tsxCore change: useMemouseState lazy-init for PluginKey to prevent React from discarding and recomputing the instance, which would silently break setMeta-based reveal. The two-step reveal is extracted into revealBubbleMenu with TSDoc documenting the required showupdatePosition ordering.
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/extensions.tsInline // comment explaining the CodeQL false-positive on backslash escaping moved into the block TSDoc on PipeSafeTable. No behavioral change.
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsxSingle inline comment removed from data-owned-shortcuts attribute. No behavioral change.
apps/sim/app/workspace/[workspaceId]/providers/global-commands-provider.tsxSingle inline comment removed from focusedElementOwnsShortcut call. No behavioral change.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant EditorDOM as Editor DOM
participant Window
participant BubbleMenu as BubbleMenu Component
participant Plugin as ProseMirror Plugin
User->>EditorDOM: mousedown
EditorDOM->>BubbleMenu: "onPointerDown sets isPointerDownRef=true"
Note over Plugin: shouldShow returns false
User->>Window: mouseup
Window->>BubbleMenu: onPointerUp
BubbleMenu->>BubbleMenu: "isPointerDownRef=false"
alt has formattable selection
BubbleMenu->>Plugin: setMeta(key, show)
BubbleMenu->>Plugin: setMeta(key, updatePosition)
Plugin->>User: bubble menu anchored at selection
end
User->>Window: blur
Window->>BubbleMenu: "onWindowBlur sets isPointerDownRef=false"
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant EditorDOM as Editor DOM
participant Window
participant BubbleMenu as BubbleMenu Component
participant Plugin as ProseMirror Plugin
User->>EditorDOM: mousedown
EditorDOM->>BubbleMenu: "onPointerDown sets isPointerDownRef=true"
Note over Plugin: shouldShow returns false
User->>Window: mouseup
Window->>BubbleMenu: onPointerUp
BubbleMenu->>BubbleMenu: "isPointerDownRef=false"
alt has formattable selection
BubbleMenu->>Plugin: setMeta(key, show)
BubbleMenu->>Plugin: setMeta(key, updatePosition)
Plugin->>User: bubble menu anchored at selection
end
User->>Window: blur
Window->>BubbleMenu: "onWindowBlur sets isPointerDownRef=false"
Loading

Reviews (3): Last reviewed commit: "docs(rich-md-editor): preserve bubble re..." | Re-trigger Greptile

@greptile-apps

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hardens the bubble menu's PluginKey stability by replacing useMemo with useState lazy initialization, and cleans up inline // comments by either migrating them to TSDoc /** */ blocks or dropping them where the code is self-explanatory.

  • useMemouseState for bubbleMenuKey: useMemo is a performance hint in React's concurrent model — React may discard and recompute it, creating a new PluginKey instance that would silently break any in-flight setMeta('show') or setMeta('updatePosition') calls. useState with a lazy initializer guarantees the key is created exactly once per component mount, which is the required contract for ProseMirror plugin identity.
  • Comment migration: Inline rationale that explained why constants exist (e.g. FLOATING_OPTIONS, APPEND_TO_BODY) is promoted to TSDoc; function-body implementation notes that described non-obvious two-step setMeta sequencing and viewport-clamping logic in resolveAnchor are removed entirely.
  • extensions.ts: CodeQL false-positive explanation is moved from an inline comment inside the renderMarkdown callback into the surrounding TSDoc block.

Confidence Score: 5/5

Safe to merge — the only behavioral change is swapping useMemo for useState lazy-init on the plugin key, which closes a real (if low-probability) silent failure path in React concurrent rendering; all other edits are comment reorganization with no runtime effect.

The useMemouseState fix is correct and strictly improves correctness. Comment cleanup is cosmetic. No logic was removed, no new state was introduced, and the two-step setMeta sequencing (show then updatePosition) is preserved exactly as before.

bubble-menu.tsx is the only file with a substantive change; the non-trivial two-step setMeta and anchorCacheRef viewport-clamping logic now lack inline documentation.

Important Files Changed

FilenameOverview
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/menus/bubble-menu.tsxCore change: useMemouseState lazy-init for bubbleMenuKey is correct and necessary — useMemo is not a stability guarantee, and a recomputed key would silently break setMeta-based reveal. Several non-trivial implementation comments (two-step setMeta rationale, viewport-clamping explanation) are removed without TSDoc replacement.
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/extensions.tsCodeQL false-positive explanation moved from inline comment into the TSDoc block above PipeSafeTable — no logic change.
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsxOne inline comment about claiming Mod+K for the editor removed — no logic change.
apps/sim/app/workspace/[workspaceId]/providers/global-commands-provider.tsxOne inline comment about the rich-editor shortcut ownership check removed — no logic change.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant DOM
participant BubbleMenu
participant ProseMirror
User->>DOM: mousedown (drag start)
DOM->>BubbleMenu: "onPointerDown → isPointerDownRef = true"
Note over BubbleMenu: shouldShow returns false (pointer is down)
User->>DOM: mouseup (drag end)
DOM->>BubbleMenu: onPointerUp
BubbleMenu->>BubbleMenu: hasFormattableSelection()?
alt has selection
BubbleMenu->>ProseMirror: setMeta(bubbleMenuKey, 'show')
BubbleMenu->>ProseMirror: setMeta(bubbleMenuKey, 'updatePosition')
Note over ProseMirror: bubbleMenuKey is stable (useState lazy-init)
ProseMirror->>BubbleMenu: plugin receives both metas
BubbleMenu->>User: toolbar visible + anchored
else no selection
Note over BubbleMenu: toolbar stays hidden
end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant DOM
participant BubbleMenu
participant ProseMirror
User->>DOM: mousedown (drag start)
DOM->>BubbleMenu: "onPointerDown → isPointerDownRef = true"
Note over BubbleMenu: shouldShow returns false (pointer is down)
User->>DOM: mouseup (drag end)
DOM->>BubbleMenu: onPointerUp
BubbleMenu->>BubbleMenu: hasFormattableSelection()?
alt has selection
BubbleMenu->>ProseMirror: setMeta(bubbleMenuKey, 'show')
BubbleMenu->>ProseMirror: setMeta(bubbleMenuKey, 'updatePosition')
Note over ProseMirror: bubbleMenuKey is stable (useState lazy-init)
ProseMirror->>BubbleMenu: plugin receives both metas
BubbleMenu->>User: toolbar visible + anchored
else no selection
Note over BubbleMenu: toolbar stays hidden
end
Loading

Reviews (2): Last reviewed commit: "improvement(rich-md-editor): stabilize t..." | Re-trigger Greptile

@waleedlatif1

Copy link
Copy Markdown
CollaboratorAuthor

@cursor review

@waleedlatif1

Copy link
Copy Markdown
CollaboratorAuthor

@greptile

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 27901e4. Configure here.

@waleedlatif1
waleedlatif1 merged commit 2bbf70e into stagingJun 20, 2026
16 checks passed
@waleedlatif1
waleedlatif1 deleted the improvement/rich-md-editor-cleanup branch June 20, 2026 22:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1