[WIKI-638] fix: peek overview closing while dropdowns are open#7841
[WIKI-638] fix: peek overview closing while dropdowns are open#7841
Conversation
aaryan610
added
🐛bug
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds active-dropbar tracking to the editor (commands + storage + types), exposes EditorRefApi.isAnyDropbarOpen(), updates slash-commands and table drag-handle flows to register/unregister active dropbar extensions, and threads an editorRef into peek-overview components to consult dropbar state for outside-click/keydown logic. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant PeekView as PeekOverview View
participant Detail as PeekOverviewIssueDetails
participant EditorRef as EditorRefApi
participant Utility as Utility Storage
User->>PeekView: click / keydown while peek open
PeekView->>EditorRef: isAnyDropbarOpen()
EditorRef->>Utility: read activeDropbarExtensions
Utility-->>EditorRef: hasEntries? (true/false)
EditorRef-->>PeekView: true / false
alt dropbar open (true)
PeekView->>User: ignore outside-click / keydown close
else no dropbar (false)
PeekView->>User: proceed with route removal / close
end
PeekView->>Detail: render with `editorRef`
Note right of Detail: Detail may also call `editorRef.isAnyDropbarOpen()`
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where the peek overview closes unexpectedly when clicking outside or pressing the Esc key while editor dropdowns (table, slash commands) are still open.
- Introduces a new
isAnyDropbarOpenmethod to the editor API that checks for active dropdowns - Tracks dropdown state using the utility extension storage for table and slash command dropdowns
- Updates peek overview close logic to prevent closing when dropdowns are active
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/editor/src/core/types/editor.ts | Adds isAnyDropbarOpen method to the EditorRefApi type definition |
| packages/editor/src/core/helpers/editor-ref.ts | Implements the isAnyDropbarOpen helper method using utility storage |
| packages/editor/src/core/extensions/utility.ts | Extends dropdown tracking types to include table and slash commands |
| packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx | Tracks row dropdown state and adds outside click prevention attributes |
| packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx | Tracks column dropdown state and adds outside click prevention attributes |
| packages/editor/src/core/extensions/slash-commands/root.tsx | Tracks slash command dropdown state in utility storage |
| apps/web/core/components/issues/peek-overview/view.tsx | Uses editor ref to check dropdown state before closing peek overview |
| apps/web/core/components/issues/peek-overview/issue-detail.tsx | Receives editor ref as prop instead of creating its own |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/editor/src/core/extensions/utility.ts (1)
14-19: Consider adding type safety for activeDropbarExtensions array operations.The expanded
TActiveDropbarExtensionstype now includes more extension types, but the array operations in the drag handle components useindexOfandsplicewithout type checking. Consider adding helper functions to safely manage this array.Consider adding type-safe helper functions for managing the activeDropbarExtensions array:
// In packages/editor/src/core/helpers/extension-storage-helpers.ts export const addActiveDropbarExtension = ( storage: UtilityExtensionStorage, extension: TActiveDropbarExtensions ): void => { if (!storage.activeDropbarExtensions.includes(extension)) { storage.activeDropbarExtensions.push(extension); } }; export const removeActiveDropbarExtension = ( storage: UtilityExtensionStorage, extension: TActiveDropbarExtensions ): void => { const index = storage.activeDropbarExtensions.indexOf(extension); if (index !== -1) { storage.activeDropbarExtensions.splice(index, 1); } };packages/editor/src/core/extensions/slash-commands/root.tsx (3)
63-68: Avoid duplicate entries when marking SLASH_COMMANDS active.Prevents multiple pushes which otherwise require multiple exits to clean up.
Apply this diff:
- // Track active dropdown - getExtensionStorage(props.editor, CORE_EXTENSIONS.UTILITY).activeDropbarExtensions.push( - CORE_EXTENSIONS.SLASH_COMMANDS - ); + // Track active dropdown + const utilityStorage = getExtensionStorage(props.editor, CORE_EXTENSIONS.UTILITY); + if (!utilityStorage.activeDropbarExtensions.includes(CORE_EXTENSIONS.SLASH_COMMANDS)) { + utilityStorage.activeDropbarExtensions.push(CORE_EXTENSIONS.SLASH_COMMANDS); + }
94-103: Also clear active state on Escape to avoid leaks.If Suggestion doesn’t call onExit after hiding, the active flag can stick.
Apply this diff:
onKeyDown: (props) => { if (props.event.key === "Escape") { + try { + const utilityStorage = getExtensionStorage(props.editor, CORE_EXTENSIONS.UTILITY); + let idx = utilityStorage.activeDropbarExtensions.lastIndexOf(CORE_EXTENSIONS.SLASH_COMMANDS); + while (idx > -1) { + utilityStorage.activeDropbarExtensions.splice(idx, 1); + idx = utilityStorage.activeDropbarExtensions.lastIndexOf(CORE_EXTENSIONS.SLASH_COMMANDS); + } + } catch {} popup?.[0].hide(); return true; }
60-62: Typepopupcorrectly and drop the ts-expect-error (optional).
tippy("body", ...)returns an array of instances. Usedocument.bodyfor correct typing and declarepopupasInstance[].Apply this diff:
- let popup: Instance | null = null; + let popup: Instance[] | null = null;- // @ts-expect-error - Tippy types are incorrect - popup = tippy("body", { + popup = tippy(document.body, { getReferenceClientRect: props.clientRect, appendTo: tippyContainer, content: component.element, showOnCreate: true, interactive: true, trigger: "manual", placement: "bottom-start", });Also applies to: 76-85
apps/web/core/components/issues/peek-overview/view.tsx (2)
83-91: Coerce dropbar check to boolean; optionally gate on editor fullscreen modal too.Prevents undefined from bypassing the guard; consider also ignoring outside-click when the editor image fullscreen modal is open.
Apply this diff:
usePeekOverviewOutsideClickDetector( issuePeekOverviewRef, () => { - const isAnyDropbarOpen = editorRef.current?.isAnyDropbarOpen(); + const isAnyDropbarOpen = !!editorRef.current?.isAnyDropbarOpen(); + const editorImageFullScreenModalElement = document.querySelector(".editor-image-full-screen-modal"); if (!embedIssue) { - if (!isAnyModalOpen && !isAnyEpicModalOpen && !isAnyLocalModalOpen && !isAnyDropbarOpen) { + if ( + !isAnyModalOpen && + !isAnyEpicModalOpen && + !isAnyLocalModalOpen && + !isAnyDropbarOpen && + !editorImageFullScreenModalElement + ) { removeRoutePeekId(); } } }, issueId );
98-104: Coerce dropbar check to boolean in Esc handler.Ensures consistent gating.
Apply this diff:
- const isAnyDropbarOpen = editorRef.current?.isAnyDropbarOpen(); - if (!isAnyModalOpen && !dropdownElement && !isAnyDropbarOpen && !editorImageFullScreenModalElement) { + const isAnyDropbarOpen = !!editorRef.current?.isAnyDropbarOpen(); + if (!isAnyModalOpen && !dropdownElement && !isAnyDropbarOpen && !editorImageFullScreenModalElement) { removeRoutePeekId();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/core/components/issues/peek-overview/issue-detail.tsx(2 hunks)apps/web/core/components/issues/peek-overview/view.tsx(6 hunks)packages/editor/src/core/extensions/slash-commands/root.tsx(4 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx(5 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx(5 hunks)packages/editor/src/core/extensions/utility.ts(1 hunks)packages/editor/src/core/helpers/editor-ref.ts(1 hunks)packages/editor/src/core/types/editor.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (2)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)packages/editor/src/index.ts (1)
CORE_EXTENSIONS(16-16)
packages/editor/src/core/extensions/slash-commands/root.tsx (2)
packages/editor/src/core/extensions/slash-commands/command-menu.tsx (1)
SlashCommandsMenuProps(10-14)packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx (2)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)packages/editor/src/index.ts (1)
CORE_EXTENSIONS(16-16)
packages/editor/src/core/helpers/editor-ref.ts (1)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)
apps/web/core/components/issues/peek-overview/issue-detail.tsx (1)
packages/editor/src/core/types/editor.ts (1)
EditorRefApi(99-138)
apps/web/core/components/issues/peek-overview/view.tsx (2)
packages/editor/src/core/types/editor.ts (1)
EditorRefApi(99-138)apps/web/core/store/issue/issue-details/root.store.ts (1)
isAnyModalOpen(227-238)
packages/editor/src/core/extensions/utility.ts (1)
packages/editor/src/ce/types/utils.ts (1)
TAdditionalActiveDropbarExtensions(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (10)
packages/editor/src/core/types/editor.ts (1)
122-122: LGTM!The addition of the
isAnyDropbarOpenmethod to theEditorRefApiinterface is appropriate and aligns well with the PR's objective to fix the peek overview closing issue. The method signature follows the existing pattern and provides a clean way to query dropdown state.packages/editor/src/core/helpers/editor-ref.ts (1)
84-88: LGTM! Clean implementation of dropbar state check.The implementation correctly:
- Guards against a missing editor instance
- Retrieves the utility storage using the helper function
- Returns a boolean based on the presence of active dropbar extensions
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (1)
206-206: Good use of data-prevent-outside-click attribute.The addition of
data-prevent-outside-clickto both theFloatingOverlayand the dropdown container properly prevents unintended closures when interacting with dropdown content.Also applies to: 216-216
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx (2)
205-205: Good use of data-prevent-outside-click attribute.The addition of
data-prevent-outside-clickattributes maintains consistency with the column drag handle implementation.Also applies to: 215-215
164-177: Extract duplicate dropbar tracking logic into a shared hook.This is identical to the logic in
column/drag-handle.tsx. As mentioned in the column drag handle review, this should be extracted into a shared custom hook.packages/editor/src/core/extensions/utility.ts (2)
23-23: Good use of dynamic key for Commands augmentation.Using
[CORE_EXTENSIONS.UTILITY]as a computed property key instead of a hardcoded string "utility" improves maintainability and type safety.
14-19: Import path is correct — no action needed.
tsconfig (packages/editor/tsconfig.json) maps "@/plane-editor/" → ./src/ce/, so "@/plane-editor/types/utils" resolves to packages/editor/src/ce/types/utils.ts.apps/web/core/components/issues/peek-overview/issue-detail.tsx (2)
173-176: Restore via editorRef LGTM.Using
setEditorValue(descriptionHTML, true)matches EditorRefApi.
32-42: Prop addition LGTM — verify all call sites are updated.Automated search in the sandbox failed (rg errored on --type=tsx and returned no matches); ensure every <PeekOverviewIssueDetails ...> invocation passes the new required
editorRefprop. To re-check locally: rg -n -S '<PeekOverviewIssueDetails' .apps/web/core/components/issues/peek-overview/view.tsx (1)
171-181: Passing editorRef down LGTM.Both layout branches correctly propagate
editorReftoPeekOverviewIssueDetails.Also applies to: 213-222
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/editor/src/core/extensions/slash-commands/root.tsx (1)
63-67: Deduplicate storage push to avoid stale open-state.Guard against duplicate entries when the menu re-initializes.
Apply this diff:
- // Track active dropdown - getExtensionStorage(props.editor, CORE_EXTENSIONS.UTILITY).activeDropbarExtensions.push( - CORE_EXTENSIONS.SLASH_COMMANDS - ); + // Track active dropdown (dedup) + const utilityStorage = getExtensionStorage(props.editor, CORE_EXTENSIONS.UTILITY); + if (!utilityStorage.activeDropbarExtensions.includes(CORE_EXTENSIONS.SLASH_COMMANDS)) { + utilityStorage.activeDropbarExtensions.push(CORE_EXTENSIONS.SLASH_COMMANDS); + }packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (1)
164-177: Add unmount cleanup to avoid stuck TABLE entry.If the component unmounts while open,
TABLEcan remain inactiveDropbarExtensions.Apply this diff:
useEffect(() => { const utilityStorage = getExtensionStorage(editor, CORE_EXTENSIONS.UTILITY); const index = utilityStorage.activeDropbarExtensions.indexOf(CORE_EXTENSIONS.TABLE); if (isDropdownOpen) { if (index === -1) { utilityStorage.activeDropbarExtensions.push(CORE_EXTENSIONS.TABLE); } } else { if (index !== -1) { utilityStorage.activeDropbarExtensions.splice(index, 1); } } - }, [editor, isDropdownOpen]); + return () => { + const i = utilityStorage.activeDropbarExtensions.indexOf(CORE_EXTENSIONS.TABLE); + if (i !== -1) utilityStorage.activeDropbarExtensions.splice(i, 1); + }; + }, [editor, isDropdownOpen]);packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx (1)
163-177: Add unmount cleanup to avoid stuck TABLE entry.Mirror the column handle cleanup to prevent stale open-state.
Apply this diff:
useEffect(() => { const utilityStorage = getExtensionStorage(editor, CORE_EXTENSIONS.UTILITY); const index = utilityStorage.activeDropbarExtensions.indexOf(CORE_EXTENSIONS.TABLE); if (isDropdownOpen) { if (index === -1) { utilityStorage.activeDropbarExtensions.push(CORE_EXTENSIONS.TABLE); } } else { if (index !== -1) { utilityStorage.activeDropbarExtensions.splice(index, 1); } } - }, [editor, isDropdownOpen]); + return () => { + const i = utilityStorage.activeDropbarExtensions.indexOf(CORE_EXTENSIONS.TABLE); + if (i !== -1) utilityStorage.activeDropbarExtensions.splice(i, 1); + }; + }, [editor, isDropdownOpen]);apps/web/core/components/issues/peek-overview/view.tsx (1)
85-88: Use a default false to avoid undefined checks.Minor clarity/readability improvement.
Apply this diff:
- const isAnyDropbarOpen = editorRef.current?.isAnyDropbarOpen(); + const isAnyDropbarOpen = editorRef.current?.isAnyDropbarOpen() ?? false;- const isAnyDropbarOpen = editorRef.current?.isAnyDropbarOpen(); + const isAnyDropbarOpen = editorRef.current?.isAnyDropbarOpen() ?? false;Also applies to: 98-100
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/core/components/issues/peek-overview/issue-detail.tsx(2 hunks)apps/web/core/components/issues/peek-overview/view.tsx(6 hunks)packages/editor/src/core/extensions/slash-commands/root.tsx(4 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx(3 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx(3 hunks)packages/editor/src/core/extensions/utility.ts(1 hunks)packages/editor/src/core/helpers/editor-ref.ts(1 hunks)packages/editor/src/core/types/editor.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
packages/editor/src/core/extensions/slash-commands/root.tsx (2)
packages/editor/src/core/extensions/slash-commands/command-menu.tsx (1)
SlashCommandsMenuProps(10-14)packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)
apps/web/core/components/issues/peek-overview/issue-detail.tsx (1)
packages/editor/src/core/types/editor.ts (1)
EditorRefApi(99-138)
packages/editor/src/core/extensions/utility.ts (1)
packages/editor/src/ce/types/utils.ts (1)
TAdditionalActiveDropbarExtensions(1-1)
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (2)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)packages/editor/src/index.ts (1)
CORE_EXTENSIONS(16-16)
apps/web/core/components/issues/peek-overview/view.tsx (2)
packages/editor/src/core/types/editor.ts (1)
EditorRefApi(99-138)apps/web/core/store/issue/issue-details/root.store.ts (1)
isAnyModalOpen(227-238)
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx (1)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)
packages/editor/src/core/helpers/editor-ref.ts (1)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (10)
packages/editor/src/core/extensions/slash-commands/root.tsx (2)
59-59: Good type annotation for render.Typing
renderItemsasSuggestionOptions["render"]improves clarity and tooling.
104-112: Bug: removes EMOJI instead of SLASH_COMMANDS on exit.This leaves the slash-commands entry stuck in storage, making the app think a dropdown is still open.
Apply this diff:
- const index = utilityStorage.activeDropbarExtensions.indexOf(CORE_EXTENSIONS.EMOJI); + const index = utilityStorage.activeDropbarExtensions.indexOf(CORE_EXTENSIONS.SLASH_COMMANDS);packages/editor/src/core/extensions/utility.ts (2)
14-19: Active dropbar set looks right.Including MENTION, EMOJI, SLASH_COMMANDS, and TABLE aligns with the new UI checks.
23-23: No action required — CORE_EXTENSIONS.UTILITY matches the extension name.
CORE_EXTENSIONS.UTILITY = "utility" in packages/editor/src/core/constants/extension.ts and the extension uses name: "utility" in packages/editor/src/core/extensions/utility.ts.apps/web/core/components/issues/peek-overview/issue-detail.tsx (2)
32-46: Prop-based editorRef looks good.Passing
editorReffrom parent removes local ref divergence and enables shared state checks.
127-137: Correctly threading editorRef to the description input.This enables restore and dropdown state detection downstream.
apps/web/core/components/issues/peek-overview/view.tsx (2)
82-90: Outside-click gating by editor dropbars: LGTM.Prevents closing when any editor dropdown is open.
171-181: Passing editorRef to details: LGTM.Ensures consistent state checks across peek modes.
Also applies to: 213-222
packages/editor/src/core/types/editor.ts (1)
122-122: isAnyDropbarOpen implemented and exposed on editor-ref.
Implementation found in packages/editor/src/core/helpers/editor-ref.ts; type declared in packages/editor/src/core/types/editor.ts; used in apps/web/core/components/issues/peek-overview/view.tsx.packages/editor/src/core/helpers/editor-ref.ts (1)
84-88: LGTM! Well-implemented dropbar state check.The length property is blazing fast because it's stored as a property on the array object - no computation needed, so checking
activeDropbarExtensions.length > 0is performant. The implementation follows a defensive pattern by returningfalsewhen the editor is unavailable, ensuring the peek overview behavior remains predictable.The function correctly leverages the utility extension's storage to determine if any dropbar extensions are currently open, which aligns with the PR objective to prevent peek overview closure when editor dropdowns are active.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx
Outdated
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/editor/src/core/extensions/slash-commands/root.tsx(3 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx(2 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx(2 hunks)packages/editor/src/core/extensions/utility.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/editor/src/core/extensions/slash-commands/root.tsx (2)
packages/editor/src/core/extensions/slash-commands/command-menu.tsx (1)
SlashCommandsMenuProps(10-14)packages/editor/src/index.ts (1)
CORE_EXTENSIONS(16-16)
packages/editor/src/core/extensions/utility.ts (2)
packages/editor/src/index.ts (1)
CORE_EXTENSIONS(16-16)packages/editor/src/ce/types/utils.ts (1)
TAdditionalActiveDropbarExtensions(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
packages/editor/src/core/extensions/utility.ts (4)
14-19: Active dropbar union looks right.Covers the needed core extensions and leaves room for CE via TAdditionalActiveDropbarExtensions.
34-35: Good: public add/remove commands for dropbar tracking.Clear API surface and consistent with other extensions’ usage.
112-123: Idempotent add/remove logic LGTM.indexOf guard prevents dupes; single-splice removal is fine given the add-side guard.
23-23: CORE_EXTENSIONS.UTILITY is the string literal "utility" — no change required.
Defined as UTILITY = "utility" in packages/editor/src/core/constants/extension.ts, so the computed Commands key is safe for TypeScript typings.packages/editor/src/core/extensions/slash-commands/root.tsx (3)
62-66: Correct: tracks SLASH_COMMANDS on start.Matches the intent and fixes prior EMOJI/SLASH mismatch in older revisions.
91-95: Verify Escape path triggers onExit.onKeyDown hides the popup and returns true. Ensure Suggestion still calls onExit so the removeActiveDropbarExtension runs; otherwise, also remove in this branch.
If needed, adjust:
if (props.event.key === "Escape") { - popup?.[0].hide(); + popup?.[0].hide(); + props.editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.SLASH_COMMANDS); return true; }
101-104: Cleanup on exit is correct.Removes SLASH_COMMANDS and tears down tippy/renderer.
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx
Show resolved
Hide resolved
| onExit: () => { | ||
| onExit: ({ editor }) => { | ||
| // Remove from active dropdowns | ||
| editor?.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.SLASH_COMMANDS); |
There was a problem hiding this comment.
Bug: TipTap Suggestion Plugin Callback Parameter Issue
The onExit callback for the TipTap Suggestion plugin now destructures { editor }, but the callback typically receives no parameters. This makes editor undefined, preventing removeActiveDropbarExtension from running and leaving the slash command in the active dropdowns list, which breaks dropdown tracking.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx(3 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (2)
packages/editor/src/index.ts (1)
CORE_EXTENSIONS(16-16)packages/editor/src/core/extensions/table/plugins/drag-handles/column/dropdown.tsx (1)
ColumnOptionsDropdown(62-100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (3)
18-19: LGTM: Importing CORE_EXTENSIONS here makes the intent explicit.
64-73: Deduplicate dropbar tracking logic via a shared hook (same as prior review).Extract this onOpenChange add/remove into a reusable hook and reuse in row/drag-handle to keep behavior consistent.
208-208: Good: Route close through FloatingUI context.Centralizing via context.onOpenChange(false) keeps state transitions consistent and avoids local/desynced toggles.
| onOpenChange: (open) => { | ||
| setIsDropdownOpen(open); | ||
| if (open) { | ||
| editor.commands.addActiveDropbarExtension(CORE_EXTENSIONS.TABLE); | ||
| } else { | ||
| setTimeout(() => { | ||
| editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.TABLE); | ||
| }, 0); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Fix race when switching between table dropbars (shared key + deferred removal).
Problem:
- Using a single key (CORE_EXTENSIONS.TABLE) and removing it via setTimeout can clear the active state when rapidly switching from one table dropdown to another (e.g., column -> row). The “close” of A schedules a removal that runs after B’s “open” added the same key, resulting in a false “no dropbar open”.
Recommendations:
- Track per-instance IDs (ref-count) in the utility extension. Extend commands to accept an instanceId and remove only that instance.
- Prefer queueMicrotask over setTimeout to reduce ordering surprises.
- Ensure cleanup on unmount to avoid stale state.
Proposed change in this component (requires extending commands to accept instanceId):
- onOpenChange: (open) => {
- setIsDropdownOpen(open);
- if (open) {
- editor.commands.addActiveDropbarExtension(CORE_EXTENSIONS.TABLE);
- } else {
- setTimeout(() => {
- editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.TABLE);
- }, 0);
- }
- },
+ onOpenChange: (open) => {
+ setIsDropdownOpen(open);
+ if (open) {
+ editor.commands.addActiveDropbarExtension(CORE_EXTENSIONS.TABLE, instanceIdRef.current);
+ } else {
+ queueMicrotask(() => {
+ editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.TABLE, instanceIdRef.current);
+ });
+ }
+ },Additionally (outside this hunk), create a stable instance id and clean up on unmount:
// imports: add useRef, useEffect
const instanceIdRef = useRef(Symbol("table-col-dropbar"));
useEffect(() => {
return () => {
// defensive cleanup
try {
editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.TABLE, instanceIdRef.current);
} catch {}
};
}, [editor]);Run to verify whether ref-counting already exists (if so, just pass instance ids everywhere):
Expected:
- Storage holds a map/set of instance IDs per extension (not a single array of extension keys).
- Commands support an instance identifier for add/remove.
If not, introduce it in the utility extension and update all call sites accordingly.
🏁 Script executed:
#!/bin/bash
rg -n -C2 "activeDropbarExtensions" packages/editor
rg -n -C3 "addActiveDropbarExtension|removeActiveDropbarExtension" packages/editorLength of output: 11357
Fix race when switching dropbars: implement per-instance tracking for activeDropbarExtensions
Utility currently stores activeDropbarExtensions as a simple array and add/remove only accept the extension key; the deferred removal via setTimeout can remove a key added by another instance (race). Implement per-instance ref-counting and update call sites.
- Change storage to Map<TActiveDropbarExtensions, Set> (or equivalent ref-counting).
- Change commands to accept instanceId (e.g. addActiveDropbarExtension(extension, instanceId) / removeActiveDropbarExtension(extension, instanceId)) and only remove the instanceId; delete the map entry only when its set is empty.
- Replace setTimeout(...) removals with queueMicrotask(...) to reduce ordering surprises and ensure microtask ordering with React updates.
- Add stable per-instance ids in components (useRef(Symbol(...))) and remove on unmount (useEffect cleanup).
- Update consumers to query “any active dropbars” via the new storage shape (helper like hasActiveDropbars) instead of checking activeDropbarExtensions.length.
Call sites to update:
- packages/editor/src/core/extensions/utility.ts (storage + command implementations)
- packages/editor/src/core/helpers/editor-ref.ts
- packages/editor/src/core/extensions/enter-key.ts
- packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx
- packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx
- packages/editor/src/core/extensions/mentions/utils.ts
- packages/editor/src/core/extensions/emoji/suggestion.ts
- packages/editor/src/core/extensions/slash-commands/root.tsx
- (Any other locations that push/splice activeDropbarExtensions)
Component-level change (example; requires command API change):
- onOpenChange: (open) => {
- setIsDropdownOpen(open);
- if (open) {
- editor.commands.addActiveDropbarExtension(CORE_EXTENSIONS.TABLE);
- } else {
- setTimeout(() => {
- editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.TABLE);
- }, 0);
- }
- },
+ onOpenChange: (open) => {
+ setIsDropdownOpen(open);
+ if (open) {
+ editor.commands.addActiveDropbarExtension(CORE_EXTENSIONS.TABLE, instanceIdRef.current);
+ } else {
+ queueMicrotask(() => {
+ editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.TABLE, instanceIdRef.current);
+ });
+ }
+ },Also add in the component:
const instanceIdRef = useRef(Symbol("table-col-dropbar"));
useEffect(() => () => {
try { editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.TABLE, instanceIdRef.current); } catch {}
}, [editor]);🤖 Prompt for AI Agents
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx
lines 64-73: the current add/remove of CORE_EXTENSIONS.TABLE uses a global array
and deferred setTimeout removal which can remove a key added by another instance
(race); replace this by giving each component a stable per-instance id
(useRef(Symbol("table-col-dropbar"))), pass that id into
editor.commands.addActiveDropbarExtension(extension, instanceId) and
removeActiveDropbarExtension(extension, instanceId), change the utility storage
to Map<ExtensionKey, Set<InstanceId>> (or ref-counting set) and only delete the
map entry when the set is empty, switch deferred removal to queueMicrotask(...)
instead of setTimeout, add a helper hasActiveDropbars() that checks map
non-emptiness and update all consumers listed in the review to use the new APIs,
and ensure the component removes its id on unmount via useEffect cleanup calling
removeActiveDropbarExtension inside a try/catch.
Description
This PR fixes the bug where peek overview is getting closed on outside click/pressing the
Esckey while editor dropdowns are still open.Solution:
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes
Documentation