[WIKI-569] chore: migrate to tiptap v3 #7526
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughMigrates TipTap deps to catalog/v3; enables strict TypeScript and minified tsdown; removes Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Editor
participant SuggestionPlugin as Suggestion
participant Storage as Editor.storage
participant FloatingUI
User->>Editor: trigger suggestion/menu (mention / slash / emoji)
Editor->>Suggestion: suggestion.onStart({ editor, clientRect })
Suggestion->>Storage: read config/state via editor.storage.<extension>
Suggestion->>FloatingUI: computePosition(posToDOMRect(selection))
FloatingUI-->>Suggestion: position coords
Suggestion->>Editor: mount floating DOM (FloatingPortal / ReactRenderer)
User->>Suggestion: navigate/select/close
Suggestion->>Editor: execute command / update editor.storage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
![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 (1)
packages/editor/src/core/plugins/markdown-clipboard.ts (1)
12-12: Appropriate temporary fix for outdated type definitions.The
@ts-expect-errordirective with descriptive comment is a reasonable temporary solution for handling outdatedtiptap-markdowntype definitions during the v3 upgrade.Consider creating a follow-up task to remove this suppression once the
tiptap-markdownpackage is updated with compatible type definitions for Tiptap v3.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (31)
apps/live/package.json(1 hunks)packages/editor/package.json(1 hunks)packages/editor/src/ce/types/storage.ts(0 hunks)packages/editor/src/core/components/menus/bubble-menu/root.tsx(9 hunks)packages/editor/src/core/extensions/code/code-block-lowlight.ts(1 hunks)packages/editor/src/core/extensions/code/code-block-node-view.tsx(1 hunks)packages/editor/src/core/extensions/core-without-props.ts(1 hunks)packages/editor/src/core/extensions/custom-image/components/block.tsx(1 hunks)packages/editor/src/core/extensions/custom-image/components/node-view.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/components/upload-status.tsx(1 hunks)packages/editor/src/core/extensions/custom-image/components/uploader.tsx(4 hunks)packages/editor/src/core/extensions/custom-image/extension-config.ts(2 hunks)packages/editor/src/core/extensions/custom-image/utils.ts(1 hunks)packages/editor/src/core/extensions/custom-link/extension.tsx(1 hunks)packages/editor/src/core/extensions/emoji/suggestion.ts(3 hunks)packages/editor/src/core/extensions/enter-key.ts(1 hunks)packages/editor/src/core/extensions/extensions.ts(1 hunks)packages/editor/src/core/extensions/headings-list.ts(2 hunks)packages/editor/src/core/extensions/image/extension.tsx(2 hunks)packages/editor/src/core/extensions/mentions/extension-config.ts(0 hunks)packages/editor/src/core/extensions/mentions/utils.ts(2 hunks)packages/editor/src/core/extensions/placeholder.ts(2 hunks)packages/editor/src/core/extensions/starter-kit.ts(1 hunks)packages/editor/src/core/extensions/table/table/table-view.tsx(6 hunks)packages/editor/src/core/extensions/table/table/table.ts(1 hunks)packages/editor/src/core/extensions/utility.ts(2 hunks)packages/editor/src/core/helpers/editor-ref.ts(4 hunks)packages/editor/src/core/helpers/get-extension-storage.ts(0 hunks)packages/editor/src/core/hooks/use-editor.ts(3 hunks)packages/editor/src/core/hooks/use-file-upload.ts(3 hunks)packages/editor/src/core/plugins/markdown-clipboard.ts(1 hunks)
💤 Files with no reviewable changes (3)
- packages/editor/src/core/extensions/mentions/extension-config.ts
- packages/editor/src/core/helpers/get-extension-storage.ts
- packages/editor/src/ce/types/storage.ts
🧰 Additional context used
🧠 Learnings (11)
packages/editor/src/core/extensions/core-without-props.ts (2)
Learnt from: lifeiscontent
PR: #7164
File: packages/ui/.storybook/main.ts:24-47
Timestamp: 2025-06-04T16:22:44.344Z
Learning: In packages/ui/.storybook/main.ts, the webpackFinal function intentionally overrides the CSS loader strategy by finding and replacing existing CSS rules. This is a temporary workaround for a known upstream issue in Storybook's CSS handling that has been communicated to the Storybook maintainers. The current implementation should not be changed until the upstream issue is resolved.
Learnt from: mathalav55
PR: #6107
File: web/ce/components/workspace-notifications/sidebar/notification-card/options/archive.tsx:11-14
Timestamp: 2024-11-28T07:02:54.664Z
Learning: When components are still located in core, it's appropriate for files to import them using @/components/..., and the migration to the new import paths is not necessary in such cases.
packages/editor/src/core/extensions/extensions.ts (1)
Learnt from: lifeiscontent
PR: #7164
File: packages/ui/.storybook/main.ts:24-47
Timestamp: 2025-06-04T16:22:44.344Z
Learning: In packages/ui/.storybook/main.ts, the webpackFinal function intentionally overrides the CSS loader strategy by finding and replacing existing CSS rules. This is a temporary workaround for a known upstream issue in Storybook's CSS handling that has been communicated to the Storybook maintainers. The current implementation should not be changed until the upstream issue is resolved.
packages/editor/src/core/extensions/custom-image/components/block.tsx (1)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
packages/editor/src/core/extensions/custom-image/utils.ts (2)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
Learnt from: mathalav55
PR: #6107
File: web/ce/components/workspace-notifications/sidebar/notification-card/options/archive.tsx:11-14
Timestamp: 2024-11-28T07:02:54.664Z
Learning: When components are still located in core, it's appropriate for files to import them using @/components/..., and the migration to the new import paths is not necessary in such cases.
packages/editor/src/core/extensions/custom-image/components/upload-status.tsx (2)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
Learnt from: mathalav55
PR: #6107
File: web/ce/components/workspace-notifications/sidebar/notification-card/options/archive.tsx:11-14
Timestamp: 2024-11-28T07:02:54.664Z
Learning: When components are still located in core, it's appropriate for files to import them using @/components/..., and the migration to the new import paths is not necessary in such cases.
packages/editor/src/core/extensions/custom-image/extension-config.ts (1)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
packages/editor/src/core/extensions/custom-image/components/uploader.tsx (1)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
packages/editor/src/core/extensions/utility.ts (1)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
packages/editor/src/core/extensions/image/extension.tsx (1)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
packages/editor/src/core/extensions/custom-image/components/node-view.tsx (1)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
packages/editor/src/core/hooks/use-editor.ts (1)
Learnt from: SatishGandham
PR: #5864
File: packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts:60-60
Timestamp: 2024-10-22T08:03:04.373Z
Learning: In packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts, the getImageSource command returns a string directly, not a function.
⏰ 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). (5)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
- GitHub Check: Build and lint web apps
🔇 Additional comments (54)
packages/editor/src/core/extensions/core-without-props.ts (1)
3-3: LGTM! Correct import for Tiptap v3.The change from default import to named import for
TextStyleis correct and aligns with Tiptap v3's API changes where extensions are now exported as named exports.apps/live/package.json (1)
29-30: Verify Tiptap v3 API usage in the live appWe’ve identified direct imports of Tiptap APIs in your helper:
• apps/live/src/core/helpers/page.ts
import { getSchema } from "@tiptap/core"import { generateHTML, generateJSON } from "@tiptap/html"Since you’ve jumped from v2.x → v3.x, please:
- Cross-check your calls to
getSchema,generateHTML, andgenerateJSONagainst the official v3 migration guide to ensure no signature or behavior changes have been overlooked.- Double-verify that any extensions (none were detected by our scan) have also been updated or renamed per v3.
- Run your editor tests and manually smoke-test the live editor to catch any runtime breakages.
packages/editor/src/core/extensions/custom-image/components/block.tsx (1)
192-192: Excellent defensive programming practice.The guard clause prevents runtime errors when
getPos()returnsundefined, which can happen in certain edge cases. This improves the robustness of the image selection handling.packages/editor/src/core/extensions/table/table/table.ts (1)
259-259: LGTM! Improved type inference in Tiptap v3.The removal of explicit type assertions indicates that Tiptap v3 has better type definitions that allow the
TableViewconstructor to properly infer parameter types without manual casting.packages/editor/src/core/extensions/extensions.ts (2)
4-4: LGTM! Import updated correctly for Tiptap v3.The
TextStyleimport has been correctly updated to use named import syntax from@tiptap/extension-text-style, which aligns with the Tiptap v3 API changes.
6-6: LGTM! Extension consolidated import for Tiptap v3.The
CharacterCountimport has been correctly updated to use the consolidated@tiptap/extensionspackage, which is the new approach in Tiptap v3 where multiple extensions are bundled together.packages/editor/src/core/extensions/starter-kit.ts (1)
44-44: LGTM! StarterKit configuration updated for Tiptap v3.The configuration property has been correctly updated from
history: falsetoundoRedo: false, which reflects the API changes in Tiptap v3's StarterKit extension.packages/editor/src/core/extensions/code/code-block-node-view.tsx (1)
59-59: LGTM! Semantic HTML element updated for better rendering.The change from
codetodivelement maintains the same functionality with thewhitespace-pre-wrapclass while potentially improving rendering behavior. The semantic meaning is preserved since it's already wrapped in apreelement.packages/editor/src/core/extensions/custom-image/utils.ts (1)
15-15: LGTM! Simplified extension storage access.The function has been correctly refactored to use direct access to
editor.storage.imageComponent?.fileMapinstead of thegetExtensionStoragehelper, which simplifies the code and removes unnecessary indirection.packages/editor/src/core/extensions/enter-key.ts (1)
12-12: LGTM! Extension storage access simplified.The code has been correctly updated to use direct access to
this.editor.storage.utility.activeDropbarExtensionsinstead of thegetExtensionStoragehelper, which aligns with the broader refactoring effort to simplify extension storage access patterns.packages/editor/src/core/extensions/custom-image/components/upload-status.tsx (1)
19-19: LGTM! Correct implementation of direct storage access.The change from using
getExtensionStoragehelper to directeditor.storage.utility?.assetsUploadStatus?.[nodeId]access is correct and aligns with the Tiptap v3 upgrade pattern. The optional chaining ensures safe property access.packages/editor/src/core/extensions/custom-image/extension-config.ts (2)
6-11: LGTM! Proper import expansion for storage typing.The expanded import correctly includes
CustomImageExtensionStorageneeded for the module augmentation.
20-22: LGTM! Correct module augmentation for type safety.The Storage interface augmentation properly provides typed access to the custom image extension storage, enabling safe direct access via
editor.storage[CORE_EXTENSIONS.CUSTOM_IMAGE].packages/editor/src/core/extensions/placeholder.ts (2)
1-1: LGTM! Correct import update for Tiptap v3.The import change from
@tiptap/extension-placeholderto@tiptap/extensionsaligns with Tiptap v3's consolidated package structure.
20-20: LGTM! Proper direct storage access implementation.The change from using
getExtensionStorageto directeditor.storage.utility?.uploadInProgressaccess is correct and follows the new storage access pattern consistently used throughout the codebase.packages/editor/src/core/extensions/mentions/utils.ts (2)
32-32: LGTM! Correct direct storage access for adding extension.The change to direct
props.editor.storage.utility.activeDropbarExtensions.push(...)properly replaces the helper function while maintaining the same functionality.
68-71: LGTM! Proper direct storage access with destructuring.The destructuring assignment and direct array manipulation correctly replaces the helper function usage while maintaining the same logic for removing the mention extension from activeDropbarExtensions.
packages/editor/src/core/extensions/image/extension.tsx (2)
2-3: LGTM! Necessary import for module augmentation.The import of
CORE_EXTENSIONSis correctly added to support the Storage interface augmentation.
12-16: LGTM! Proper Storage interface augmentation.The module augmentation correctly extends the Storage interface with typed access to the image extension storage, enabling safe direct access via
editor.storage[CORE_EXTENSIONS.IMAGE].packages/editor/src/core/extensions/code/code-block-lowlight.ts (1)
14-19: LGTM! Improved defensive programming for parent options.The addition of nullish coalescing fallback with explicit defaults is a good defensive programming practice, especially during major version upgrades where parent extension behavior might change.
packages/editor/package.json (1)
48-66: Confirmed: TipTap v3 extensions correctly consolidatedNo remaining imports of
@tiptap/extension-character-countor@tiptap/extension-placeholderwere detected. BothCharacterCountandPlaceholderare now imported from@tiptap/extensionsand used in:• packages/editor/src/core/extensions/placeholder.ts
• packages/editor/src/core/extensions/extensions.tsNo further changes needed.
packages/editor/src/core/extensions/headings-list.ts (1)
6-10: LGTM! Proper TypeScript module augmentation for storage typing.The module augmentation correctly extends the Storage interface for type safety when accessing extension storage directly via
editor.storage.packages/editor/src/core/extensions/utility.ts (2)
18-18: LGTM! Improved type safety with constant-based key.Using
CORE_EXTENSIONS.UTILITYinstead of a string literal improves type safety and ensures consistency across the codebase.
31-34: LGTM! Consistent storage interface augmentation.The Storage interface augmentation follows the same pattern as other extensions and ensures proper typing for direct storage access.
packages/editor/src/core/extensions/custom-image/components/uploader.tsx (3)
91-91: LGTM! Direct storage access aligns with new pattern.The change from
getExtensionStoragehelper to directeditor.storage.utilityaccess is consistent with the broader refactoring across the codebase.
61-66: LGTM! Improved position handling with proper null checks.Adding the
pos !== undefinedcheck prevents potential runtime errors whengetPos()returns undefined, making the cursor manipulation more robust.
113-113: LGTM! Consistent position handling improvements.The changes to use
getPosfunction and add proper undefined checks improve error handling and prevent potential runtime issues when position is not available.Also applies to: 143-150
packages/editor/src/core/extensions/emoji/suggestion.ts (2)
13-13: LGTM! Direct storage access improves type safety.The change from
getExtensionStorageto directeditor.storage.emojiaccess aligns with the Tiptap v3 upgrade and provides better type safety.
57-57: LGTM! Consistent storage access pattern.The direct access to
editor.storage.utility.activeDropbarExtensionsfollows the same pattern as other storage access changes and maintains the existing functionality for tracking active dropdown extensions.Also applies to: 101-104
packages/editor/src/core/extensions/custom-image/components/node-view.tsx (2)
1-1: LGTM! Improved import specificity.The change to a type-only import of
NodeViewPropsis more specific and follows TypeScript best practices for type-only imports.
80-80: LGTM! Direct storage access with safe property chaining.The direct access to
editor.storage.imageComponent?.maxFileSizefollows the refactoring pattern and uses optional chaining for safety.packages/editor/src/core/extensions/custom-link/extension.tsx (2)
53-56: LGTM! Proper module augmentation for storage typing.The module augmentation correctly extends the Tiptap Storage interface, providing type safety for direct storage access patterns used throughout the codebase.
59-59: Enhancement: Added bubble menu state tracking.The addition of
isBubbleMenuOpenproperty enhances the custom link extension's state management capabilities, allowing better control over bubble menu visibility.packages/editor/src/core/hooks/use-editor.ts (4)
3-3: LGTM! Proper module augmentation for markdown storage.The addition of module augmentation for
MarkdownStorageensures type safety for the markdown extension's storage access throughout the codebase.Also applies to: 13-17
87-87: LGTM! Direct storage access with improved readability.The change to
editor.storage.utilityprovides direct access to the upload progress status, eliminating the helper function indirection.
90-95: LGTM! Updated setContent API for Tiptap v3.The change to use an options object with
emitUpdateandparseOptionsaligns with the Tiptap v3 API and improves code clarity over positional arguments.
119-119: LGTM! Safe storage access with optional chaining.The direct access to
editor?.storage.utility?.assetsListuses appropriate optional chaining and nullish coalescing for safety.packages/editor/src/core/helpers/editor-ref.ts (5)
41-43: LGTM! Direct storage access for document metrics.The changes to access character count directly from
editor?.storage.characterCountfollow the refactoring pattern and maintain proper null safety with the nullish coalescing operator.
45-45: LGTM! Simplified headings access.Direct access to
editor.storage.headingsList?.headingseliminates the helper function indirection while maintaining safety with optional chaining.
47-48: LGTM! Safe markdown access with fallback.The direct access to markdown storage with a proper empty string fallback ensures consistent behavior when markdown content is not available.
55-60: LGTM! Consistent setContent API usage.The update to use the options object format for
setContentmatches the pattern used inuse-editor.tsand aligns with the Tiptap v3 API.
111-111: LGTM! Consistent direct storage access pattern.All the storage access changes follow the same pattern of replacing
getExtensionStoragewith directeditor.storageproperty access, maintaining functionality while simplifying the codebase.Also applies to: 127-129, 145-145
packages/editor/src/core/hooks/use-file-upload.ts (4)
1-1: LGTM: Proper type-only import for Tiptap v3.The import correctly uses type-only syntax for
Editorand addsNodeViewPropsto support the new position handling pattern.
93-93: Good refactoring: Dynamic position handling.Changing from static
pos: numberto dynamicgetPos: NodeViewProps["getPos"]ensures the position is current at the time of the drop event, which is more robust than storing a potentially stale position.
104-104: Smart optimization: Position caching.Caching the position result prevents multiple calls to
getPos()within the same operation while ensuring we have the most current position.
130-130: Essential guard: Undefined position check.The guard against
pos === undefinedprevents invalid operations when the position cannot be determined, improving runtime safety.packages/editor/src/core/components/menus/bubble-menu/root.tsx (3)
1-2: LGTM: Proper import structure for Tiptap v3.The separation of type and component imports with appropriate type-only syntax is correct for the v3 upgrade.
53-58: Good refactoring: Simplified component structure.The props destructuring and type definition improve code readability and maintainability.
78-92: LGTM: Improved useEditorState usage.The simplified selector pattern with destructured editor parameter is cleaner and more consistent with v3 patterns.
packages/editor/src/core/extensions/table/table/table-view.tsx (5)
1-7: Excellent type safety: Proper type-only imports.The consistent use of type-only imports for
Editor,NodeViewProps,Node,Decoration,NodeView, andtippytypes improves bundle size and type clarity.
259-261: Good type refinements: Readonly decorations and proper getPos typing.Making decorations readonly prevents accidental mutations, and using
NodeViewProps["getPos"]ensures type consistency with the Tiptap v3 API.
472-481: Robust position handling in selectColumn.The position caching and undefined guard follow the established pattern from other files, preventing runtime errors when position cannot be determined.
484-493: Consistent pattern in selectRow.The same robust position handling pattern is applied consistently, improving code reliability and maintainability.
149-149: Proper placeholder typing.The placeholder actions are correctly typed with
_args: unknownto indicate they're intentionally unused, which is good TypeScript practice.Also applies to: 177-177
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/editor/src/core/extensions/mentions/utils.ts(2 hunks)
⏰ 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: Build and lint web apps
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (5)
packages/editor/src/core/extensions/mentions/utils.ts (5)
1-2: LGTM: Clean migration to floating-ui.The import changes properly replace tippy.js with @floating-ui/dom for positioning, and the addition of
posToDOMRectfrom @tiptap/core provides the necessary integration with Tiptap's selection system.
49-52: LGTM: Proper DOM element setup and positioning.The DOM manipulation correctly sets up absolute positioning, appends to document body, and calls the positioning function. This replaces the previous tippy.js approach effectively.
59-72: LGTM: Event handling maintains functionality.The keyboard event handling preserves the original logic while properly handling component destruction and method calls through the optional chaining pattern.
48-52: Direct storage access verified
Theeditor.storage.utilitystructure is defined and consistently used across multiple extensions (mentions, emoji, placeholder, enter-key, custom-image, etc.), and there are no remaining calls to the removedgetExtensionStoragehelper. No further action is needed.
73-76: Improve cleanup robustness.The cleanup in
onExitshould handle cases where the element might not exist or already be removed from the DOM.onExit: () => { - component?.element.remove(); + if (component?.element && component.element.parentNode) { + component.element.remove(); + } component?.destroy(); },Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/editor/src/core/extensions/table/table/table-view.tsx (1)
83-83: Double-check and updateupdateColumnsOnResizecall to match Tiptap v3 signatureThe upstream
@tiptap/pm/tablesexport forupdateColumnsOnResizein v3 has changed: it now expectsupdateColumnsOnResize( tr: Transaction, table: Node, start: number, mapping?: Mapping, ): Transaction— i.e. it takes a ProseMirror
Transaction, the tableNode, the table’s start offset, and an optionalMapping, returning a newTransaction.On line 83 of
packages/editor/src/core/extensions/table/table/table-view.tsx, the code still calls:updateColumnsOnResize(this.node, this.colgroup, this.table, this.cellMinWidth);These arguments don’t match the new signature. You’ll need to:
- Obtain or create a
Transaction(state.tror similar)- Pass the table
Nodeand its document offset, not the HTML elements- Provide the optional
Mappingif required- Dispatch the returned
TransactionFor example, you might refactor to something like:
const { state, dispatch } = view; let tr = state.tr; tr = updateColumnsOnResize( tr, this.node, // ProseMirror Node tableStartPos, // number: offset of `this.node` in the doc state.doc.map // or other Mapping if needed ); dispatch(tr);Please adjust this call to align with the v3 API so column resizing continues to work correctly.
packages/editor/src/core/components/menus/ai-menu.tsx (1)
53-61: Fix: useclosest('#ai-handle')to support clicks on nested elements within the handleUsing
target.matches('#ai-handle')fails when clicking a child of the handle. Switch toclosestand anchor the popup to the handle if present.- const target = e.target as HTMLElement; - if (target.matches("#ai-handle") || menuRef.current?.contains(e.target as Node)) { + const target = e.target as HTMLElement; + const handle = target.closest("#ai-handle") as HTMLElement | null; + if (handle || menuRef.current?.contains(target)) { e.preventDefault(); if (!isPopupVisible) { popup.current?.setProps({ - getReferenceClientRect: () => target.getBoundingClientRect(), + getReferenceClientRect: () => (handle ?? target).getBoundingClientRect(), }); popup.current?.show(); setIsPopupVisible(true); } return; }packages/editor/src/core/components/menus/block-menu.tsx (1)
144-178: Update Tiptap v3 command usage to match current signaturesThe
insertContentAtandfocuscalls with options objects do not align with Tiptap v3’s current TypeScript definitions. You must remove the unsupported third-argument options and adjust the chain accordingly:• In
packages/editor/src/core/components/menus/block-menu.tsxat the duplicated-content flow (lines 144–178):
- Change
editor .chain() .insertContentAt(insertPos, contentToInsert, { updateSelection: true }) .focus(Math.min(insertPos + 1, docSize), { scrollIntoView: false }) .run();- To something like
editor .chain() .insertContentAt(insertPos, contentToInsert) .focus(Math.min(insertPos + 1, docSize)) .run();• If you need to prevent scrolling, call the dedicated
scrollIntoViewchain command (or omit it; focus no longer accepts options).
• If you intend to move the selection, omitupdateSelection(v3’sinsertContentAthas no such option) and rely on the default behavior or explicitly call.setSelection(...)afterward.Making these changes will satisfy the following v3 signatures:
insertContentAt(position, content): Chainable<any>focus(position?: number | 'start' | 'end'): Chainable<any>
♻️ Duplicate comments (1)
packages/editor/src/core/extensions/headings-list.ts (1)
52-56: Fix TipTap v3 "update" event payload; remove unsupported fields and the misleading transaction.
- TipTap v3’s
"update"event expects{ editor }only;transactionandappendedTransactionsare not part of that payload.newState.trcreates a brand-new empty transaction; passing it as “the transaction” is incorrect and can mislead listeners.Apply this diff:
- this.editor.emit("update", { - editor: this.editor, - transaction: newState.tr, - appendedTransactions: [], - }); + this.editor.emit("update", { editor: this.editor });If you truly need transaction details, emit a separate
"transaction"event from a place where you have the actual transaction, e.g., use thetransactionsparameter ofappendTransactionor subscribe to TipTap’s"transaction"event.
🧹 Nitpick comments (14)
packages/editor/src/core/extensions/table/table/table-view.tsx (5)
10-10: Avoid cloning decorations on each update; also consider dropping unused stateYou changed decorations to a readonly array (good), but you still clone on update. The clone is unnecessary and adds churn on frequent updates.
- Minimal change: assign directly.
- Optional: If decorations aren’t used in this view anymore (looks unused), consider removing the field entirely and the constructor/update assignments. The selection-outline plugin already supplies decorations to the view layer without TableView needing to store them (see packages/editor/src/core/extensions/table/plugins/selection-outline/plugin.ts Lines 54-56).
Apply minimal fix:
- this.decorations = [...decorations]; + this.decorations = decorations;Also applies to: 32-32, 69-69
12-12: getPos typing matches NodeViewProps in v3Using NodeViewProps["getPos"] is the right move for Tiptap v3 and ProseMirror compatibility. If this property isn’t used inside TableView anymore, you can drop it later to reduce surface area, but keeping it is harmless.
Also applies to: 34-34
79-80: Streamline col creationMinor readability/perf nit: avoid creating an intermediate array of sentinel values.
- const cols = Array.from({ length: this.map.width }, () => 1).map(() => h("col")); + const cols = Array.from({ length: this.map.width }, () => h("col"));
19-19: Remove unused fieldscontrols and hoveredCell aren’t referenced anymore after dropping toolboxes/hover logic. Safe to remove to reduce mental overhead.
- hoveredCell: ResolvedPos | null = null; ... - controls?: HTMLElement;Also applies to: 13-13
86-88: Be intentional about ignoreMutation always returning trueAlways returning true can mask DOM mutations that ProseMirror might want to react to. It’s likely fine here because contentDOM is used and you control outer structure, but please validate IME/composition and selection behaviors in tables post-upgrade.
Suggested narrower guard (if needed later):
ignoreMutation(mutation: MutationRecord | { type: "selection"; target: Element }) { // Ignore selection/style mutations; let PM handle childList inside contentDOM if ((mutation as any).type === "selection") return true; if (mutation.type === "attributes" && (mutation.target as Element).closest("colgroup")) return true; return false; }packages/editor/src/core/extensions/headings-list.ts (1)
52-56: Use a scoped event instead of re-emitting the global “update”We ran a search for any
editor.on('update', …)listeners in the repo and found none—so right now no internal duplication occurs—but since TipTap itself emits a global"update"after every transaction, re-emitting it here risks doubling notifications for any consumer that does listen. Instead, emit a namespaced event just for headings if you need a post-transaction hook.• File:
packages/editor/src/core/extensions/headings-list.ts
Lines: ~52–56
• Replace the global update emission with a scoped event:- this.editor.emit("update", { - editor: this.editor, - transaction: newState.tr, - appendedTransactions: [], - }); + this.editor.emit("headings:update", { + editor: this.editor, + headings, // your computed headings payload + });• If you still want to react to overall state updates, rely on TipTap’s built-in
"update"event instead of re-emitting it here.packages/editor/src/core/components/menus/ai-menu.tsx (4)
27-31: HardenappendToto avoid null return fromquerySelector
document.querySelector('.frame-renderer')can return null, which breaks tippy’s expectation. Provide a safe fallback todocument.body.- appendTo: () => document.querySelector(".frame-renderer"), + appendTo: () => document.querySelector(".frame-renderer") ?? document.body,
71-80: Only close on Escape; avoid collapsing the menu on any key pressAttaching
hidePopupdirectly tokeydowncloses the AI menu on every keystroke, which is jarring when typing. Restrict to Escape.- document.addEventListener("keydown", hidePopup); + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === "Escape") hidePopup(); + }; + document.addEventListener("keydown", onKeyDown); ... - document.removeEventListener("keydown", hidePopup); + document.removeEventListener("keydown", onKeyDown);
88-93: Make the popup container focusable and accessible
onShown: () => menuRef.current?.focus()won’t focus a div without atabIndex. Add basic dialog semantics.- <div ref={menuRef} className="z-10"> + <div ref={menuRef} className="z-10" tabIndex={-1} role="dialog" aria-modal="true" aria-label="AI menu">
2-2: Consider migrating this menu to Floating UI for consistencyMost menus in this PR moved to Floating UI; keeping Tippy here diverges in patterns and increases maintenance. Consider refactoring to the same virtual-reference +
useFloatingapproach used inblock-menu.tsxfor consistency and easier positioning fixes.packages/editor/src/core/components/menus/block-menu.tsx (3)
73-96: Reduce duplicated global listeners; lean more on Floating UI interactionsYou’re already using
useDismiss(context)which handles outside clicks/Escape. The additional document-level click/contextmenu/keydown listeners partly duplicate that behavior and add global event noise.Consider:
- Rely on
useDismissfor outside/Escape.- Keep only the click/contextmenu handler that detects
#drag-handle, but convert it to a delegated handler on the editor container instead ofdocument.- Drop the extra
keydownhandler (Escape) in favor ofuseDismiss.
137-141: Guardselection.content()usage and avoid redundant checksAccessing
selection.content()repeatedly can be avoided, and for empty text selections it may not represent the user’s intent.
- Cache
const slice = editor.state.selection.content(); const first = slice.content.firstChild;- For collapsed text selections, consider duplicating the containing block node instead of a zero-width slice (optional, UX-dependent).
186-204: Add menu semantics for accessibilityAdd
role="menu"on the container androle="menuitem"on buttons for AT navigation. Also consideraria-labelledbyif there’s a visible title.- <div + <div ref={(node) => { refs.setFloating(node); menuRef.current = node; }} style={{ ...floatingStyles, animationFillMode: "forwards", transitionTimingFunction: "cubic-bezier(0.16, 1, 0.3, 1)", // Expo ease out }} - className={cn( + className={cn( "z-20 max-h-60 min-w-[7rem] overflow-y-scroll rounded-lg border border-custom-border-200 bg-custom-background-100 p-1.5 shadow-custom-shadow-rg", "transition-all duration-300 transform origin-top-right", isAnimatedIn ? "opacity-100 scale-100" : "opacity-0 scale-75" )} + role="menu" + aria-orientation="vertical" {...getFloatingProps()} >And for items:
- <button + <button key={item.key} type="button" className="flex w-full items-center gap-1.5 truncate rounded px-1 py-1.5 text-xs text-custom-text-200 hover:bg-custom-background-90" onClick={item.onClick} disabled={item.isDisabled} + role="menuitem" + aria-disabled={item.isDisabled ? true : undefined} >packages/editor/package.json (1)
48-65: Consolidate Tiptap imports to a single sourceWe’ve confirmed that the codebase currently mixes imports from the umbrella package (
@tiptap/extensions) with individual extension packages (@tiptap/extension-*). This can lead to duplicate registrations and unnecessary bundle weight. Please pick one approach and update both your imports and dependencies inpackages/editor/package.jsonaccordingly:• Files importing from the umbrella package:
packages/editor/src/core/extensions/placeholder.tspackages/editor/src/core/extensions/extensions.ts(e.g.CharacterCount)• Files importing individual extensions (partial list):
packages/editor/src/core/hooks/use-collaborative-editor.ts(extension-collaboration)packages/editor/src/core/extensions/core-without-props.ts(extension-task-item,extension-task-list,extension-text-style,starter-kit)packages/editor/src/core/extensions/emoji/*.ts(extension-emoji)packages/editor/src/core/extensions/quote.ts(extension-blockquote)packages/editor/src/core/extensions/mentions/extension-config.ts(extension-mention)packages/editor/src/core/extensions/image/extension-config.tsx(extension-image)- (…)
Two safe paths—choose based on which import style you prefer:
Option A — keep umbrella, drop individuals:
- "@tiptap/extension-blockquote": "^3.0.7", - "@tiptap/extension-collaboration": "^3.0.7", - "@tiptap/extension-emoji": "^3.0.7", - "@tiptap/extension-image": "^3.0.7", - "@tiptap/extension-list-item": "^3.0.7", - "@tiptap/extension-mention": "^3.0.7", - "@tiptap/extension-task-item": "^3.0.7", - "@tiptap/extension-task-list": "^3.0.7", - "@tiptap/extension-text-align": "^3.0.7", - "@tiptap/extension-text-style": "^3.0.7", - "@tiptap/starter-kit": "^3.0.7",Option B — keep individuals, drop umbrella:
- "@tiptap/extensions": "^3.0.7",Once you decide, update all import sites and remove the unused deps from
package.json. This will eliminate duplicate code and trim your final bundle.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
packages/editor/package.json(1 hunks)packages/editor/src/core/components/menus/ai-menu.tsx(1 hunks)packages/editor/src/core/components/menus/block-menu.tsx(5 hunks)packages/editor/src/core/extensions/core-without-props.ts(1 hunks)packages/editor/src/core/extensions/custom-image/components/block.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/components/uploader.tsx(5 hunks)packages/editor/src/core/extensions/headings-list.ts(2 hunks)packages/editor/src/core/extensions/slash-commands/root.tsx(3 hunks)packages/editor/src/core/extensions/table/table/table-view.tsx(3 hunks)packages/editor/src/core/helpers/editor-ref.ts(5 hunks)packages/editor/src/styles/table.css(0 hunks)
💤 Files with no reviewable changes (1)
- packages/editor/src/styles/table.css
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/editor/src/core/extensions/custom-image/components/block.tsx
- packages/editor/src/core/extensions/core-without-props.ts
- packages/editor/src/core/helpers/editor-ref.ts
- packages/editor/src/core/extensions/custom-image/components/uploader.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/editor/src/core/extensions/slash-commands/root.tsx (2)
packages/editor/src/core/helpers/tippy.ts (1)
CommandListInstance(1-3)packages/editor/src/core/extensions/slash-commands/command-menu.tsx (2)
SlashCommandsMenuProps(10-14)SlashCommandsMenu(16-151)
packages/editor/src/core/extensions/table/table/table-view.tsx (2)
packages/editor/src/core/extensions/code/lowlight-plugin.ts (1)
decorations(148-150)packages/editor/src/core/extensions/table/plugins/selection-outline/plugin.ts (1)
decorations(55-57)
⏰ 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 (7)
packages/editor/src/core/extensions/table/table/table-view.tsx (2)
1-4: Type-only imports: good v3 alignment and smaller bundlesSwitching to type-only imports for Editor, NodeView, Decoration keeps runtime clean and matches Tiptap v3 packages. No issues spotted.
55-55: CSS class verification completed – all classes exist and scrolling/full-width behavior preserved
- The
.table-wrapperclass (overflow-x: auto; padding-bottom: 26px) is defined inpackages/editor/src/styles/table.cssand applies horizontal scrolling.- The
.editor-full-width-blockmodifier is inpackages/editor/src/styles/variables.css, ensuring the table spans full width of the editor container.- The
.horizontal-scrollbarutility (overflow-x: auto) and.scrollbar-smsizing (12px height/width) are present in global styles underpackages/ui/styles/globals.css(and mirrored inapps/web/styles/globals.css, etc.), providing the desired scrollbar appearance.Given these definitions, the new wrapper classes maintain the previous horizontal-scroll and full-width functionality after removing the old toolbox DOM. Feel free to proceed.
packages/editor/src/core/components/menus/ai-menu.tsx (1)
23-25: Verify the invisible overlay doesn’t block interactions with the tippy elementYou remove the menu node from the invisible full-screen overlay and append it elsewhere. With the overlay still active (
pointer-events-autowhen open), confirm it doesn’t intercept clicks meant for the menu if z-index stacking changes.Would you sanity-check by toggling different themes/containers to ensure the tippy content remains above the overlay and interactions aren’t blocked?
packages/editor/src/core/components/menus/block-menu.tsx (1)
31-42: Nice migration to Floating UI with a virtual referenceGood use of
useFloating+autoUpdateand a virtual reference. This keeps the menu anchored correctly and simplifies positioning.packages/editor/src/core/extensions/slash-commands/root.tsx (1)
1-1: Overall: solid migration off Tippy; positioning logic is cleanUsing a virtual caret rect with
posToDOMRectand converting to Floating UI is the right call. With theautoUpdateaddition, the UX should be robust.packages/editor/package.json (2)
60-61: @tiptap/html is used at runtime – keep in dependenciesThe calls to
generateHTMLandgenerateJSONoccur in runtime helper modules, so this package must remain independencies:
- packages/editor/src/core/helpers/yjs-utils.ts (HTML⇄JSON for rich-text and document editors)
- apps/live/src/core/helpers/page.ts (HTML⇄JSON for live page content)
No change needed to move or remove
@tiptap/html.Likely an incorrect or invalid review comment.
60-65: Ensure ProseMirror dependency unificationI wasn’t able to detect multiple ProseMirror versions via the lockfile (pnpm-lock.yaml) or
npm lsin the editor package, which is expected when using pnpm workspaces. However, absence of evidence isn’t evidence of absence—subtle mismatches between the ProseMirror core libs pulled in by@tiptap/pm(v3.0.7) and by@tiptap/y-tiptap(v3.0.0) can still occur.Please manually verify that both extensions resolve to the same versions of:
- prosemirror-model
- prosemirror-state
- prosemirror-view
You can run at the monorepo root:
# List all ProseMirror core versions across the workspace pnpm list --pattern prosemirror-model prosemirror-state prosemirror-viewIf you see more than one version (e.g. 1.25.x vs 1.40.x), consider adding a root-level override in
package.jsonorpnpm.overridesto pin them to a single version, for example:{ "pnpm": { "overrides": { "prosemirror-model": "1.25.1", "prosemirror-state": "1.25.1", "prosemirror-view": "1.25.1" } } }This will ensure a consistent ProseMirror runtime and prevent subtle runtime or type conflicts.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/editor/src/core/helpers/editor-ref.ts (2)
115-116: Unsafe access to editor.state. can still throw despite optional chaining.*Patterns like
editor?.state.selection.fromand usingeditor.stateinside??fall through.editor?.stateyieldsundefined, then.selectionis read offundefined. Alsopos ?? editor.state.selection.fromwill evaluate the RHS whenposisnullish, even ifeditorisundefined.Apply:
- getCoordsFromPos: (pos) => editor?.view.coordsAtPos(pos ?? editor.state.selection.from), - getCurrentCursorPosition: () => editor?.state.selection.from, + getCoordsFromPos: (pos) => + editor ? editor.view.coordsAtPos(pos ?? editor.state.selection.from) : undefined, + getCurrentCursorPosition: () => (editor ? editor.state.selection.from : undefined), @@ - const resolvedPos = pos ?? editor?.state.selection.from; + const resolvedPos = pos ?? (editor ? editor.state.selection.from : undefined); @@ - if (editor?.state.selection) { + if (editor?.state?.selection) {This keeps API shape while eliminating potential
TypeErrorwheneditoris not available.Also applies to: 217-217, 222-222
1-251: Address unsafe optional-chaining patterns in EditorRef helpersThe ripgrep scan surfaced several locations where
editor?.state.selectionandeditor?.storage.<…>are accessed without the second?., which will throw ifeditoris nullish. Please update each of the following to use full optional-chain guards (editor?.state?.selectionandeditor?.storage?.…):• packages/editor/src/core/helpers/editor-ref.ts
– Line 115:
diff - getCoordsFromPos: (pos) => editor?.view.coordsAtPos(pos ?? editor.state.selection.from), + getCoordsFromPos: (pos) => editor?.view.coordsAtPos(pos ?? editor?.state?.selection.from),
– Line 116:
diff - getCurrentCursorPosition: () => editor?.state.selection.from, + getCurrentCursorPosition: () => editor?.state?.selection.from,
– Line 217:
diff - const resolvedPos = pos ?? editor?.state.selection.from; + const resolvedPos = pos ?? editor?.state?.selection.from;
– Line 222:
diff - if (editor?.state.selection) { + if (editor?.state?.selection) {• packages/editor/src/core/helpers/editor-ref.ts (storage access)
– Line 74:
diff - characters: editor?.storage.characterCount?.characters?.() ?? 0, + characters: editor?.storage?.characterCount?.characters?.() ?? 0,
– Line 76:
diff - words: editor?.storage.characterCount?.words?.() ?? 0, + words: editor?.storage?.characterCount?.words?.() ?? 0,
– Line 155:
diff - isEditorReadyToDiscard: () => editor?.storage.utility?.uploadInProgress === false, + isEditorReadyToDiscard: () => editor?.storage?.utility?.uploadInProgress === false,• packages/editor/src/core/hooks/use-editor.ts
– Line 123:
diff - assets: editor?.storage.utility?.assetsList ?? [], + assets: editor?.storage?.utility?.assetsList ?? [],These changes are required to prevent potential runtime TypeErrors when
editoror its sub-objects are undefined.
🧹 Nitpick comments (2)
packages/editor/src/core/helpers/editor-ref.ts (2)
171-174: Update handler uses storage correctly; suggest mirroring the null-safe pattern.Guard is present (
if (!editor) return;), so this won’t crash. For consistency withgetDocumentInfo(), you could also default values the same way:- characters: editor.storage.characterCount?.characters?.() ?? 0, + characters: editor.storage.characterCount?.characters?.() ?? 0, paragraphs: getParagraphCount(editor?.state), - words: editor.storage.characterCount?.words?.() ?? 0, + words: editor.storage.characterCount?.words?.() ?? 0,No functional change; just aligning style.
189-193: Heading change callback: consider always emitting an array to clear stale UI.Right now the callback is invoked only when
headingsis truthy. Emitting[]when headings are undefined helps consumers clear previously rendered items after structure edits.- const headings = editor.storage.headingsList?.headings; - if (headings) { - callback(headings); - } + const headings = editor.storage.headingsList?.headings ?? []; + callback(headings);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/editor/src/core/helpers/editor-ref.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/editor/src/core/helpers/editor-ref.ts (2)
packages/editor/src/core/helpers/common.ts (1)
getParagraphCount(104-111)packages/editor/src/core/helpers/scroll-to-node.ts (1)
scrollSummary(51-56)
🔇 Additional comments (5)
packages/editor/src/core/helpers/editor-ref.ts (5)
1-2: Type-only imports: good call.This avoids bundling those modules and prevents accidental runtime side effects; fits TS best practices for ambient types.
80-81: Direct markdown storage usage — LGTM.Accessing
editor.storage.markdown.getMarkdown()with a safe default is consistent with the v3 storage pattern.
28-29: Meta flags: double-check intended semantics when clearing vs setting content.
clearEditor()only setsSKIP_FILE_DELETION, whilesetEditorValue()sets bothSKIP_FILE_DELETIONandINTENTIONAL_DELETION. If consumers rely on both flags to differentiate user-initiated clears vs programmatic resets, confirm this divergence is intentional.Do you want a quick pass to standardize meta usage across clear vs set operations?
Also applies to: 88-99
73-79: Events and counters: v3 storage/event usage looks correct.
- Character/word counts are sourced from
editor.storage.characterCount, which is the recommended v3 path.- Subscribing to
"update"for these UI refreshes is appropriate.Reference docs for counters and storage. (tiptap.dev)
Also applies to: 167-185
88-99: chain().setContent usage is correct for Tiptap v3The chainable command
editor.chain().setContent(...)uses Tiptap’s commands API, which in v3 definessetContent(content, options?)—i.e. a content argument followed by a single options object containingemitUpdate,parseOptions, etc. (tiptap.dev). The three-parameter signature(content, emitUpdate, parseOptions)belongs to theEditor#setContentinstance method, not the chainable commands (v1.tiptap.dev). Your current code correctly passes an options object and does not need to be changed.Likely an incorrect or invalid review comment.
aaryan610
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/editor/src/core/extensions/mentions/utils.ts (3)
18-27: HandlecomputePositionerrors to avoid unhandled rejections and stale UI.If the promise rejects, the dropdown can remain mispositioned without feedback. Add a catch (and optionally ensure visibility toggles are consistent).
computePosition(virtualElement, element, { placement: "bottom-start", strategy: "absolute", middleware: [shift(), flip()], }).then(({ x, y, strategy }) => { element.style.width = "max-content"; element.style.position = strategy; element.style.left = `${x}px`; element.style.top = `${y}px`; - }); + }).catch((error) => { + console.warn("Mentions dropdown: failed to compute position", error); + // Keep UI stable if positioning fails + element.style.visibility = "hidden"; + }).finally(() => { + // Restore visibility when we have coordinates + if (element.style.left && element.style.top) { + element.style.visibility = ""; + } + });
54-60: Null-guard on element during update — good fix.This resolves the race highlighted earlier; safe to proceed when
component?.elementexists.
13-28: DRY: ExtractupdatePositionto a shared helper.This positioning logic is used across multiple dropdowns (per earlier review). Centralizing it (and the error handling) will simplify maintenance.
Example new helper (suggested file:
packages/editor/src/core/helpers/floating.ts):import { computePosition, flip, shift } from "@floating-ui/dom"; import { type Editor, posToDOMRect } from "@tiptap/core"; export async function positionDropdown( editor: Editor, element: HTMLElement, getBoundingClientRect?: () => DOMRect ) { const virtual = { getBoundingClientRect: getBoundingClientRect ?? (() => posToDOMRect(editor.view, editor.state.selection.from, editor.state.selection.to)), }; try { const { x, y, strategy } = await computePosition(virtual as any, element, { placement: "bottom-start", strategy: "absolute", middleware: [shift(), flip()], }); element.style.width = "max-content"; element.style.position = strategy; element.style.left = `${x}px`; element.style.top = `${y}px`; element.style.visibility = ""; } catch (e) { console.warn("Dropdown positioning failed:", e); element.style.visibility = "hidden"; } }Then replace local
updatePositioncalls withpositionDropdown(...).
🧹 Nitpick comments (2)
packages/editor/src/core/extensions/mentions/utils.ts (2)
49-52: Prevent flash-of-unpositioned content (FOUC) before first positioning.Hide the element until the first coordinates are applied.
- const element = component.element as HTMLElement; - element.style.position = "absolute"; + const element = component.element as HTMLElement; + element.style.position = "absolute"; + element.style.visibility = "hidden"; document.body.appendChild(element); updatePosition(props.editor, element);
61-74: Key handling polish: stop propagation on Escape and return a strict boolean.
- Stop propagation on Escape to avoid editor-level side effects.
- Coalesce
onKeyDownreturn to boolean to satisfy the contract even ifrefis missing.onKeyDown: (props) => { if (props.event.key === "Escape") { - component?.destroy(); + props.event?.stopPropagation(); + component?.destroy(); return true; } const navigationKeys = ["ArrowUp", "ArrowDown", "Enter"]; if (navigationKeys.includes(props.event.key)) { props.event?.stopPropagation(); - return component?.ref?.onKeyDown(props); + return component?.ref?.onKeyDown(props) ?? false; } - return component?.ref?.onKeyDown(props); + return component?.ref?.onKeyDown(props) ?? false; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/editor/src/core/extensions/mentions/utils.ts(2 hunks)packages/editor/src/core/helpers/editor-ref.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/editor/src/core/helpers/editor-ref.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/editor/src/core/extensions/mentions/utils.ts (3)
packages/editor/src/core/types/mention.ts (1)
TMentionHandler(21-25)packages/editor/src/core/helpers/tippy.ts (1)
CommandListInstance(1-3)packages/editor/src/core/extensions/mentions/mentions-list-dropdown.tsx (1)
MentionsListDropdownProps(13-17)
⏰ 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 (2)
packages/editor/src/core/extensions/mentions/utils.ts (2)
1-2: Imports look correct for v3 migration.Using
posToDOMRectfrom@tiptap/coreand Floating UI imports aligns with the v3 stack.
13-28: Prefer anchoring to TipTap’sclientRectwhen available.
posToDOMRecton the current selection usually works, but the Suggestion plugin already provides a precise anchor viaprops.clientRect. Accepting a rect getter improves accuracy during IME composition and complex ranges.
[Suggest_optional_refactor]-const updatePosition = (editor: Editor, element: HTMLElement) => { - const virtualElement = { - getBoundingClientRect: () => posToDOMRect(editor.view, editor.state.selection.from, editor.state.selection.to), - }; +const updatePosition = ( + editor: Editor, + element: HTMLElement, + getBoundingClientRect: () => DOMRect = () => + posToDOMRect(editor.view, editor.state.selection.from, editor.state.selection.to) +) => { + const virtualElement = { getBoundingClientRect };And pass the getter where you already validate
props.clientRect:- updatePosition(props.editor, element); + updatePosition(props.editor, element, props.clientRect!);- updatePosition(props.editor, component?.element as HTMLElement); + updatePosition(props.editor, component?.element as HTMLElement, props.clientRect!);
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
packages/editor/src/core/extensions/mentions/utils.ts (2)
21-36: Keep dropdown aligned on scroll/resize via autoUpdate (optional, shared helper available)Position only updates on Start/Update; it won’t follow scroll/viewport changes. Either use Floating UI’s autoUpdate here or the new attachFloatingUIAutoUpdate helper to wire/cleanup automatically.
+import { attachFloatingUIAutoUpdate } from "@/helpers/floating-ui"; @@ () => { const { searchCallback } = props; let component: ReactRenderer<CommandListInstance, MentionsListDropdownProps> | null = null; + let cleanup: VoidFunction | null = null; @@ const element = component.element as HTMLElement; element.style.position = "absolute"; - document.body.appendChild(element); - updateFloatingUIFloaterPosition(props.editor, element); + document.body.appendChild(element); + cleanup = attachFloatingUIAutoUpdate(props.editor, element); }, @@ onExit: () => { - component?.element.remove(); + cleanup?.(); + cleanup = null; + component?.element?.remove(); component?.destroy(); },
31-31: Symmetric cleanup for activeDropbar state and DOM elementYou push CORE_EXTENSIONS.MENTION into editor.storage.utility.activeDropbarExtensions but never remove it; also Escape path doesn’t remove the appended element. This can leave stale state and stray DOM.
onKeyDown: (props) => { if (props.event.key === "Escape") { - component?.destroy(); + // remove active state + const list = props.editor.storage?.utility?.activeDropbarExtensions; + if (Array.isArray(list)) { + const idx = list.lastIndexOf(CORE_EXTENSIONS.MENTION); + if (idx !== -1) list.splice(idx, 1); + } + component?.element?.remove(); + component?.destroy(); return true; } @@ onExit: () => { - component?.element.remove(); - component?.destroy(); + // remove active state + const list = component?.editor?.storage?.utility?.activeDropbarExtensions; + if (Array.isArray(list)) { + const idx = list.lastIndexOf(CORE_EXTENSIONS.MENTION); + if (idx !== -1) list.splice(idx, 1); + } + component?.element?.remove(); + component?.destroy(); },Also applies to: 58-60
packages/editor/src/core/extensions/slash-commands/root.tsx (3)
56-76: Create component only when usable and wire autoUpdate to track layout changes
- Minor: Don’t construct ReactRenderer if clientRect is missing.
- Optional: Use Floating UI’s autoUpdate so the menu stays glued to the caret on scroll/resize/line wraps. Clean it up on close.
-import { updateFloatingUIFloaterPosition } from "@/helpers/floating-ui"; +import { updateFloatingUIFloaterPosition, attachFloatingUIAutoUpdate } from "@/helpers/floating-ui"; @@ - let component: ReactRenderer<CommandListInstance, SlashCommandsMenuProps> | null = null; + let component: ReactRenderer<CommandListInstance, SlashCommandsMenuProps> | null = null; + let cleanup: VoidFunction | null = null; @@ - onStart: (props) => { - const MenuComponent = SlashCommandsMenu as unknown as FC< + onStart: (props) => { + if (!props.clientRect) { + return; + } + const MenuComponent = SlashCommandsMenu as unknown as FC< SlashCommandsMenuProps & { ref: React.Ref<CommandListInstance> } >; component = new ReactRenderer(MenuComponent, { props, editor: props.editor, }); - - if (!props.clientRect) { - return; - } const element = component.element as HTMLElement; element.style.position = "absolute"; element.style.zIndex = "100"; (props.editor.options.element || document.body).appendChild(element); - updateFloatingUIFloaterPosition(props.editor, element); + cleanup = attachFloatingUIAutoUpdate(props.editor, element); },
90-98: Dispose autoUpdate and remove element on EscapePrevent leaks and ensure the DOM node is removed even if ReactRenderer fails to.
onKeyDown: (props) => { if (props.event.key === "Escape") { - component?.destroy(); - component = null; + cleanup?.(); + cleanup = null; + component?.element?.remove(); + component?.destroy(); + component = null; return true; } return component?.ref?.onKeyDown(props) ?? false; },
100-103: Dispose autoUpdate and remove element on exit as wellMirror the Escape path for a clean teardown.
onExit: () => { - component?.destroy(); - component = null; + cleanup?.(); + cleanup = null; + component?.element?.remove(); + component?.destroy(); + component = null; },
🧹 Nitpick comments (4)
packages/editor/src/core/helpers/floating-ui.ts (3)
18-31: Harden async positioning: add error handling and a small default offsetcomputePosition is async and can throw (e.g., selection/view teardown). Add a catch to avoid noisy console errors and optionally include a small offset so menus don’t overlap the caret.
-import { computePosition, flip, type Middleware, type Strategy, type Placement, shift } from "@floating-ui/dom"; +import { computePosition, flip, type Middleware, type Strategy, type Placement, shift, offset } from "@floating-ui/dom"; @@ - computePosition(virtualElement, element, { - placement: options?.placement ?? "bottom-start", - strategy: options?.strategy ?? "absolute", - middleware: options?.middleware ?? [shift(), flip()], - }).then(({ x, y, strategy }) => { + computePosition(virtualElement, element, { + placement: options?.placement ?? "bottom-start", + strategy: options?.strategy ?? "absolute", + middleware: options?.middleware ?? [offset(4), shift(), flip()], + }).then(({ x, y, strategy }) => { Object.assign(element.style, { width: "max-content", position: strategy, left: `${x}px`, top: `${y}px`, ...options?.elementStyle, }); - }); + }).catch((err) => { + // Non-fatal: positioning can fail during view/selection teardown + if (process.env.NODE_ENV !== "production") console.warn("Floating UI positioning failed:", err); + });
14-16: Defensive virtual reference to avoid crashes during teardownIf editor.view is destroyed between scheduling and running, posToDOMRect can throw. Minor, but worth guarding.
- const virtualElement = { - getBoundingClientRect: () => posToDOMRect(editor.view, editor.state.selection.from, editor.state.selection.to), - }; + const virtualElement = { + getBoundingClientRect: () => { + const view = editor?.view; + const state = editor?.state; + if (!view || !state) return new DOMRect(0, 0, 0, 0); + return posToDOMRect(view, state.selection.from, state.selection.to); + }, + };
1-31: Optional: provide an autoUpdate helper so call sites don’t re-implement itMultiple callers need scroll/resize tracking. Export a small helper that returns a disposer; low-risk addition that doesn’t break existing usage.
import { computePosition, flip, type Middleware, type Strategy, type Placement, shift, offset } from "@floating-ui/dom"; +import { autoUpdate } from "@floating-ui/dom"; @@ export const updateFloatingUIFloaterPosition = ( @@ }; + +export const attachFloatingUIAutoUpdate = ( + editor: Editor, + element: HTMLElement, + options?: { + elementStyle?: Partial<CSSStyleDeclaration>; + middleware?: Middleware[]; + placement?: Placement; + strategy?: Strategy; + } +) => { + const virtualRef = { + getBoundingClientRect: () => posToDOMRect(editor.view, editor.state.selection.from, editor.state.selection.to), + } as any; + const update = () => updateFloatingUIFloaterPosition(editor, element, options); + // Immediately position once and then subscribe + update(); + const cleanup = autoUpdate(virtualRef, element, update); + return cleanup; +};Happy to wire this into mentions/slash-commands in a follow-up if you want.
packages/editor/src/core/extensions/mentions/utils.ts (1)
33-36: Append inside editor container and set stacking contextFor consistency with Slash Commands and to avoid z-index layering issues, append to editor.options.element when available and set a safe z-index.
- const element = component.element as HTMLElement; - element.style.position = "absolute"; - document.body.appendChild(element); + const element = component.element as HTMLElement; + element.style.position = "absolute"; + element.style.zIndex = "100"; + (props.editor.options.element || document.body).appendChild(element);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/editor/src/core/extensions/mentions/utils.ts(3 hunks)packages/editor/src/core/extensions/slash-commands/root.tsx(2 hunks)packages/editor/src/core/helpers/floating-ui.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/editor/src/core/extensions/mentions/utils.ts (1)
packages/editor/src/core/helpers/floating-ui.ts (1)
updateFloatingUIFloaterPosition(4-31)
packages/editor/src/core/extensions/slash-commands/root.tsx (3)
packages/editor/src/core/helpers/tippy.ts (1)
CommandListInstance(1-3)packages/editor/src/core/extensions/slash-commands/command-menu.tsx (2)
SlashCommandsMenuProps(10-14)SlashCommandsMenu(16-151)packages/editor/src/core/helpers/floating-ui.ts (1)
updateFloatingUIFloaterPosition(4-31)
⏰ 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 (2)
packages/editor/src/core/extensions/mentions/utils.ts (1)
37-43: Good fix: null-check before repositioningThe extra element guard in onUpdate avoids rare race conditions. Looks good.
packages/editor/src/core/extensions/slash-commands/root.tsx (1)
77-88: LGTM: update path is safe and idempotentGuarding component existence, updating props, and repositioning is correct.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/live/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T08:45:15.953Z
Learnt from: sriramveeraghanta
PR: makeplane/plane#7672
File: pnpm-workspace.yaml:8-9
Timestamp: 2025-08-29T08:45:15.953Z
Learning: The makeplane/plane repository uses pnpm v10.12.1, which supports onlyBuiltDependencies configuration in pnpm-workspace.yaml files.
Applied to files:
apps/live/package.json
⏰ 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)
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/editor/package.json (1)
76-76: Replacetiptap-markdownwith a TipTap v3-compatible Markdown extension
tiptap-markdown@^0.8.10targets TipTap v2 and won’t work with your v3 editor. Swap it for either the official paid@tiptap-proimport/export extensions (e.g.@tiptap-pro/extension-import&@tiptap-pro/extension-export) or a maintained v3 fork such as@sequa-ai/tiptap-markdown. Remove the current dependency and add your chosen v3 package in packages/editor/package.json (line 76).
🧹 Nitpick comments (11)
packages/editor/package.json (1)
49-65: Optional: unify all TipTap versions via workspace catalog to prevent skewOnly
@tiptap/coreand@tiptap/htmlusecatalog:. To harden upgrades, consider adding catalog entries for the other TipTap packages at 3.5.3 and referencing them here. This avoids accidental drift across packages in the monorepo.packages/editor/src/core/components/menus/floating-menu/use-floating-menu.ts (1)
17-49: Allow the hook to be called without an argumentEvery call site currently passes
{}just to satisfy the parameter. Making the args parameter optional keeps the API ergonomic and avoids needless object allocations on every render.-type TArgs = { +type TArgs = { handleOpenChange?: (open: boolean) => void; }; -export const useFloatingMenu = (args: TArgs) => { - const { handleOpenChange } = args; +export const useFloatingMenu = (args: TArgs = {}) => { + const { handleOpenChange } = args;packages/editor/src/core/components/menus/bubble-menu/link-selector.tsx (1)
31-38: Guard against empty/whitespace link submissionsOn Line 31, pressing Enter or the check icon after clearing the field is a silent no‑op because we return before touching the error state. Trimming and treating empty input as invalid surfaces the existing “Please enter a valid URL” feedback and covers the common case of pasted URLs with trailing spaces.
- const url = input.value; - if (!url) return; + const url = input.value.trim(); + if (!url) { + setError(true); + return; + }packages/editor/src/core/components/menus/floating-menu/root.tsx (3)
1-6: Add FloatingFocusManager for basic a11y and returnFocus.Without focus management, ESC/Tab behavior is less robust and focus may not return to the trigger.
Apply this diff to import the focus manager:
import { FloatingOverlay, FloatingPortal, + FloatingFocusManager, type UseInteractionsReturn, type UseFloatingReturn, } from "@floating-ui/react";As per coding guidelines
43-61: Wrap floating content with FloatingFocusManager.Improves keyboard navigation and restores focus to the trigger on close.
{context.open && ( <FloatingPortal> {/* Backdrop */} <FloatingOverlay style={{ zIndex: 99, }} lockScroll /> - <div - ref={refs.setFloating} - {...getFloatingProps()} - style={{ - ...floatingStyles, - zIndex: 100, - }} - > - {children} - </div> + <FloatingFocusManager context={context} modal={false} returnFocus> + <div + ref={refs.setFloating} + {...getFloatingProps()} + style={{ + ...floatingStyles, + zIndex: 100, + }} + role="menu" + > + {children} + </div> + </FloatingFocusManager> </FloatingPortal> )}As per coding guidelines
45-50: Reconsider lockScroll on small dropdowns.Locking body scroll for small menus can feel heavy and interferes with reading/scrolling the document while a menu is open. Consider removing lockScroll or gating it via a prop.
packages/editor/src/core/components/menus/bubble-menu/node-selector.tsx (1)
77-101: Add basic ARIA roles to menu and items.Improve screen reader support and announce selection state.
- <section className="w-48 max-h-[90vh] mt-1 flex flex-col overflow-y-scroll rounded-md border-[0.5px] border-custom-border-300 bg-custom-background-100 px-2 py-2.5 shadow-custom-shadow-rg"> + <section + className="w-48 max-h-[90vh] mt-1 flex flex-col overflow-y-scroll rounded-md border-[0.5px] border-custom-border-300 bg-custom-background-100 px-2 py-2.5 shadow-custom-shadow-rg" + role="menu" + aria-label="Block type" + > {items.map((item) => ( <button key={item.name} type="button" onClick={(e) => { item.command(); context.onOpenChange(false); e.stopPropagation(); }} className={cn( "flex items-center justify-between rounded px-1 py-1.5 text-sm text-custom-text-200 hover:bg-custom-background-80", { "bg-custom-background-80": activeItem.name === item.name, } )} + role="menuitemradio" + aria-checked={activeItem.name === item.name} >As per coding guidelines
packages/editor/src/core/components/menus/bubble-menu/color-selector.tsx (4)
21-23: Expose context to allow consistent close-on-select.You’ll need context to close after picking a color (consistency with node selector).
- const { options, getReferenceProps, getFloatingProps } = useFloatingMenu({}); + const { options, getReferenceProps, getFloatingProps } = useFloatingMenu({}); + const { context } = options;As per coding guidelines
67-77: Close the menu after selecting a text color; stop propagation.Aligns UX with node selector and prevents outside click handlers from interfering.
- {COLORS_LIST.map((color) => ( - <button + {COLORS_LIST.map((color) => ( + <button key={color.key} type="button" className="flex-shrink-0 size-6 rounded border-[0.5px] border-custom-border-400 hover:opacity-60 transition-opacity" style={{ backgroundColor: color.textColor, }} - onClick={() => TextColorItem(editor).command({ color: color.key })} + onClick={(e) => { + TextColorItem(editor).command({ color: color.key }); + context.onOpenChange(false); + e.stopPropagation(); + }} /> ))} <button type="button" className="flex-shrink-0 size-6 grid place-items-center rounded text-custom-text-300 border-[0.5px] border-custom-border-400 hover:bg-custom-background-80 transition-colors" - onClick={() => TextColorItem(editor).command({ color: undefined })} + onClick={(e) => { + TextColorItem(editor).command({ color: undefined }); + context.onOpenChange(false); + e.stopPropagation(); + }} > <Ban className="size-4" /> </button>As per coding guidelines
88-110: Close the menu after selecting a background color; stop propagation.Mirror behavior of text color.
- {COLORS_LIST.map((color) => ( - <button + {COLORS_LIST.map((color) => ( + <button key={color.key} type="button" className="flex-shrink-0 size-6 rounded border-[0.5px] border-custom-border-400 hover:opacity-60 transition-opacity" style={{ backgroundColor: color.backgroundColor, }} - onClick={() => BackgroundColorItem(editor).command({ color: color.key })} + onClick={(e) => { + BackgroundColorItem(editor).command({ color: color.key }); + context.onOpenChange(false); + e.stopPropagation(); + }} /> ))} <button type="button" className="flex-shrink-0 size-6 grid place-items-center rounded text-custom-text-300 border-[0.5px] border-custom-border-400 hover:bg-custom-background-80 transition-colors" - onClick={() => BackgroundColorItem(editor).command({ color: undefined })} + onClick={(e) => { + BackgroundColorItem(editor).command({ color: undefined }); + context.onOpenChange(false); + e.stopPropagation(); + }} > <Ban className="size-4" /> </button>As per coding guidelines
28-36: A11y: add aria attributes to trigger.Expose expanded state and intent (menu).
- <FloatingMenuRoot + <FloatingMenuRoot classNames={{ buttonContainer: "h-full", button: "flex items-center gap-1 h-full whitespace-nowrap px-3 text-sm font-medium text-custom-text-300 hover:bg-custom-background-80 active:bg-custom-background-80 rounded transition-colors", }}No code change needed here once FloatingMenuRoot adds aria-expanded/aria-haspopup as suggested.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
apps/live/package.json(1 hunks)packages/editor/package.json(1 hunks)packages/editor/src/core/components/menus/bubble-menu/color-selector.tsx(1 hunks)packages/editor/src/core/components/menus/bubble-menu/index.ts(0 hunks)packages/editor/src/core/components/menus/bubble-menu/link-selector.tsx(2 hunks)packages/editor/src/core/components/menus/bubble-menu/node-selector.tsx(3 hunks)packages/editor/src/core/components/menus/bubble-menu/root.tsx(9 hunks)packages/editor/src/core/components/menus/floating-menu/root.tsx(1 hunks)packages/editor/src/core/components/menus/floating-menu/use-floating-menu.ts(1 hunks)packages/editor/src/core/extensions/utility.ts(2 hunks)packages/editor/src/core/helpers/editor-ref.ts(5 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- packages/editor/src/core/components/menus/bubble-menu/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/editor/src/core/extensions/utility.ts
- packages/editor/src/core/helpers/editor-ref.ts
- apps/live/package.json
🧰 Additional context used
🧬 Code graph analysis (4)
packages/editor/src/core/components/menus/bubble-menu/link-selector.tsx (3)
packages/editor/src/core/components/menus/floating-menu/use-floating-menu.ts (1)
useFloatingMenu(17-50)packages/editor/src/core/components/menus/floating-menu/root.tsx (1)
FloatingMenuRoot(21-65)packages/editor/src/core/helpers/editor-commands.ts (1)
unsetLinkEditor(126-128)
packages/editor/src/core/components/menus/bubble-menu/node-selector.tsx (2)
packages/editor/src/core/components/menus/floating-menu/use-floating-menu.ts (1)
useFloatingMenu(17-50)packages/editor/src/core/components/menus/floating-menu/root.tsx (1)
FloatingMenuRoot(21-65)
packages/editor/src/core/components/menus/bubble-menu/root.tsx (6)
packages/editor/src/core/components/menus/menu-items.ts (7)
CodeItem(171-177)BoldItem(107-113)ItalicItem(115-121)UnderLineItem(123-129)StrikeThroughItem(131-137)TextAlignItem(239-248)EditorMenuItem(57-63)packages/editor/src/core/types/editor.ts (1)
TEditorCommands(31-60)packages/editor/src/core/components/menus/bubble-menu/node-selector.tsx (1)
BubbleMenuNodeSelector(32-104)packages/editor/src/core/components/menus/bubble-menu/link-selector.tsx (1)
BubbleMenuLinkSelector(18-121)packages/editor/src/core/components/menus/bubble-menu/color-selector.tsx (1)
BubbleMenuColorSelector(19-113)packages/editor/src/core/components/menus/bubble-menu/alignment-selector.tsx (1)
TextAlignmentSelector(16-82)
packages/editor/src/core/components/menus/bubble-menu/color-selector.tsx (5)
packages/editor/src/core/components/menus/bubble-menu/root.tsx (1)
EditorStateType(33-58)packages/editor/src/core/components/menus/floating-menu/use-floating-menu.ts (1)
useFloatingMenu(17-50)packages/editor/src/core/components/menus/floating-menu/root.tsx (1)
FloatingMenuRoot(21-65)packages/editor/src/core/constants/common.ts (1)
COLORS_LIST(183-243)packages/editor/src/core/components/menus/menu-items.ts (2)
TextColorItem(217-226)BackgroundColorItem(228-237)
⏰ 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: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/editor/package.json (1)
49-67: Pin @y-tiptap/y-tiptap to prerelease matching collaboration peerDependencyThe @tiptap/extension-collaboration@^3.5.x still requires
@tiptap/y-tiptap@^3.0.0-beta.3, so update your dependency:packages/editor/package.json … - "@tiptap/y-tiptap": "^3.0.0", + "@tiptap/y-tiptap": "3.0.0-beta.3",packages/editor/src/core/components/menus/bubble-menu/root.tsx (1)
121-144: Fix bubble menu open/close callbacks for the TipTap v3 APIIn v3 the
optionsprop is forwarded straight to Floating UI’suseFloating, so keys such asonShow,onHide, andonDestroyare now ignored. As a result, we never flipeditor.storage.link.isBubbleMenuOpenand we never add/remove the"bubble-menu"dropbar entry, which regresses link editing and extension bookkeeping. Please move these side-effects to the newonOpenChangecallback (and reintroduce the actual Floating UI options for placement/offset). For example:- options: { - onShow: () => { - editor.storage.link.isBubbleMenuOpen = true; - editor.commands.addActiveDropbarExtension("bubble-menu"); - }, - onHide: () => { - editor.storage.link.isBubbleMenuOpen = false; - setTimeout(() => { - editor.commands.removeActiveDropbarExtension("bubble-menu"); - }, 0); - }, - onDestroy: () => { - setTimeout(() => { - editor.commands.removeActiveDropbarExtension("bubble-menu"); - }, 0); - }, - }, - // TODO: Migrate these to floating UI options - // tippyOptions: { - // moveTransition: "transform 0.15s ease-out", - // duration: [300, 0], - // zIndex: 9, - // }, + onOpenChange: (open) => { + editor.storage.link.isBubbleMenuOpen = open; + if (open) { + editor.commands.addActiveDropbarExtension("bubble-menu"); + return; + } + setTimeout(() => { + editor.commands.removeActiveDropbarExtension("bubble-menu"); + }, 0); + }, + options: { + placement: "top-start", + middleware: [offset(6), flip(), shift()], + },(Remember to import
offset,flip, andshiftfrom@floating-ui/react.) As per coding guidelinespackages/editor/src/core/components/menus/bubble-menu/node-selector.tsx (1)
56-76: Leverage composed handlers from FloatingMenuRoot.Once FloatingMenuRoot composes getReferenceProps correctly, no change needed here. Button active state tied to context.open looks good.
Please verify the trigger still opens with keyboard (Enter/Space) after applying the root fix.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/editor/src/core/plugins/file/restore.ts (1)
60-79: Async forEach + duplicate restores; add de-dupe sentinel and prefer for…of.
Array.forEach(async ...)doesn’t await; duplicates of the samesrccan trigger multiple concurrentrestoreHandlercalls. Also,// @ts-expect-errorhides storage typing.- addedFiles.forEach(async (node) => { - const nodeType = node.type.name as CORE_EXTENSIONS; - const src = node.attrs.src; - const nodeFileSetDetails = NODE_FILE_MAP[nodeType]; - if (!nodeFileSetDetails) return; - // @ts-expect-error add proper types for storage - const extensionFileSetStorage = editor.storage[nodeType]?.[nodeFileSetDetails.fileSetName]; - const wasDeleted = extensionFileSetStorage?.get(src); - if (!nodeFileSetDetails || !src) return; - if (wasDeleted === undefined) { - extensionFileSetStorage?.set(src, false); - } else if (wasDeleted === true) { - try { - await restoreHandler(src); - extensionFileSetStorage?.set(src, false); - } catch (error) { - console.error("Error restoring file via restore utility plugin:", error); - } - } - }); + for (const node of addedFiles) { + const nodeTypeName = node.type.name as string; + if (!(nodeTypeName in NODE_FILE_MAP)) continue; + const nodeType = nodeTypeName as CORE_EXTENSIONS; + const src = node.attrs?.src as string | undefined; + const nodeFileSetDetails = NODE_FILE_MAP[nodeType]; + if (!nodeFileSetDetails || !src) continue; + const extensionFileSetStorage = + (editor.storage as any)[nodeType]?.[nodeFileSetDetails.fileSetName] as + Map<string, boolean | "restoring"> | undefined; + const wasDeleted = extensionFileSetStorage?.get(src); + if (wasDeleted === undefined) { + extensionFileSetStorage?.set(src, false); + } else if (wasDeleted === true) { + try { + // De-dupe concurrent restores for same src + extensionFileSetStorage?.set(src, "restoring"); + await restoreHandler(src); + extensionFileSetStorage?.set(src, false); + } catch (error) { + console.error("Error restoring file via restore utility plugin:", error); + extensionFileSetStorage?.set(src, true); + } + } + }To remove the
// @ts-expect-error, add a minimal storage type:// Place near the top of the file type FileSetStorage = Map<string, boolean | "restoring">;…and use
as FileSetStorage | undefinedinstead ofany. Based on learnings.packages/editor/src/core/plugins/file/delete.ts (2)
36-71: Bug: possible duplicate deletions when multiple transactions are batched.You loop over
transactionsbut diffoldStatevsnewFileSourceseach time, re-triggering deletes for the same nodes whentransactions.length > 1. This can cause repeated external deletes and storage mutations.Suggested fix: compute removals once, then act once.
- transactions.forEach((transaction) => { - if (transaction.getMeta(CORE_EDITOR_META.SKIP_FILE_DELETION)) return; - const removedFiles: TFileNode[] = []; - oldState.doc.descendants((node) => { - const nodeType = node.type.name as CORE_EXTENSIONS; - const isAValidNode = NODE_FILE_MAP[nodeType]; - if (!isAValidNode) return; - if (!newFileSources[nodeType]?.has(node.attrs.src)) { - removedFiles.push(node as TFileNode); - } - }); - removedFiles.forEach(async (node) => { - const nodeType = node.type.name as CORE_EXTENSIONS; - const src = node.attrs.src; - const nodeFileSetDetails = NODE_FILE_MAP[nodeType]; - if (!nodeFileSetDetails || !src) return; - try { - // @ts-expect-error add proper types for storage - editor.storage[nodeType]?.[nodeFileSetDetails.fileSetName]?.set(src, true); - editor.commands.updateAssetsList?.({ idToRemove: node.attrs.id }); - await deleteHandler(src); - } catch (error) { - console.error("Error deleting file via delete utility plugin:", error); - } - }); - }); + // Skip if any transaction marks SKIP_FILE_DELETION + if (transactions.some((tr) => tr.getMeta(CORE_EDITOR_META.SKIP_FILE_DELETION))) { + return null; + } + const removed: TFileNode[] = []; + oldState.doc.descendants((node) => { + const nodeType = node.type.name as CORE_EXTENSIONS; + if (!NODE_FILE_MAP[nodeType]) return; + const src = node.attrs.src as string | undefined; + if (typeof src !== "string" || src.length === 0) return; + if (!newFileSources[nodeType]?.has(src)) removed.push(node as TFileNode); + }); + // De‑dupe by nodeType+src + const seen = new Set<string>(); + await Promise.allSettled( + removed.map(async (node) => { + const nodeType = node.type.name as CORE_EXTENSIONS; + const src = node.attrs.src as string | undefined; + if (!src) return; + const key = `${nodeType}:${src}`; + if (seen.has(key)) return; + seen.add(key); + const nodeFileSetDetails = NODE_FILE_MAP[nodeType]!; + try { + // @ts-expect-error add proper types for storage + editor.storage[nodeType]?.[nodeFileSetDetails.fileSetName]?.set(src, true); + editor.commands.updateAssetsList?.({ idToRemove: node.attrs.id }); + await deleteHandler(src); + } catch (error) { + console.error("Error deleting file via delete utility plugin:", error); + } + }) + );
24-33: Guard against undefined/empty src when buildingnewFileSources.Avoid inserting non-strings into Sets to keep
.haschecks reliable.- newState.doc.descendants((node) => { + newState.doc.descendants((node) => { const nodeType = node.type.name as CORE_EXTENSIONS; const nodeFileSetDetails = NODE_FILE_MAP[nodeType]; if (nodeFileSetDetails) { - if (newFileSources[nodeType]) { - newFileSources[nodeType].add(node.attrs.src); + const src = node.attrs.src as string | undefined; + if (typeof src !== "string" || src.length === 0) return; + if (newFileSources[nodeType]) { + newFileSources[nodeType]!.add(src); } else { - newFileSources[nodeType] = new Set([node.attrs.src]); + newFileSources[nodeType] = new Set([src]); } } });packages/editor/src/core/components/menus/menu-items.ts (1)
155-161: UseTASK_LISTfor To-do list active state
Replaceeditor.isActive(CORE_EXTENSIONS.TASK_ITEM)witheditor.isActive(CORE_EXTENSIONS.TASK_LIST)inTodoListItem(packages/editor/src/core/components/menus/menu-items.ts:158) to match other list toggles.
🧹 Nitpick comments (37)
packages/editor/src/core/plugins/file/restore.ts (5)
37-81: Unnecessary O(k) rescans and duplicate work across transactions; collapse to a single pass.
transactions.forEach(() => { ... })ignores eachtrand re-walksnewState.docfor every transaction in the batch. This is redundant and increases cost on large docs.Apply:
- transactions.forEach(() => { + { const addedFiles: TFileNode[] = []; // ... - addedFiles.forEach(async (node) => { + addedFiles.forEach(async (node) => { // ... - }); - }); + }); + }
26-34: Harden type narrowing: guard node types and src before using them.Blindly asserting
node.type.name as CORE_EXTENSIONSand readingnode.attrs.srcrisks addingundefinedto aSet<string>and hides mismatches between nodeType andNODE_FILE_MAP.- const nodeType = node.type.name as CORE_EXTENSIONS; - const nodeFileSetDetails = NODE_FILE_MAP[nodeType]; - if (nodeFileSetDetails) { - if (oldFileSources[nodeType]) { - oldFileSources[nodeType].add(node.attrs.src); - } else { - oldFileSources[nodeType] = new Set([node.attrs.src]); - } - } + const nodeTypeName = node.type.name as string; + if (!(nodeTypeName in NODE_FILE_MAP)) return; + const nodeType = nodeTypeName as CORE_EXTENSIONS; + const src = node.attrs?.src as unknown; + if (typeof src !== "string" || src.length === 0) return; + const nodeFileSetDetails = NODE_FILE_MAP[nodeType]; + if (nodeFileSetDetails) { + if (oldFileSources[nodeType]) { + oldFileSources[nodeType]!.add(src); + } else { + oldFileSources[nodeType] = new Set([src]); + } + }
41-58: Repeat the same guards in the newState scan; drop redundant pos bounds check.
- Use the same safe narrowing and
srcchecks.posfromdescendantsis always valid; the bounds check is redundant.- const nodeType = node.type.name as CORE_EXTENSIONS; - const isAValidNode = NODE_FILE_MAP[nodeType]; - // if the node doesn't match, then return as no point in checking - if (!isAValidNode) return; - if (pos < 0 || pos > newState.doc.content.size) return; - if (oldFileSources[nodeType]?.has(node.attrs.src)) return; + const nodeTypeName = node.type.name as string; + if (!(nodeTypeName in NODE_FILE_MAP)) return; + const nodeType = nodeTypeName as CORE_EXTENSIONS; + const src = node.attrs?.src as unknown; + if (typeof src !== "string" || src.length === 0) return; + if (oldFileSources[nodeType]?.has(src)) return; // update assets list storage value const assetMetaData = CORE_ASSETS_META_DATA_RECORD[nodeType]?.(node.attrs); if (assetMetaData) { editor.commands.updateAssetsList?.({ asset: assetMetaData, }); } // if the src is just a id (private bucket), then we don't need to handle restore from here but // only while it fails to load - if (nodeType === CORE_EXTENSIONS.CUSTOM_IMAGE && !node.attrs.src?.startsWith("http")) return; + if (nodeType === CORE_EXTENSIONS.CUSTOM_IMAGE && !src.startsWith("http")) return; addedFiles.push(node as TFileNode);
14-14: Namespace the PluginKey to avoid collisions across editor instances.A generic key like
"restore-utility"risks collision if multiple plugins use similar keys. Prefix with your app namespace.-const RESTORE_PLUGIN_KEY = new PluginKey("restore-utility"); +const RESTORE_PLUGIN_KEY = new PluginKey("plane.restore-utility");
64-69: Redundant check for nodeFileSetDetails; remove duplicate condition.You already guard
!nodeFileSetDetailsearlier; the laterif (!nodeFileSetDetails || !src) return;repeats it.- const wasDeleted = extensionFileSetStorage?.get(src); - if (!nodeFileSetDetails || !src) return; + const wasDeleted = extensionFileSetStorage?.get(src); + if (!src) return;packages/editor/src/core/extensions/emoji/suggestion.ts (3)
2-3: Type source: preferEditorfrom@tiptap/core.Avoids accidental version skew and keeps types aligned with core.
-import { ReactRenderer, type Editor } from "@tiptap/react"; +import { ReactRenderer } from "@tiptap/react"; +import type { Editor } from "@tiptap/core";
57-57: Guard against duplicate registrations; fully clean up on exit.Prevents multiple entries and ensures complete removal.
- editor.storage.utility.activeDropbarExtensions.push(CORE_EXTENSIONS.EMOJI); + { + const { activeDropbarExtensions } = editor.storage.utility; + if (!activeDropbarExtensions.includes(CORE_EXTENSIONS.EMOJI)) { + activeDropbarExtensions.push(CORE_EXTENSIONS.EMOJI); + } + } @@ - const index = activeDropbarExtensions.indexOf(CORE_EXTENSIONS.EMOJI); - if (index > -1) { - activeDropbarExtensions.splice(index, 1); - } + let idx: number; + while ((idx = activeDropbarExtensions.indexOf(CORE_EXTENSIONS.EMOJI)) > -1) { + activeDropbarExtensions.splice(idx, 1); + }Also applies to: 101-105
47-49: Avoid double-destroy; null outcomponentafter destroy.Minor hygiene to prevent repeated
.destroy()calls.- let component: ReactRenderer<EmojiListRef>; + let component: ReactRenderer<EmojiListRef> | null = null; @@ - if (component) { - component.destroy(); - } + if (component) { + component.destroy(); + component = null; + } @@ - if (component) { - component.destroy(); - } + if (component) { + component.destroy(); + component = null; + }Also applies to: 89-91, 110-111
packages/editor/tsconfig.json (1)
8-8: Good move enabling strict mode; consider tightening and de-duplicating options.
- Redundant: "strictNullChecks": true is implied by "strict". You can drop it for clarity.
- Optional: enable "noUncheckedIndexedAccess": true to surface unsafe map indexing (useful for editor.storage and NODE_FILE_MAP access).
Apply:
"noEmit": true, "strict": true, "skipLibCheck": true, + "noUncheckedIndexedAccess": true, "sourceMap": true, @@ - "strictNullChecks": true,packages/editor/tsdown.config.ts (1)
10-10: Minify only in production to keep dev builds readable.Minifying in watch/dev mode slows iteration and obscures stack traces. Gate it by NODE_ENV; also consider cleaning dist on prod builds.
export default defineConfig({ @@ - sourcemap: true, - minify: true, + sourcemap: true, + minify: process.env.NODE_ENV === "production", copy: ["src/styles"], });Optionally make
clean: process.env.NODE_ENV === "production".packages/editor/src/ce/constants/utility.ts (1)
1-18: Type-safe literal map: preferas constwithsatisfiesto avoid widening.Locks keys/values and keeps compiler checks without losing literal types.
-import { CORE_EXTENSIONS } from "@/constants/extension"; +import { CORE_EXTENSIONS } from "@/constants/extension"; @@ -export const NODE_FILE_MAP: Partial< - Record< - CORE_EXTENSIONS, - { - fileSetName: ExtensionFileSetStorageKey; - } - > -> = { +export const NODE_FILE_MAP = { [CORE_EXTENSIONS.IMAGE]: { fileSetName: "deletedImageSet", }, [CORE_EXTENSIONS.CUSTOM_IMAGE]: { fileSetName: "deletedImageSet", }, -}; +} as const satisfies Partial< + Record<CORE_EXTENSIONS, { fileSetName: ExtensionFileSetStorageKey }> +>;packages/editor/src/core/plugins/file/delete.ts (2)
25-26: Replace casts with a type guard to narrow node types safely.Reduces reliance on
as CORE_EXTENSIONSand aligns with TS strictness.Add a helper:
const isCoreExt = (name: string): name is CORE_EXTENSIONS => Object.prototype.hasOwnProperty.call(NODE_FILE_MAP, name);Then:
-const nodeType = node.type.name as CORE_EXTENSIONS; +const name = node.type.name; +if (!isCoreExt(name)) return; +const nodeType = name;Also applies to: 44-45, 55-56
60-62: Remove@ts-expect-errorby declaring storage types via module augmentation.Define the shape of
editor.storage[CORE_EXTENSIONS]so.setis type-safe.Create a types file (e.g., packages/editor/src/core/types/editor-storage.d.ts):
import "@tiptap/core"; declare module "@tiptap/core" { interface Commands<ReturnType> { updateAssetsList?: (args: { idToRemove?: string }) => ReturnType; } interface ImageExtensionStorage { deletedImageSet: Map<string, boolean>; } interface Storage { image?: ImageExtensionStorage; customImage?: ImageExtensionStorage; } }Then remove the
@ts-expect-errorhere.packages/editor/package.json (2)
26-26: Build script OK; consider skipping full TSC emit-less run during build.You already run type checks via "check:types". To speed builds, you can rely on tsdown’s dts emission and keep
tsc --noEmitonly in CI/check scripts.-"build": "tsc && tsdown", +"build": "tsdown",Or keep as-is if you prefer belt-and-suspenders in builds.
49-66: Remove stray tippy.js dependency
Drop the"tippy.js": "^6.3.7"entry frompackages/editor/package.json(line 75)—there are no remaining imports or requires oftippy.jsin the codebase.packages/editor/src/core/extensions/callout/block.tsx (1)
26-26: Use enum when reading the attribute for consistencyAccessing via node.attrs[ECalloutAttributeNames.BACKGROUND] avoids drift between literals and enum values and benefits from key typing.
-const activeBackgroundColor = COLORS_LIST.find((c) => node.attrs["data-background"] === c.key)?.backgroundColor; +const activeBackgroundColor = COLORS_LIST.find( + (c) => node.attrs[ECalloutAttributeNames.BACKGROUND] === c.key +)?.backgroundColor;packages/editor/src/core/extensions/callout/types.ts (1)
21-26: Optional: tighten attribute value typing via “as const” defaultsIf DEFAULT_CALLOUT_BLOCK_ATTRIBUTES is marked as const, TCalloutBlockAttributes literal types (e.g., "callout-component") remain narrow through inference. Not required, but helps catch accidental widening.
packages/editor/src/core/extensions/callout/extension.tsx (1)
49-53: Minor: Backspace guard could use childCount instead of content.sizecontent.size > 2 relies on internals; checking node.childCount > 0 better expresses intent. Not blocking.
-if (parentNodeDetails.node.content.size > 2 && isCursorAtCalloutBeginning) { +if (parentNodeDetails.node.childCount > 0 && isCursorAtCalloutBeginning) {packages/editor/src/core/extensions/callout/utils.ts (1)
33-43: Avoid sanitizing JSON before parsing; sanitize only on renderSanitizing the raw JSON string can invalidate it. Read raw, parse, and sanitize only when inserting into the DOM.
-const storedData = sanitizeHTML(localStorage.getItem("editor-calloutComponent-logo") ?? ""); +const storedData = localStorage.getItem("editor-calloutComponent-logo") ?? "";packages/editor/src/core/components/menus/block-menu.tsx (6)
12-13: Use type-only import for LucideIcon; drop unused props.
- Import LucideIcon as a type to avoid bundling runtime value.
- Props exposes disabledExtensions/flaggedExtensions but they are unused here—remove to prevent API drift.
-import { Copy, LucideIcon, Trash2 } from "lucide-react"; +import type { LucideIcon } from "lucide-react"; +import { Copy, Trash2 } from "lucide-react"; @@ -type Props = { - disabledExtensions?: IEditorProps["disabledExtensions"]; - editor: Editor; - flaggedExtensions?: IEditorProps["flaggedExtensions"]; -}; +type Props = { + editor: Editor; +};Also applies to: 18-24
49-63: Avoid duplicate IDs; prefer a data attribute for the drag handle.Querying by id "#drag-handle" risks multiple identical ids across the document and is brittle. Prefer a data attribute selector.
- const dragHandle = target.closest("#drag-handle"); + const dragHandle = target.closest("[data-drag-handle='true']");And update the handle element(s) to use
data-drag-handle="true"instead ofid="drag-handle".Also applies to: 69-75
77-101: Tighten global listeners: passive scroll; consider delegating Escape to useDismiss.
- Add
{ passive: true }to the scroll listener for smoother scrolling.- Optional:
useDismissalready handles Escape; you can drop the document keydown handler to avoid duplication.- document.addEventListener("scroll", handleScroll, true); + document.addEventListener("scroll", handleScroll, { capture: true, passive: true }); @@ - document.removeEventListener("scroll", handleScroll, true); + document.removeEventListener("scroll", handleScroll, { capture: true } as any);
141-176: Duplication: rely on updateSelection; drop manual position math in focus.
insertContentAt(..., { updateSelection: true })already moves the selection to the inserted content in v3. Passing a numeric position tofocus()can be off-by-one for block nodes and becomes stale if the doc changes. Prefer restoring focus without moving the cursor and disable scroll.- .insertContentAt(insertPos, contentToInsert, { - updateSelection: true, - }) - .focus(Math.min(insertPos + 1, docSize), { scrollIntoView: false }) + .insertContentAt(insertPos, contentToInsert, { updateSelection: true }) + .focus(null, { scrollIntoView: false }) .run();This matches Tiptap v3 docs for
insertContentAtandfocus. (tiptap.dev)
191-207: Add a11y roles/labels to the floating panel.Expose menu semantics for screen readers.
- <div + <div + role="menu" + aria-label="Block options"
209-219: Don’t remove disabled items from the list; keep them visible but disabled.Rendering
nullhides options and can hurt discoverability. Keep items withdisabled/aria-disabledfor clarity.- {MENU_ITEMS.map((item) => { - if (item.isDisabled) return null; - return ( + {MENU_ITEMS.map((item) => { + return ( <button key={item.key} type="button" className="flex w-full items-center gap-1.5 truncate rounded px-1 py-1.5 text-xs text-custom-text-200 hover:bg-custom-background-90" onClick={item.onClick} - disabled={item.isDisabled} + disabled={item.isDisabled} + aria-disabled={item.isDisabled || undefined} >packages/editor/src/core/components/menus/menu-items.ts (3)
115-137: Drop unnecessary optional chaining oneditorfor cleaner types.
editoris non-null; optional chaining widens types and is redundant.- isActive: () => editor?.isActive(CORE_EXTENSIONS.BOLD), + isActive: () => editor.isActive(CORE_EXTENSIONS.BOLD), @@ - isActive: () => editor?.isActive(CORE_EXTENSIONS.ITALIC), + isActive: () => editor.isActive(CORE_EXTENSIONS.ITALIC), @@ - isActive: () => editor?.isActive(CORE_EXTENSIONS.UNDERLINE), + isActive: () => editor.isActive(CORE_EXTENSIONS.UNDERLINE), @@ - isActive: () => editor?.isActive(CORE_EXTENSIONS.STRIKETHROUGH), + isActive: () => editor.isActive(CORE_EXTENSIONS.STRIKETHROUGH), @@ - isActive: () => editor?.isActive(CORE_EXTENSIONS.BULLET_LIST), + isActive: () => editor.isActive(CORE_EXTENSIONS.BULLET_LIST), @@ - isActive: () => editor?.isActive(CORE_EXTENSIONS.ORDERED_LIST), + isActive: () => editor.isActive(CORE_EXTENSIONS.ORDERED_LIST), @@ - isActive: () => editor?.isActive(CORE_EXTENSIONS.BLOCKQUOTE), + isActive: () => editor.isActive(CORE_EXTENSIONS.BLOCKQUOTE), @@ - isActive: () => editor?.isActive(CORE_EXTENSIONS.CODE_INLINE) || editor?.isActive(CORE_EXTENSIONS.CODE_BLOCK), + isActive: () => editor.isActive(CORE_EXTENSIONS.CODE_INLINE) || editor.isActive(CORE_EXTENSIONS.CODE_BLOCK), @@ - isActive: () => editor?.isActive(CORE_EXTENSIONS.TABLE), + isActive: () => editor.isActive(CORE_EXTENSIONS.TABLE), @@ - isActive: () => editor?.isActive(CORE_EXTENSIONS.IMAGE) || editor?.isActive(CORE_EXTENSIONS.CUSTOM_IMAGE), + isActive: () => editor.isActive(CORE_EXTENSIONS.IMAGE) || editor.isActive(CORE_EXTENSIONS.CUSTOM_IMAGE), @@ - isActive: () => editor?.isActive(CORE_EXTENSIONS.HORIZONTAL_RULE), + isActive: () => editor.isActive(CORE_EXTENSIONS.HORIZONTAL_RULE), @@ - isActive: () => editor?.isActive("link"), + isActive: () => editor.isActive("link"),Also applies to: 139-153, 155-161, 163-177, 179-185, 187-193, 195-203, 204-216, 217-237, 239-248
204-216: Consistency: prefer constant over string literal for link.If
CORE_EXTENSIONS.LINKexists, use it for consistency.- isActive: () => editor?.isActive("link"), + isActive: () => editor.isActive(CORE_EXTENSIONS.LINK),
250-278: Avoid broad cast at return; usesatisfiesfor safer typing.This keeps concrete keys while asserting the array shape.
-export const getEditorMenuItems = (editor: Editor | null): EditorMenuItem<TEditorCommands>[] => { +export const getEditorMenuItems = (editor: Editor | null) => { if (!editor) return []; - return [ + const items = [ // ... - ] as EditorMenuItem<TEditorCommands>[]; + ] satisfies EditorMenuItem<TEditorCommands>[]; + return items; };packages/editor/src/core/components/menus/bubble-menu/node-selector.tsx (3)
1-4: Import Editor as a type only.Avoid a runtime import when only used for typing.
-import { Editor } from "@tiptap/react"; +import type { Editor } from "@tiptap/react";
79-83: Use a stable, non‑localized key.Use
item.keyinstead ofitem.nameto avoid key churn if labels change (i18n).- <button - key={item.name} + <button + key={item.key}
77-101: Menu roles for a11y.Ensure the floating panel has
role="menu"and items exposerole="menuitemradio"witharia-checkedfor the active entry. You can pass a role touseRolein the hook.- const { options, getReferenceProps, getFloatingProps } = useFloatingMenu({}); + const { options, getReferenceProps, getFloatingProps } = useFloatingMenu({}); @@ - <section className="w-48 max-h-[90vh] mt-1 flex flex-col overflow-y-scroll rounded-md border-[0.5px] border-custom-border-300 bg-custom-background-100 px-2 py-2.5 shadow-custom-shadow-rg"> + <section + role="menu" + className="w-48 max-h-[90vh] mt-1 flex flex-col overflow-y-scroll rounded-md border-[0.5px] border-custom-border-300 bg-custom-background-100 px-2 py-2.5 shadow-custom-shadow-rg" + > @@ - > + role="menuitemradio" + aria-checked={activeItem.name === item.name} + >Alternatively, update
useFloatingMenuto calluseRole(context, { role: 'menu' })so the container receives the correct role.packages/editor/src/core/components/menus/bubble-menu/root.tsx (3)
146-172: Harden event listener cleanup during unmount.
If the component unmounts mid‑drag (before mouseup), mousemove/mouseup listeners remain until the next mouseup. Store the handlers and remove them in the effect cleanup as a fallback.Apply this minimal refactor:
@@ - const menuRef = useRef<HTMLDivElement>(null); + const menuRef = useRef<HTMLDivElement>(null); + const moveHandlerRef = useRef<((e: MouseEvent) => void) | null>(null); + const upHandlerRef = useRef<((e: MouseEvent) => void) | null>(null); @@ - function handleMouseDown(e: MouseEvent) { + function handleMouseDown(e: MouseEvent) { if (menuRef.current?.contains(e.target as Node)) return; - function handleMouseMove() { + const handleMouseMove = () => { if (!editor.state.selection.empty) { setIsSelecting(true); document.removeEventListener("mousemove", handleMouseMove); } - } + }; - function handleMouseUp() { + const handleMouseUp = () => { setIsSelecting(false); document.removeEventListener("mousemove", handleMouseMove); document.removeEventListener("mouseup", handleMouseUp); - } + }; document.addEventListener("mousemove", handleMouseMove); document.addEventListener("mouseup", handleMouseUp); + moveHandlerRef.current = handleMouseMove; + upHandlerRef.current = handleMouseUp; } @@ - return () => { - document.removeEventListener("mousedown", handleMouseDown); - }; + return () => { + document.removeEventListener("mousedown", handleMouseDown); + if (moveHandlerRef.current) { + document.removeEventListener("mousemove", moveHandlerRef.current); + moveHandlerRef.current = null; + } + if (upHandlerRef.current) { + document.removeEventListener("mouseup", upHandlerRef.current); + upHandlerRef.current = null; + } + };
72-81: Memoize formattingItems to avoid recreating command objects on every render.
Small perf/alloc win; stable identities also help future optimizations.- const formattingItems = { - code: CodeItem(editor), - bold: BoldItem(editor), - italic: ItalicItem(editor), - underline: UnderLineItem(editor), - strikethrough: StrikeThroughItem(editor), - "text-align": TextAlignItem(editor), - } satisfies { - [K in TEditorCommands]?: EditorMenuItem<K>; - }; + const formattingItems = useMemo(() => ({ + code: CodeItem(editor), + bold: BoldItem(editor), + italic: ItalicItem(editor), + underline: UnderLineItem(editor), + strikethrough: StrikeThroughItem(editor), + "text-align": TextAlignItem(editor), + } satisfies { [K in TEditorCommands]?: EditorMenuItem<K> }, [editor]);
176-185: Add basic ARIA for the toolbar container.
Improves accessibility with negligible cost.- {!isSelecting && ( - <div - ref={menuRef} - className="flex py-2 divide-x divide-custom-border-200 rounded-lg border border-custom-border-200 bg-custom-background-100 shadow-custom-shadow-rg overflow-x-scroll horizontal-scrollbar scrollbar-xs" - > + {!isSelecting && ( + <div + ref={menuRef} + role="toolbar" + aria-label="Text formatting" + className="flex py-2 divide-x divide-custom-border-200 rounded-lg border border-custom-border-200 bg-custom-background-100 shadow-custom-shadow-rg overflow-x-auto horizontal-scrollbar scrollbar-xs" + >packages/editor/src/core/extensions/callout/extension-config.ts (3)
19-19: Type the extension storage instead of usingunknown.
Tiptap v3’sNode.create<Options, Storage>benefits from a precise storage type foreditor.storage. Minimal change:-export const CustomCalloutExtensionConfig = Node.create<unknown, unknown>({ +export const CustomCalloutExtensionConfig = Node.create< + unknown, + { markdown: { serialize(state: MarkdownSerializerState, node: NodeType): void } } +>({Based on learnings
27-36: Tighten typing ofaddAttributesbuilder; avoidanyleakage fromObject.values.
Object.values(enum)is loosely typed, andRecord<ECalloutAttributeNames, …>loses key-specific defaults. Build from the defaults’ keys and preserve per-key types:- ...Object.values(ECalloutAttributeNames).reduce( - (acc, value) => { - acc[value] = { - default: DEFAULT_CALLOUT_BLOCK_ATTRIBUTES[value], - }; - return acc; - }, - {} as Record<ECalloutAttributeNames, { default: TCalloutBlockAttributes[ECalloutAttributeNames] }> - ), + ...Object.fromEntries( + (Object.keys(DEFAULT_CALLOUT_BLOCK_ATTRIBUTES) as ECalloutAttributeNames[]).map( + (key) => [key, { default: DEFAULT_CALLOUT_BLOCK_ATTRIBUTES[key] }] + ) + ) as { [K in ECalloutAttributeNames]: { default: TCalloutBlockAttributes[K] } },
45-54: Replace magic string keys with enum members for robustness.
Keeps serialization aligned with future enum/name changes.- const attrs = node.attrs as TCalloutBlockAttributes; - const logoInUse = attrs["data-logo-in-use"]; + const attrs = node.attrs as TCalloutBlockAttributes; + const logoInUse = attrs[ECalloutAttributeNames.LOGO_IN_USE]; // add callout logo if (logoInUse === "emoji") { state.write( - `> <img src="${attrs["data-emoji-url"]}" alt="${attrs["data-emoji-unicode"]}" width="30px" />\n` + `> <img src="${attrs[ECalloutAttributeNames.EMOJI_URL]}" alt="${attrs[ECalloutAttributeNames.EMOJI_UNICODE]}" width="30px" />\n` ); } else { - state.write(`> <icon>${attrs["data-icon-name"]} icon</icon>\n`); + state.write(`> <icon>${attrs[ECalloutAttributeNames.ICON_NAME]} icon</icon>\n`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
packages/editor/package.json(3 hunks)packages/editor/src/ce/constants/utility.ts(1 hunks)packages/editor/src/core/components/menus/block-menu.tsx(6 hunks)packages/editor/src/core/components/menus/bubble-menu/node-selector.tsx(3 hunks)packages/editor/src/core/components/menus/bubble-menu/root.tsx(9 hunks)packages/editor/src/core/components/menus/menu-items.ts(4 hunks)packages/editor/src/core/extensions/callout/block.tsx(2 hunks)packages/editor/src/core/extensions/callout/extension-config.ts(3 hunks)packages/editor/src/core/extensions/callout/extension.tsx(2 hunks)packages/editor/src/core/extensions/callout/types.ts(2 hunks)packages/editor/src/core/extensions/callout/utils.ts(2 hunks)packages/editor/src/core/extensions/code/code-block-node-view.tsx(1 hunks)packages/editor/src/core/extensions/emoji/extension.ts(1 hunks)packages/editor/src/core/extensions/emoji/suggestion.ts(4 hunks)packages/editor/src/core/plugins/file/delete.ts(4 hunks)packages/editor/src/core/plugins/file/restore.ts(4 hunks)packages/editor/tsconfig.json(1 hunks)packages/editor/tsdown.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/editor/src/core/extensions/code/code-block-node-view.tsx
🧰 Additional context used
🧬 Code graph analysis (10)
packages/editor/src/core/extensions/emoji/suggestion.ts (2)
packages/editor/src/core/extensions/emoji/emoji.ts (2)
EmojiOptions(71-77)EmojiItem(32-69)packages/editor/src/core/extensions/emoji/components/emojis-list.tsx (1)
EmojiItem(8-14)
packages/editor/src/ce/constants/utility.ts (1)
packages/editor/src/ce/types/storage.ts (1)
ExtensionFileSetStorageKey(4-4)
packages/editor/src/core/extensions/callout/extension-config.ts (2)
packages/editor/src/core/extensions/callout/utils.ts (1)
DEFAULT_CALLOUT_BLOCK_ATTRIBUTES(12-20)packages/editor/src/core/extensions/callout/types.ts (1)
TCalloutBlockAttributes(21-26)
packages/editor/src/core/plugins/file/restore.ts (2)
packages/editor/src/index.ts (1)
CORE_EXTENSIONS(16-16)packages/editor/src/ce/constants/utility.ts (1)
NODE_FILE_MAP(4-18)
packages/editor/src/core/components/menus/bubble-menu/node-selector.tsx (4)
packages/editor/src/core/components/menus/floating-menu/use-floating-menu.ts (1)
useFloatingMenu(17-50)packages/editor/src/core/components/menus/menu-items.ts (1)
EditorMenuItem(57-63)packages/editor/src/core/types/editor.ts (1)
TEditorCommands(31-60)packages/editor/src/core/components/menus/floating-menu/root.tsx (1)
FloatingMenuRoot(21-65)
packages/editor/src/core/plugins/file/delete.ts (2)
packages/editor/src/index.ts (1)
CORE_EXTENSIONS(16-16)packages/editor/src/ce/constants/utility.ts (1)
NODE_FILE_MAP(4-18)
packages/editor/src/core/components/menus/bubble-menu/root.tsx (6)
packages/editor/src/core/components/menus/menu-items.ts (7)
CodeItem(171-177)BoldItem(107-113)ItalicItem(115-121)UnderLineItem(123-129)StrikeThroughItem(131-137)TextAlignItem(239-248)EditorMenuItem(57-63)packages/editor/src/core/types/editor.ts (1)
TEditorCommands(31-60)packages/editor/src/core/components/menus/bubble-menu/node-selector.tsx (1)
BubbleMenuNodeSelector(32-104)packages/editor/src/core/components/menus/bubble-menu/link-selector.tsx (1)
BubbleMenuLinkSelector(18-121)packages/editor/src/core/components/menus/bubble-menu/color-selector.tsx (1)
BubbleMenuColorSelector(19-113)packages/editor/src/core/components/menus/bubble-menu/alignment-selector.tsx (1)
TextAlignmentSelector(16-82)
packages/editor/src/core/extensions/callout/utils.ts (1)
packages/editor/src/core/extensions/callout/types.ts (3)
TCalloutBlockAttributes(21-26)TCalloutBlockEmojiAttributes(16-19)TCalloutBlockIconAttributes(11-14)
packages/editor/src/core/components/menus/menu-items.ts (1)
packages/editor/src/core/types/editor.ts (1)
TEditorCommands(31-60)
packages/editor/src/core/components/menus/block-menu.tsx (1)
packages/editor/src/core/types/editor.ts (1)
IEditorProps(141-168)
🔇 Additional comments (22)
packages/editor/src/core/plugins/file/restore.ts (1)
48-53: Defer asset updates outside appendTransaction
DispatchingupdateAssetsListinsideappendTransactionrisks nested-dispatch errors or infinite loops if it issues a new transaction. Wrap the call in a microtask (or emit a transaction meta for post-processing).- if (assetMetaData) { - editor.commands.updateAssetsList?.({ - asset: assetMetaData, - }); - } + if (assetMetaData) { + // Defer to avoid dispatch during appendTransaction + queueMicrotask(() => { + if (!editor.isDestroyed) { + editor.commands.updateAssetsList?.({ asset: assetMetaData }); + } + }); + }packages/editor/src/core/extensions/emoji/extension.ts (1)
10-11: Good fix: capture options to avoid “this” pitfalls inside serializer.This prevents accidental binding issues in nested callbacks. LGTM.
If you expect runtime reconfiguration of emojis/options after init, confirm this closure is acceptable; otherwise consider reading from storage in the serializer for greater resilience.
Also applies to: 16-16
packages/editor/package.json (4)
60-60: Avoid aggregated “@tiptap/extensions”; import per‑package instead.Using the umbrella package bloats bundles and complicates tree‑shaking. Replace imports with individual packages (e.g., @tiptap/extension-placeholder, @tiptap/extension-character-count) and drop this dependency.
#!/bin/bash # Find umbrella imports to replace rg -nP "from ['\"]@tiptap/extensions['\"]|require\\(['\"]@tiptap/extensions['\"]\\)"
66-66: Peer mismatch: @tiptap/y-tiptap vs collaboration extension.
@tiptap/extension-collaboration@^3.5.3expects@tiptap/y-tiptap@^3.0.0-beta.3. Pin the beta, downgrade collaboration, or add an override.Suggested remedies:
- "@tiptap/y-tiptap": "^3.0.0", + "@tiptap/y-tiptap": "3.0.0-beta.3",or
- "@tiptap/extension-collaboration": "^3.5.3", + "@tiptap/extension-collaboration": "3.0.0",or add an overrides block mapping the peer.
49-66: Audit TipTap v3 ecosystem compatibility in deps block.
- tiptap-markdown@^0.8.10 targets TipTap v2; replace with a v3-compatible alternative or remove usage.
- Keep only specific extension packages; avoid umbrella deps.
#!/bin/bash # Confirm markdown plugin version and imports npm view tiptap-markdown version peerDependencies 2>/dev/null || true rg -nP --glob '!**/dist/**' "from ['\"]tiptap-markdown['\"]|require\\(['\"]tiptap-markdown['\"]\\)"
90-90: Confirmuuidresolution and remove redundant@types/uuid
The editor package depends on"uuid": "catalog:", which isn’t a fixed version. Runpnpm list uuid --filter ./packages/editor --depth 0to see which version is installed. If it’s v9 or above—which bundles its own types—remove
@types/uuidfrompackages/editor/package.jsonto prevent conflicts.packages/editor/src/core/extensions/callout/block.tsx (1)
48-49: Good switch to enum-based attribute keyUsing [ECalloutAttributeNames.BACKGROUND] in updateAttributes keeps types aligned across the extension.
packages/editor/src/core/extensions/callout/types.ts (1)
1-9: Enum rename looks correctKeys map to the expected data-* attributes and match downstream usage.
packages/editor/src/core/extensions/callout/extension.tsx (1)
28-29: Nice: using CORE_EXTENSIONS.PARAGRAPH avoids hard-coded stringsKeeps node names centralized; good for future refactors.
packages/editor/src/core/extensions/callout/utils.ts (2)
12-20: LGTM: defaults now use enum keysThis aligns all attribute access behind a single enum source of truth.
73-78: Background color retrieval is fineReturning a sanitized string protects CSS injection when later applied as inline style.
packages/editor/src/core/components/menus/block-menu.tsx (2)
35-42: LGTM: autoUpdate + middleware on useFloating.Virtual reference + autoUpdate + left-start placement is a solid setup; no issues spotted.
49-75: Drop duplicate-ID and listener-duplication concerns
The only#drag-handleID is set inpackages/editor/src/core/plugins/drag-handle.tsand is guarded by aquerySelectorcheck inside-menu.ts, so no static or duplicate IDs exist in TSX. The mouse-down handler is memoized with a stablerefsdependency, resulting in a single listener registration.Likely an incorrect or invalid review comment.
packages/editor/src/core/components/menus/menu-items.ts (2)
1-1: Nice: type-only imports for Editor and LucideIcon.Keeps bundles lean; no concerns.
Also applies to: 21-22
73-87: Heading item helpers look correct and read well.Clearer public surface and tighter typing; good.
Also applies to: 89-106
packages/editor/src/core/components/menus/bubble-menu/node-selector.tsx (1)
35-37: LGTM: shared Floating UI via useFloatingMenu/FloatingMenuRoot.The consolidation improves consistency and removes local isOpen plumbing.
Also applies to: 57-76
packages/editor/src/core/components/menus/bubble-menu/root.tsx (3)
1-3: Correct v3 imports for BubbleMenu.
Importing BubbleMenu and its types from @tiptap/react/menus aligns with Tiptap v3. No issues here.
103-121: Bubble menu visibility guard looks solid.
The shouldShow check covers empty selections, non‑editable state, node/cell selections, and drag‑select in progress. LGTM.
122-144: Finish Floating UI migration: move handlers out of options; add middleware; remove tippy remnants.
- onShow/onHide shouldn’t live inside options; use top‑level props.
- Provide Floating UI options (placement, middleware, update/resize debounce).
- Remove the commented tippyOptions block.
- Consider a unique pluginKey to avoid collisions if multiple BubbleMenus exist.
Apply this diff within bubbleMenuProps:
const bubbleMenuProps: EditorBubbleMenuProps = { - editor, + editor, + pluginKey: 'bubble-menu-root', shouldShow: ({ state, editor }) => { ... }, - options: { - onShow: () => { - if (editor.storage.link) { - editor.storage.link.isBubbleMenuOpen = true; - } - editor.commands.addActiveDropbarExtension("bubble-menu"); - }, - onHide: () => { - if (editor.storage.link) { - editor.storage.link.isBubbleMenuOpen = false; - } - setTimeout(() => { - editor.commands.removeActiveDropbarExtension("bubble-menu"); - }, 0); - }, - }, - // TODO: Migrate these to floating UI options - // tippyOptions: { - // moveTransition: "transform 0.15s ease-out", - // duration: [300, 0], - // zIndex: 9, - // }, + onShow: () => { + if (editor.storage.link) editor.storage.link.isBubbleMenuOpen = true; + editor.commands.addActiveDropbarExtension("bubble-menu"); + }, + onHide: () => { + if (editor.storage.link) editor.storage.link.isBubbleMenuOpen = false; + setTimeout(() => editor.commands.removeActiveDropbarExtension("bubble-menu"), 0); + }, + options: { + placement: 'top', + middleware: [offset(6), flip(), shift()], + updateDelay: 250, + resizeDelay: 100, + // zIndex: 9, // if supported; otherwise style via CSS + }, };Add required imports (outside the hunk):
import { offset, flip, shift } from "@floating-ui/dom";To confirm the exact v3 prop names (options, updateDelay/resizeDelay, top‑level onShow/onHide), run:
Tiptap v3 BubbleMenu React props: where to pass Floating UI options, and are onShow/onHide top-level or under options? Does BubbleMenu support pluginKey and updateDelay/resizeDelay in v3?packages/editor/src/core/extensions/callout/extension-config.ts (3)
7-7: Import rename aligns with enum/type migration.
Looks consistent with the rest of the callout types.
68-69: Selector update looks correct.
UsingECalloutAttributeNames.BLOCK_TYPEwith the default value keeps parse rules in sync.
1-77: Update repo-wide verification script for leftover names
The previous ripgrep commands used--type=tsx/--type=tsm, which aren’t recognized. Replace them with glob filters for.ts,.tsx, and.tsm:#!/bin/bash set -euo pipefail echo "Searching for old enum name usages…" rg -nP '\bEAttributeNames\b' -g '*.ts' -g '*.tsx' -g '*.tsm' || true echo "Searching for callout magic data-* strings…" rg -nP '"data-(logo-in-use|emoji-url|emoji-unicode|icon-name|background|block-type)"' -g '*.ts' -g '*.tsx' -g '*.tsm' || trueRun this to confirm no stray enum references or magic
data-*keys remain.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/editor/src/core/extensions/callout/extension-config.ts (1)
46-60: Consider using enum values instead of string literalsThe serialize method uses string literals for attribute access (lines 46, 50, 51, 53) instead of the defined enum constants. This bypasses type safety.
serialize(state: MarkdownSerializerState, node: ProseMirrorNode) { const attrs = node.attrs as TCalloutBlockAttributes; - const logoInUse = attrs["data-logo-in-use"]; + const logoInUse = attrs[ECalloutAttributeNames.LOGO_IN_USE]; // add callout logo if (logoInUse === "emoji") { state.write( - `> <img src="${attrs["data-emoji-url"]}" alt="${attrs["data-emoji-unicode"]}" width="30px" />\n` + `> <img src="${attrs[ECalloutAttributeNames.EMOJI_URL]}" alt="${attrs[ECalloutAttributeNames.EMOJI_UNICODE]}" width="30px" />\n` ); } else { - state.write(`> <icon>${attrs["data-icon-name"]} icon</icon>\n`); + state.write(`> <icon>${attrs[ECalloutAttributeNames.ICON_NAME]} icon</icon>\n`); }packages/editor/src/core/extensions/custom-image/extension-config.ts (1)
16-24: Computed property names in interface augmentation will not compileTypeScript only allows computed names in interfaces for well-known symbols. Using [CORE_EXTENSIONS.CUSTOM_IMAGE] here will error. Define the literal key to match your extension name (likely "customImage").
declare module "@tiptap/core" { interface Commands<ReturnType> { - [CORE_EXTENSIONS.CUSTOM_IMAGE]: { + // Ensure this string matches CORE_EXTENSIONS.CUSTOM_IMAGE + customImage: { insertImageComponent: ({ file, pos, event }: InsertImageComponentProps) => ReturnType; }; } - interface Storage { - [CORE_EXTENSIONS.CUSTOM_IMAGE]: CustomImageExtensionStorage; - } + interface Storage { + customImage: CustomImageExtensionStorage; + } }packages/editor/src/core/extensions/custom-image/extension.tsx (2)
45-53: Storage shape mismatch: ‘markdown’ not in CustomImageExtensionStorageaddStorage returns a markdown property that isn’t declared in CustomImageExtensionStorage, which will trigger an excess property error on strict TS settings.
Apply either of the following:
- Option A (preferred): add markdown to the storage type.
- export type CustomImageExtensionStorage = { - fileMap: Map<string, UploadEntity>; - deletedImageSet: Map<string, boolean>; - maxFileSize: number; - }; + export type CustomImageExtensionStorage = { + fileMap: Map<string, UploadEntity>; + deletedImageSet: Map<string, boolean>; + maxFileSize: number; + markdown: { + serialize: () => void; + }; + };
- Option B: if markdown is temporary, remove it here or tuck it under an internal object you type explicitly.
- markdown: { - serialize() {}, - }, + // add back when typed on storage
98-103: Falsy check on pos breaks insert at doc startIf pos === 0, the command falls back to insertContent(). Check for undefined instead.
- if (props.pos) { + if (props.pos !== undefined) { return commands.insertContentAt(props.pos, { type: this.name, attrs: attributes, }); }
🧹 Nitpick comments (6)
packages/editor/src/core/hooks/use-file-upload.ts (1)
69-71: Preserve the underlying upload error.Dropping the error parameter means we now lose the stack/message when an upload fails, which makes production incidents much harder to debug. Please keep the error when logging (or pipe it through to
onInvalidFile) so we retain the context we need.Apply this diff to restore the error details:
- } catch { - console.error("useFileUpload: Error in uploading file"); + } catch (error) { + console.error("useFileUpload: Error in uploading file", error);packages/editor/src/core/extensions/callout/types.ts (1)
30-31: Define proper types instead ofunknownUsing
unknownforCustomCalloutExtensionOptionsandCustomCalloutExtensionStoragereduces type safety. Consider defining proper interfaces based on what options and storage the callout extension actually uses.-export type CustomCalloutExtensionOptions = unknown; -export type CustomCalloutExtensionStorage = unknown; +export interface CustomCalloutExtensionOptions { + // Add specific options if needed, e.g.: + // allowedIcons?: string[]; + // defaultBackground?: string; +} + +export interface CustomCalloutExtensionStorage { + markdown?: { + serialize: (state: any, node: any) => void; + }; + // Add other storage properties as needed +}packages/editor/src/core/extensions/custom-image/extension-config.ts (2)
6-13: Use type-only imports for types to avoid runtime baggageCustomImageExtensionOptions and TCustomImageAttributes are types. Mark them as type-only for clarity and treeshaking.
import { type CustomImageExtensionType, type CustomImageExtensionStorage, ECustomImageAttributeNames, type InsertImageComponentProps, - CustomImageExtensionOptions, - TCustomImageAttributes, + type CustomImageExtensionOptions, + type TCustomImageAttributes, } from "./types";
38-46: Minor: avoid the ‘as Record<…>’ cast by shaping a typed helperOptional: extract a helper that builds the attribute record to keep strong typing without the cast.
const makeAttrDefaults = () => { const acc = {} as Record< ECustomImageAttributeNames, { default: TCustomImageAttributes[ECustomImageAttributeNames] } >; for (const key of Object.values(ECustomImageAttributeNames)) { acc[key] = { default: DEFAULT_CUSTOM_IMAGE_ATTRIBUTES[key] }; } return acc; }; // ... const attributes = { ...this.parent?.(), ...makeAttrDefaults(), };packages/editor/src/core/extensions/custom-image/extension.tsx (2)
62-70: Avoid alert() in editor command error pathsalert() is blocking and hostile to UX. Prefer a non-blocking notification mechanism (app toast) or a callback passed via options.
- onError: (_error, message) => alert(message), + onError: (_error, message) => { + // replace with your app’s notifier + this.editor.emit?.("notify", { type: "error", message }); + },
46-48: Use Set for deleted flagsdeletedImageSet is a Map<string, boolean> but presence suffices. A Set is simpler and cheaper.
- deletedImageSet: new Map<string, boolean>(), + deletedImageSet: new Set<string>(),And update the storage type accordingly:
- deletedImageSet: Map<string, boolean>; + deletedImageSet: Set<string>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/editor/src/core/extensions/callout/extension-config.ts(3 hunks)packages/editor/src/core/extensions/callout/extension.tsx(2 hunks)packages/editor/src/core/extensions/callout/types.ts(2 hunks)packages/editor/src/core/extensions/custom-image/components/node-view.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/extension-config.ts(2 hunks)packages/editor/src/core/extensions/custom-image/extension.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/types.ts(1 hunks)packages/editor/src/core/hooks/use-file-upload.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/editor/src/core/extensions/callout/extension.tsx
- packages/editor/src/core/extensions/custom-image/components/node-view.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
packages/editor/src/core/extensions/custom-image/extension.tsx (2)
packages/editor/src/core/extensions/custom-image/extension-config.ts (1)
CustomImageExtensionConfig(27-63)packages/editor/src/core/extensions/custom-image/types.ts (2)
CustomImageExtensionOptions(43-48)CustomImageExtensionStorage(50-54)
packages/editor/src/core/extensions/custom-image/extension-config.ts (2)
packages/editor/src/core/extensions/custom-image/types.ts (4)
CustomImageExtensionStorage(50-54)CustomImageExtensionType(56-56)CustomImageExtensionOptions(43-48)TCustomImageAttributes(26-33)packages/editor/src/core/extensions/custom-image/utils.ts (1)
DEFAULT_CUSTOM_IMAGE_ATTRIBUTES(6-13)
packages/editor/src/core/extensions/callout/extension-config.ts (2)
packages/editor/src/core/extensions/callout/types.ts (2)
CustomCalloutExtensionType(33-33)TCalloutBlockAttributes(23-28)packages/editor/src/core/extensions/callout/utils.ts (1)
DEFAULT_CALLOUT_BLOCK_ATTRIBUTES(12-20)
packages/editor/src/core/hooks/use-file-upload.ts (1)
packages/editor/src/core/types/editor.ts (2)
NodeViewProps(205-205)TEditorCommands(31-60)
🔇 Additional comments (6)
packages/editor/src/core/hooks/use-file-upload.ts (1)
97-140: Dynamic position lookup looks good.Deriving
posfromgetPosinside the hook keeps the value in sync with the node view lifecycle and the extraundefinedguard avoids accidental drops when the node is gone. 👍packages/editor/src/core/extensions/callout/extension-config.ts (2)
19-19: Good type safety improvement!The explicit typing of
CustomCalloutExtensionConfigasCustomCalloutExtensionTypeprovides better type safety and IDE support for the extension configuration.
27-35: Well-structured attribute type safetyThe typed accumulator with
Record<ECalloutAttributeNames, { default: TCalloutBlockAttributes[ECalloutAttributeNames] }>ensures compile-time type checking for attribute defaults. This is a significant improvement over the previous untyped approach.packages/editor/src/core/extensions/custom-image/types.ts (1)
56-56: Rename to CustomImageExtensionType looks goodType alias aligns with TipTap’s Node<Options, Storage> surface. No runtime impact expected. Please confirm all imports/exports in the feature area were updated to the new name.
packages/editor/src/core/extensions/custom-image/extension-config.ts (1)
27-30: Generic annotation on extension config is correctExtending BaseImageExtension with <CustomImageExtensionOptions, CustomImageExtensionStorage> is the right shape for TipTap’s Node generics.
packages/editor/src/core/extensions/custom-image/extension.tsx (1)
26-26: Good: carrying generics through extend()Passing <CustomImageExtensionOptions, CustomImageExtensionStorage> keeps this extension’s options and storage strongly typed through the chain.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
packages/editor/package.json(4 hunks)
⏰ 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: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/editor/package.json (4)
60-60: Remove aggregated “@tiptap/extensions” in favor of per‑extension packages.You already depend on individual @tiptap/extension-* packages; keeping @tiptap/extensions adds weight unless you import from it directly. Replace any remaining imports and drop the dependency.
- "@tiptap/extensions": "^3.5.3",#!/bin/bash rg -nP "from ['\"]@tiptap/extensions['\"]|require\\(['\"]@tiptap/extensions['\"]\\)"
76-76: No changes needed for tiptap-markdown dependency
tiptap-markdown@^0.9.0 declares a peer dependency on @tiptap/core ^3.0.1, confirming v3 compatibility. Replacing it with @sequa-ai/tiptap-markdown is unnecessary.Likely an incorrect or invalid review comment.
49-66: TipTap dependencies with ^3.5.3 are valid – all checked packages publish a 3.5.3 release, so installs won’t fail. If you’d like to stay current, consider bumping to the latest 3.6.2 dist-tag or centralizing versions via your catalog.Likely an incorrect or invalid review comment.
26-26: Build: prevent duplicate declaration emission
tsc && tsdownmay both emit.d.ts, causing conflicting outputs. Use eithertsc --emitDeclarationOnlyor rely on a single tool for declarations.- "build": "tsc && tsdown", + "build": "tsc --emitDeclarationOnly && tsdown",
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/editor/src/core/extensions/mentions/utils.ts (2)
35-36: Harden floating‑UI positioning: fire‑and‑forget without error handling.
updateFloatingUIFloaterPositionis async but errors are unhandled and the promise isn’t returned. Recommend handling errors in the helper and optionally returning the promise for awaiting if needed.Proposed helper change (packages/editor/src/core/helpers/floating-ui.ts):
export const updateFloatingUIFloaterPosition = ( editor: Editor, element: HTMLElement, options?: { @@ -) => { - const virtualElement = { +) => { + const virtualElement = { getBoundingClientRect: () => posToDOMRect(editor.view, editor.state.selection.from, editor.state.selection.to), }; - computePosition(virtualElement, element, { + return computePosition(virtualElement, element, { placement: options?.placement ?? "bottom-start", strategy: options?.strategy ?? "absolute", middleware: options?.middleware ?? [shift(), flip()], - }).then(({ x, y, strategy }) => { + }) + .then(({ x, y, strategy }) => { Object.assign(element.style, { width: "max-content", position: strategy, left: `${x}px`, top: `${y}px`, ...options?.elementStyle, }); - }); + }) + .catch((error) => { + console.warn("Failed to position floater:", error); + }); };Then (optional) await in callers if you need sequencing:
- updateFloatingUIFloaterPosition(editor, element); + void updateFloatingUIFloaterPosition(editor, element);Also applies to: 41-42
15-16: Reduce@ts-expect-errorsurface by tightening render prop types.Prefer typing
onStart/onUpdateparams via@tiptap/suggestiontypes to avoid blanket suppression.If v3 exposes suitable types, consider:
import type { SuggestionOptions, SuggestionProps } from "@tiptap/suggestion"; // ... onStart: (p: SuggestionProps<any>) => { /* ... */ } onUpdate: (p: SuggestionProps<any>) => { /* ... */ }Please confirm the exact exported names in your installed version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/editor/package.json(4 hunks)packages/editor/src/core/components/menus/block-menu.tsx(5 hunks)packages/editor/src/core/extensions/mentions/utils.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/editor/src/core/components/menus/block-menu.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/editor/src/core/extensions/mentions/utils.ts (2)
packages/editor/src/core/extensions/mentions/mentions-list-dropdown.tsx (2)
MentionsListDropdownProps(13-17)MentionsListDropdown(19-165)packages/editor/src/core/helpers/floating-ui.ts (1)
updateFloatingUIFloaterPosition(4-31)
⏰ 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: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/editor/package.json (1)
51-65: Add the required@tiptap/y-tiptapdependency
@tiptap/extension-collaboration@^3.5.3still declares a peer on@tiptap/y-tiptap@^3.0.0-beta.3. Because this package isn’t installed, any consumer building the editor will resolve the extension’srequire('@tiptap/y-tiptap')to nothing and fail at install/build time. Please add the peer to our dependencies (pin it to a compatible^3.0.0release) so the collaboration tooling actually loads. (app.unpkg.com)Apply this diff:
"@tiptap/extension-text-align": "^3.5.3", "@tiptap/extension-text-style": "^3.5.3", "@tiptap/extensions": "^3.5.3", "@tiptap/html": "catalog:", "@tiptap/pm": "^3.5.3", "@tiptap/react": "^3.5.3", "@tiptap/starter-kit": "^3.5.3", "@tiptap/suggestion": "^3.5.3", + "@tiptap/y-tiptap": "^3.0.0",Likely an incorrect or invalid review comment.
packages/editor/src/core/extensions/mentions/utils.ts (2)
44-57: Prevent default for Arrow/Enter to stop caret movement; normalize returns.Call
preventDefaultin addition tostopPropagationfor navigation keys; return a boolean even when ref is null. This was flagged earlier.const navigationKeys = ["ArrowUp", "ArrowDown", "Enter"]; - if (navigationKeys.includes(event.key)) { - event?.stopPropagation(); - return component?.ref?.onKeyDown({ event }); - } - return component?.ref?.onKeyDown({ event }); + if (navigationKeys.includes(event.key)) { + event.preventDefault(); + event.stopPropagation(); + return component?.ref?.onKeyDown({ event }) ?? true; + } + return component?.ref?.onKeyDown({ event }) ?? false;
58-62: Make element removal null‑safe and clear component ref.Use optional chaining on
element.remove()and null outcomponentafter destroy.- onExit: ({ editor }) => { - editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.MENTION); - component?.element.remove(); - component?.destroy(); - }, + onExit: ({ editor }) => { + editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.MENTION); + component?.element?.remove(); + component?.destroy(); + component = null; + },
| onUpdate: ({ clientRect, editor }) => { | ||
| component?.updateProps(props); | ||
| popup?.[0]?.setProps({ | ||
| getReferenceClientRect: props.clientRect, | ||
| }); | ||
| if (!clientRect) return; | ||
| if (component?.element) { | ||
| updateFloatingUIFloaterPosition(editor, component?.element as HTMLElement); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Update with latest render props, not the outer closure.
Same issue on updates: feed current editor/command/query to the React component.
- onUpdate: ({ clientRect, editor }) => {
- component?.updateProps(props);
+ onUpdate: (onUpdateProps) => {
+ const { clientRect, editor, command, query } = onUpdateProps as any;
+ component?.updateProps({ editor, command, query, searchCallback });
if (!clientRect) return;
- if (component?.element) {
- updateFloatingUIFloaterPosition(editor, component?.element as HTMLElement);
- }
+ if (component?.element) updateFloatingUIFloaterPosition(editor, component.element as HTMLElement);
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onUpdate: ({ clientRect, editor }) => { | |
| component?.updateProps(props); | |
| popup?.[0]?.setProps({ | |
| getReferenceClientRect: props.clientRect, | |
| }); | |
| if (!clientRect) return; | |
| if (component?.element) { | |
| updateFloatingUIFloaterPosition(editor, component?.element as HTMLElement); | |
| } | |
| }, | |
| onUpdate: (onUpdateProps) => { | |
| const { clientRect, editor, command, query } = onUpdateProps as any; | |
| component?.updateProps({ editor, command, query, searchCallback }); | |
| if (!clientRect) return; | |
| if (component?.element) updateFloatingUIFloaterPosition(editor, component.element as HTMLElement); | |
| }, |
🤖 Prompt for AI Agents
In packages/editor/src/core/extensions/mentions/utils.ts around lines 37 to 43,
the onUpdate handler currently calls component?.updateProps(props) which uses
the outer-closure props instead of the latest render-time values; change the
call to pass the current editor/context values (e.g., include the editor
instance and current command/query objects from the onUpdate arguments) so the
React component receives up-to-date props, and ensure
updateFloatingUIFloaterPosition still runs only when clientRect and
component.element exist.
| if (event.key === "Escape") { | ||
| component?.destroy(); | ||
| return true; |
There was a problem hiding this comment.
Escape should perform full cleanup (active state + DOM) to avoid leaks.
Currently only destroy() is called; active dropbar state and DOM node can linger if onExit isn’t invoked.
- if (event.key === "Escape") {
- component?.destroy();
- return true;
- }
+ if (event.key === "Escape") {
+ // ensure symmetric cleanup
+ editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.MENTION);
+ component?.element?.remove();
+ component?.destroy();
+ component = null;
+ return true;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (event.key === "Escape") { | |
| component?.destroy(); | |
| return true; | |
| if (event.key === "Escape") { | |
| // ensure symmetric cleanup | |
| editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.MENTION); | |
| component?.element?.remove(); | |
| component?.destroy(); | |
| component = null; | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In packages/editor/src/core/extensions/mentions/utils.ts around lines 45-47, the
Escape key handler currently only calls component?.destroy(), which can leave
the dropbar's active state and DOM node if onExit isn't invoked; update the
handler to first invoke the component's onExit (if present) to clear active
state, then remove the component's DOM node (or call a DOM-removal method) and
finally call destroy to fully clean up and avoid leaks.
* chore: upgrade to tiptap v3 * chore: update starter kit * chore: tippy to floating-ui migration for mentions * chore: update remaining floating menus * chore: update setEditorValue function * fix: potential bugs * chore: extract out common floating ui positioning logic * fix: storage api * fix: bubble menu * fix: type errors * fix: type errors * chore: upgrade tiptap-markdown package * fix: mentions close callback * chore: update bubbling sequence * chore: update package.json * chore: update tiptap catalogs * fix: add error handling * fix: file plugin types * chore: update live package.json * fix: broken lock file --------- Co-authored-by: Palanikannan M <[email protected]> Co-authored-by: sriramveeraghanta <[email protected]>
Description
This PR upgrades Tiptap to
v3Type of Change
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores