Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an extended editor file-handlers surface (types + CE hook) and merges it into the core editor config; refactors block menu from Tippy to Floating UI; extends drag-handle selectors; exposes an integrations-loading flag to gate editor rendering. One service file change is formatting-only. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Editor UI
participant CoreHook as useEditorConfig
participant ExtHook as useExtendedEditorConfig
participant Handlers as Editor file handlers
UI->>CoreHook: request editor file handlers
CoreHook->>ExtHook: getExtendedEditorFileHandlers({ workspaceSlug, projectId })
ExtHook-->>CoreHook: extendedHandlers ({...})
CoreHook-->>UI: { ...coreHandlers, ...extendedHandlers }
note right of Handlers: TFileHandler is intersected with TExtendedFileHandler
sequenceDiagram
autonumber
participant User as User
participant Drag as Drag-handle (virtual ref)
participant Menu as BlockMenu (Floating UI)
User->>Drag: click drag-handle
Drag->>Menu: set virtual reference
Menu->>Menu: open (FloatingPortal render)
User->>Menu: select action / outside click / Escape / scroll
alt action chosen
Menu->>Menu: perform action (delete/duplicate)
Menu->>Menu: close
else dismiss
Menu->>Menu: close
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the editor configuration by introducing extension points for file handling, updating the block menu component to use Floating UI instead of Tippy.js, and adding support for draw.io components in the editor.
- Adds
TExtendedFileHandlertype extension point for custom file handling configurations - Replaces Tippy.js with Floating UI in the block menu component for better positioning and animations
- Introduces support for draw.io components and adds reupload methods for workspace and project assets
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/editor/src/core/types/config.ts | Adds TExtendedFileHandler extension to TFileHandler type |
| packages/editor/src/core/plugins/drag-handle.ts | Includes editor-drawio-component in drag handle selectors |
| packages/editor/src/core/components/menus/block-menu.tsx | Refactors from Tippy.js to Floating UI with improved animations |
| packages/editor/src/core/components/editors/rich-text/editor.tsx | Adds BlockMenu component to RichTextEditor |
| packages/editor/src/ce/types/config.ts | Defines TExtendedFileHandler as null for CE version |
| packages/editor/src/ce/types/index.ts | Exports config types |
| apps/web/core/services/file.service.ts | Adds reupload methods for workspace and project assets |
| apps/web/core/hooks/editor/use-editor-config.ts | Integrates extended editor config |
| apps/web/ce/hooks/editor/use-editor-config.ts | Provides CE implementation of extended editor config |
Comments suppressed due to low confidence (1)
packages/editor/src/core/components/menus/block-menu.tsx:1
- Remove the trailing space character after the closing JSX tag. This adds unnecessary whitespace to the rendered output.
import {
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
apps/web/ce/hooks/editor/use-editor-config.ts (1)
6-7: Make the extension type-safe (avoidobject).Returning
objecthides unsafe overrides; use a typed surface so TS validates handler shapes across CE/EE.Apply this diff:
-export type TExtendedEditorConfig = object; +export type TExtendedEditorConfig = Partial<TFileHandler>;Add this type-only import at the top of the file:
import type { TFileHandler } from "@plane/editor";Please confirm this type-only import doesn’t introduce layering or circular-dependency issues in your build.
apps/web/core/hooks/editor/use-editor-config.ts (1)
90-93: Type-narrow the extended spread and confirm override intent.
- Cast to Partial so TS checks shapes of any overrides.
- Because the spread is last, it will override base handlers; confirm this is intentional.
Apply this diff:
- ...extendedEditorConfig({ - projectId, - workspaceSlug, - }), + ...(extendedEditorConfig({ + projectId, + workspaceSlug, + }) as Partial<TFileHandler>),If overriding core handlers is not desired, move this spread before the core properties instead.
packages/editor/src/core/plugins/drag-handle.ts (1)
108-116: Prevent matching nested Draw.io internals (parity with embed).If Draw.io contains nested DOM, clicks inside it may pick inner nodes before the block root. Mirror the existing embed skip so we only match the Draw.io root.
Apply this change near the existing embed skip:
// Skip elements inside .editor-embed-component if (elem.closest(".editor-embed-component") && !elem.matches(".editor-embed-component")) { continue; } + + // Skip elements inside .editor-drawio-component; allow only the root + if (elem.closest(".editor-drawio-component") && !elem.matches(".editor-drawio-component")) { + continue; + }packages/editor/src/core/components/editors/rich-text/editor.tsx (1)
43-48: Avoid always mounting BlockMenu; gate by dragDropEnabled.BlockMenu attaches global listeners on mount. If drag-drop is disabled, skip mounting to reduce overhead.
Apply this diff:
- <> - {editor && bubbleMenuEnabled && <EditorBubbleMenu editor={editor} />}{" "} - <BlockMenu editor={editor} flaggedExtensions={flaggedExtensions} disabledExtensions={disabledExtensions} /> - </> + <> + {editor && bubbleMenuEnabled && <EditorBubbleMenu editor={editor} />} + {editor && dragDropEnabled && ( + <BlockMenu + editor={editor} + flaggedExtensions={flaggedExtensions} + disabledExtensions={disabledExtensions} + /> + )} + </>packages/editor/src/core/types/config.ts (1)
3-4: Guard the intersection to be safe if CE leavesTExtendedFileHandlerasnull.Even after fixing CE to
{}, harden the core type sovalidationdoesn’t collapse toneverif an edition accidentally setsTExtendedFileHandlerto a non-object.Apply this diff:
-import { TExtendedFileHandler } from "@/plane-editor/types/config"; +import { TExtendedFileHandler } from "@/plane-editor/types/config"; @@ - } & TExtendedFileHandler; + } & (TExtendedFileHandler extends object ? TExtendedFileHandler : {});Also applies to: 20-21
packages/editor/src/core/components/menus/block-menu.tsx (4)
46-75: Allow toggle on drag‑handle click (open/close).Currently clicking the handle when open does not close the menu. Toggle to improve UX.
Apply this diff:
- // Show the menu - setIsOpen(true); + // Toggle the menu + setIsOpen((prev) => !prev);
86-97: Mark scroll listener as passive for perf.Use passive capture to avoid main-thread blocking during scroll.
Apply this diff:
- document.addEventListener("scroll", handleScroll, true); // Using capture phase + const scrollOpts: AddEventListenerOptions = { capture: true, passive: true }; + document.addEventListener("scroll", handleScroll, scrollOpts); // Using capture phase @@ - document.removeEventListener("scroll", handleScroll, true); + document.removeEventListener("scroll", handleScroll, scrollOpts);
190-209: A11y: add roles; remove inert attribute.
- Add role semantics for menu and items.
data-prevent-outside-clickis not used by Floating UI; remove to avoid confusion.Apply this diff:
- <div + <div + role="menu" ref={(node) => { refs.setFloating(node); menuRef.current = node; }} style={{ ...floatingStyles, zIndex: 99, animationFillMode: "forwards", transitionTimingFunction: "cubic-bezier(0.16, 1, 0.3, 1)", // Expo ease out }} 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" )} - data-prevent-outside-click {...getFloatingProps()} > {MENU_ITEMS.map((item) => { if (item.isDisabled) { return null; } return ( <button key={item.key} type="button" + role="menuitem" 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} >
196-206: Minor: avoid conflicting z-index sources.Inline style zIndex: 99 and class "z-20" conflict. Prefer one source of truth (class or style). Consider removing one.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/ce/hooks/editor/use-editor-config.ts(1 hunks)apps/web/core/hooks/editor/use-editor-config.ts(2 hunks)apps/web/core/services/file.service.ts(1 hunks)packages/editor/src/ce/types/config.ts(1 hunks)packages/editor/src/ce/types/index.ts(1 hunks)packages/editor/src/core/components/editors/rich-text/editor.tsx(2 hunks)packages/editor/src/core/components/menus/block-menu.tsx(5 hunks)packages/editor/src/core/plugins/drag-handle.ts(1 hunks)packages/editor/src/core/types/config.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/editor/src/core/components/editors/rich-text/editor.tsx (1)
packages/editor/src/core/components/menus/block-menu.tsx (1)
BlockMenu(25-230)
packages/editor/src/core/types/config.ts (1)
packages/editor/src/ce/types/config.ts (1)
TExtendedFileHandler(1-1)
apps/web/core/hooks/editor/use-editor-config.ts (1)
apps/web/ce/hooks/editor/use-editor-config.ts (1)
extendedEditorConfig(7-7)
⏰ 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: Cursor Bugbot
🔇 Additional comments (3)
apps/web/core/hooks/editor/use-editor-config.ts (1)
11-11: Verify CE/EE path alias configuration
apps/web/tsconfig.json and apps/space/tsconfig.json map@/plane-web/*→ce/*. Confirm EE builds override this toee/*and that your bundler (Next/Webpack/Vite) respects the same alias mapping.packages/editor/src/core/plugins/drag-handle.ts (1)
12-26: Selector addition looks good; confirm class name presence.Adding ".editor-drawio-component" to the drag-targets is fine. Please confirm this class is applied to the Draw.io block root so the handle targets the correct container.
packages/editor/src/ce/types/index.ts (1)
3-3: Barrel export addition is fine.Re-exporting config from the types index is OK. Watch for circulars if any downstream also re-exports from this index.
| @@ -0,0 +1 @@ | |||
| export type TExtendedFileHandler = object; | |||
There was a problem hiding this comment.
the type of the variable is object because we are destructuring it to use.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/ce/hooks/editor/use-extended-editor-config.ts (1)
15-18: Expose the args parameter for clarityThe typed signature expects
(args: TExtendedEditorFileHandlersArgs), but the implementation currently relies on TypeScript’s parameter bivariance by omitting the argument altogether. Explicitly accepting_argskeeps the intent obvious and prevents subtle regressions if the annotation ever moves or strict function types are enabled.- const getExtendedEditorFileHandlers: TExtendedEditorConfig["getExtendedEditorFileHandlers"] = useCallback( - () => ({}), - [] - ); + const getExtendedEditorFileHandlers: TExtendedEditorConfig["getExtendedEditorFileHandlers"] = useCallback( + (_args) => ({}), + [] + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/ce/hooks/editor/use-extended-editor-config.ts(1 hunks)apps/web/ce/hooks/use-editor-flagging.ts(2 hunks)apps/web/core/components/pages/editor/editor-body.tsx(2 hunks)apps/web/core/hooks/editor/use-editor-config.ts(3 hunks)packages/editor/src/ce/types/config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/editor/src/ce/types/config.ts
- apps/web/core/hooks/editor/use-editor-config.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/ce/hooks/editor/use-extended-editor-config.ts (1)
packages/editor/src/ce/types/config.ts (1)
TExtendedFileHandler(1-1)
apps/web/core/components/pages/editor/editor-body.tsx (1)
apps/web/ce/hooks/use-editor-flagging.ts (1)
useEditorFlagging(29-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/web/ce/hooks/use-editor-flagging.ts (1)
18-43: CE fallback stays consistent with new contractThe added
isLoadingIntegrations: falsekeeps the CE hook aligned with the expanded return type without changing runtime behavior. Looks good.apps/web/core/components/pages/editor/editor-body.tsx (1)
100-104: Loader guard cleanly incorporates integration stateWiring
isLoadingIntegrationsinto the existing loader gate is tidy and keeps the UI from flashing incomplete content. Nicely done.Also applies to: 182-184
aaryan610
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
iam-vipin
changed the title
Description
Update the editor config and file config
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Refactor
Note
Introduces extensible editor file handlers and updates the drag handle to recognize draw.io components.
TExtendedFileHandlerand extendTFileHandlerwith it inpackages/editor/src/core/types/config.ts.apps/web/ce/hooks/editor/use-extended-editor-config.tsexposinggetExtendedEditorFileHandlers.apps/web/core/hooks/editor/use-editor-config.tsvia spread ofgetExtendedEditorFileHandlers({ projectId, workspaceSlug })..editor-drawio-componentin drag handle selectable targets inpackages/editor/src/core/plugins/drag-handle.ts.Written by Cursor Bugbot for commit 356c3e8. This will update automatically on new commits. Configure here.