[WIKI-511] [WIKI-572] fix: extended page root and editor body#7661
[WIKI-511] [WIKI-572] fix: extended page root and editor body#7661sriramveeraghanta merged 10 commits intopreviewfrom
Conversation
WalkthroughIntroduces storeType- and project-aware page/editor wiring, navigation-pane extension API and CE facades, extended-editor props and side-effect scaffolding, content-aware comment emptiness checks, version-restore service and redirection-on-deleted-pages, plus multiple type re-exports and wrapper prop tightening. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as PageDetailsPage
participant Root as PageRoot
participant Versions as PageVersionsOverlay
participant Service as ProjectPageVersionService
participant API as REST API
User->>Versions: Click "Restore version"
Versions-->>Root: restoreVersion(pageId, versionId)
Root->>Service: restoreVersion(workspaceSlug, projectId, pageId, versionId)
Service->>API: POST /workspaces/{ws}/projects/{p}/pages/{pg}/versions/{v}/restore/
API-->>Service: 200 OK
Service-->>Root: Promise resolved
Root-->>Versions: Notify / trigger refresh
sequenceDiagram
autonumber
actor User
participant Pane as PageNavigationPaneRoot
participant Hook as usePagesPaneExtensions
participant Ext as ActiveExtension(component)
User->>Pane: Open navigation pane
Pane->>Hook: Derive isOpen, handlers, extensions
Pane->>Pane: Find ActiveExtension by triggerParam
alt Extension active
Pane->>Ext: Render component { page, extensionData, storeType }
else No extension
Pane->>Pane: Render default tabs/panels
end
sequenceDiagram
autonumber
participant Body as PageEditorBody
participant Collab as CollaborativeDocumentEditor
participant SideFx as DocumentEditorSideEffects
participant Renderer as PageRenderer
Body->>Collab: Render with extendedEditorProps
Collab->>SideFx: Render(editor, id, extendedEditorProps)
SideFx-->>Collab: (placeholder/no-op)
Collab->>Renderer: Render document UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically 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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/components/pages/version/main-content.tsx (1)
45-47: Avoid SWR cache collisions: include pageId (and storeType) in the key.Current key omits pageId and may return stale data when switching pages with the same versionId.
Apply:
- } = useSWR( - pageId && activeVersion ? `PAGE_VERSION_${activeVersion}` : null, - pageId && activeVersion ? () => fetchVersionDetails(pageId, activeVersion) : null - ); + } = useSWR( + pageId && activeVersion ? `PAGE_VERSION_${storeType}_${pageId}_${activeVersion}` : null, + pageId && activeVersion ? () => fetchVersionDetails(pageId, activeVersion) : null + );
🧹 Nitpick comments (17)
apps/web/ce/components/pages/modals/modals.tsx (1)
11-18: Avoid unused-props warnings in stub by destructuring to underscoresSince this stub returns
null, props are unused. Destructure to underscore-prefixed names to satisfy common eslint configs.-export const PageModals: React.FC<TPageModalsProps> = observer((props) => null); +export const PageModals: React.FC<TPageModalsProps> = observer( + ({ page: _page, storeType: _storeType, editorRef: _editorRef }) => null +);The CE scaffold itself is fine to land as a no-op.
packages/editor/src/ce/types/editor-extended.ts (1)
1-9: Tighten placeholders and add intent comments for DX
Unknowns are OK as scaffolding, but consider stricter defaults to reduce accidental misuse and clarify intent.-export type IEditorExtensionOptions = unknown; +// CE can augment this via module augmentation. Default: no options. +export type IEditorExtensionOptions = Record<string, never>; -export type IEditorPropsExtended = unknown; +// Extra props passed into the editor by CE. Default: none. +export type IEditorPropsExtended = Record<string, never>; -export type TExtendedCommandExtraProps = unknown; +// Extra payload for CE commands. Default: none. +export type TExtendedCommandExtraProps = never; -export type TExtendedEditorRefApi = unknown; +// Extra editor ref surface for CE. Default: empty object. +export type TExtendedEditorRefApi = Record<string, never>;If you prefer maximum flexibility, keep
unknownbut add comments documenting the augmentation pattern.apps/web/ce/hooks/pages/use-extended-editor-extensions.ts (3)
4-4: Type-only import to avoid pulling runtime values
EPageStoreTypeis used only in types here. Use a type-only import to keep the module tree lean.-import { EPageStoreType } from "../store"; +import type { EPageStoreType } from "../store";
6-14: Make optional params truly optional and prefer ReadonlyMap
Aligns with optional usage and avoids accidental mutation of handler registries.export type TExtendedEditorExtensionsHookParams = { workspaceSlug: string; page: TPageInstance; storeType: EPageStoreType; fetchEntity: (payload: TSearchEntityRequestPayload) => Promise<TSearchResponse>; - getRedirectionLink: (pageId?: string) => string; - extensionHandlers?: Map<string, unknown>; + getRedirectionLink?: (pageId?: string) => string; + extensionHandlers?: ReadonlyMap<string, unknown>; projectId?: string; };
18-18: Annotate return type and preferundefinedovernull
Better fits optional props (extendedEditorProps?) and appeases lint rules for unused params.-export const useExtendedEditorExtensions = (params: TExtendedEditorExtensionsHookParams) => null; +export const useExtendedEditorExtensions = (_params: TExtendedEditorExtensionsHookParams): TExtendedEditorExtensionsConfig | undefined => undefined;If any consumer currently checks for
null, confirm and adjust accordingly.apps/web/core/components/pages/navigation-pane/index.ts (1)
5-5: Replace with a type-only export in apps/web/core/components/pages/navigation-pane/index.ts line 5
Since./typesonly exports types, change-export * from "./types"; +export type * from "./types";TS 5.8.3 supports this syntax and it will be erased at compile time.
packages/editor/src/ce/types/index.ts (1)
2-2: Use type-only exports and imports for CE types-export * from "./editor-extended"; +export type * from "./editor-extended";Convert all consumer imports of
@plane/editorto type-only form (e.g.import type { EditorRefApi } from "@plane/editor";) to avoid pulling in runtime code.packages/editor/src/core/types/index.ts (1)
13-13: Prefer type-only re-export for types barrel.If "@/plane-editor/types" contains only types, use a type-only re-export to avoid pulling any runtime values.
-export * from "@/plane-editor/types"; +export type * from "@/plane-editor/types";apps/web/core/components/pages/version/editor.tsx (1)
12-14: storeType is required but unused—make it optional or wire it through.Avoid forcing callers to pass an unused prop; either plumb it to the editor where needed or make it optional for now.
export type TVersionEditorProps = { activeVersion: string | null; versionDetails: TPageVersion | undefined; - storeType: EPageStoreType; + storeType?: EPageStoreType; };Also applies to: 18-19
apps/web/ce/hooks/use-editor-flagging.ts (1)
3-3: Unused parameters—prefix with underscore to satisfy linters.storeType (and workspaceSlug) aren’t used. Prefix to signal intentional non-use.
-export const useEditorFlagging = ( - workspaceSlug: string, - storeType?: EPageStoreType -): TEditorFlaggingHookReturnType => ({ +export const useEditorFlagging = ( + _workspaceSlug: string, + _storeType?: EPageStoreType +): TEditorFlaggingHookReturnType => ({Also applies to: 23-26
packages/editor/src/core/types/editor.ts (1)
9-9: Prefer importing from the CE types index to avoid deep paths.Reduces churn if files move.
-import type { IEditorPropsExtended, TExtendedEditorCommands } from "@/plane-editor/types/editor-extended"; +import type { IEditorPropsExtended, TExtendedEditorCommands } from "@/plane-editor/types";apps/web/ce/hooks/pages/use-pages-pane-extensions.ts (2)
14-19: Unused hook params (likely to trip linting).
paramsis declared but never used. Either prefix with_or destructure only what you need.-export const usePagesPaneExtensions = (params: TPageExtensionHookParams) => { +export const usePagesPaneExtensions = (_params: TPageExtensionHookParams) => {
33-39: Optional: clear version param when opening the pane.If a version is selected, opening the outline should likely drop
PAGE_NAVIGATION_PANE_VERSION_QUERY_PARAMto avoid conflicting UI state.const handleOpenNavigationPane = useCallback(() => { - const updatedRoute = updateQueryParams({ - paramsToAdd: { [PAGE_NAVIGATION_PANE_TABS_QUERY_PARAM]: "outline" }, - }); + const updatedRoute = updateQueryParams({ + paramsToAdd: { [PAGE_NAVIGATION_PANE_TABS_QUERY_PARAM]: "outline" }, + // paramsToRemove: [PAGE_NAVIGATION_PANE_VERSION_QUERY_PARAM], // uncomment if imported + }); router.push(updatedRoute); }, [router, updateQueryParams]);apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx (2)
150-155: Use replace to avoid back-navigating into a deleted page.
router.replaceprevents the history stack from taking users back to a dead route.- useEffect(() => { - if (page?.deleted_at && page?.id) { - router.push(pageRootHandlers.getRedirectionLink()); - } - }, [page?.deleted_at, page?.id, router, pageRootHandlers]); + useEffect(() => { + if (page?.deleted_at && page?.id) { + router.replace(pageRootHandlers.getRedirectionLink()); + } + }, [page?.deleted_at, page?.id, router, pageRootHandlers]);
75-95: Handlers look good; minor nit: avoid early returns that silently no-op.For
fetchAllVersions/fetchVersionDetails, consider throwing or returning a typedundefinedguard result with logging when required params are missing. Silent returns make failures harder to trace.Would you like me to wire a minimal toast/logger for these early returns?
apps/web/core/components/pages/navigation-pane/types/extensions.ts (1)
5-21: Type surface looks solid.Interfaces are minimal and align with the pane root’s usage. Consider documenting that
widthis pixels.apps/web/core/components/pages/editor/page-root.tsx (1)
118-124: Close-pane cleans both tab and version params — good.Keeps URL state tidy. Consider also clearing any extension-specific
paneTab(handled implicitly).
📜 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 (25)
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx(7 hunks)apps/web/ce/components/pages/modals/index.ts(1 hunks)apps/web/ce/components/pages/modals/modals.tsx(1 hunks)apps/web/ce/hooks/pages/index.ts(1 hunks)apps/web/ce/hooks/pages/use-extended-editor-extensions.ts(1 hunks)apps/web/ce/hooks/pages/use-pages-pane-extensions.ts(1 hunks)apps/web/ce/hooks/use-editor-flagging.ts(2 hunks)apps/web/ce/types/pages/pane-extensions.ts(1 hunks)apps/web/core/components/pages/editor/editor-body.tsx(7 hunks)apps/web/core/components/pages/editor/header/root.tsx(1 hunks)apps/web/core/components/pages/editor/page-root.tsx(5 hunks)apps/web/core/components/pages/navigation-pane/index.ts(1 hunks)apps/web/core/components/pages/navigation-pane/root.tsx(5 hunks)apps/web/core/components/pages/navigation-pane/types/extensions.ts(1 hunks)apps/web/core/components/pages/navigation-pane/types/index.ts(1 hunks)apps/web/core/components/pages/version/editor.tsx(1 hunks)apps/web/core/components/pages/version/main-content.tsx(3 hunks)apps/web/core/components/pages/version/root.tsx(3 hunks)apps/web/core/services/page/project-page-version.service.ts(1 hunks)apps/web/core/store/pages/base-page.ts(4 hunks)packages/editor/src/ce/types/editor-extended.ts(1 hunks)packages/editor/src/ce/types/index.ts(1 hunks)packages/editor/src/core/types/editor.ts(3 hunks)packages/editor/src/core/types/index.ts(1 hunks)packages/types/src/page/core.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-14T13:16:23.323Z
Learnt from: vamsikrishnamathala
PR: makeplane/plane#7061
File: web/core/components/workspace-notifications/root.tsx:18-18
Timestamp: 2025-05-14T13:16:23.323Z
Learning: In the Plane project codebase, the path alias `@/plane-web` resolves to the `ce` directory, making imports like `@/plane-web/hooks/use-notification-preview` correctly resolve to files in `web/ce/hooks/`.
Applied to files:
apps/web/core/components/pages/version/root.tsx
🧬 Code graph analysis (8)
apps/web/ce/hooks/pages/use-pages-pane-extensions.ts (6)
apps/web/core/store/pages/base-page.ts (1)
TPageInstance(69-72)packages/editor/src/core/types/editor.ts (1)
EditorRefApi(95-133)apps/web/core/hooks/use-app-router.tsx (1)
useAppRouter(4-4)apps/web/core/components/pages/navigation-pane/index.ts (2)
PAGE_NAVIGATION_PANE_TABS_QUERY_PARAM(9-9)PAGE_NAVIGATION_PANE_TAB_KEYS(12-12)apps/web/ce/types/pages/pane-extensions.ts (1)
INavigationPaneExtension(7-15)apps/web/core/components/pages/navigation-pane/types/extensions.ts (1)
INavigationPaneExtension(15-21)
apps/web/core/components/pages/version/editor.tsx (1)
packages/types/src/page/core.ts (1)
TPageVersion(50-64)
apps/web/core/components/pages/navigation-pane/types/extensions.ts (2)
apps/web/core/store/pages/base-page.ts (1)
TPageInstance(69-72)apps/web/ce/types/pages/pane-extensions.ts (1)
INavigationPaneExtension(7-15)
packages/editor/src/core/types/editor.ts (1)
packages/editor/src/ce/types/editor-extended.ts (2)
TExtendedEditorCommands(5-5)IEditorPropsExtended(3-3)
apps/web/ce/types/pages/pane-extensions.ts (1)
apps/web/core/components/pages/navigation-pane/types/extensions.ts (1)
INavigationPaneExtension(15-21)
apps/web/core/components/pages/navigation-pane/root.tsx (2)
apps/web/core/components/pages/navigation-pane/types/extensions.ts (1)
INavigationPaneExtension(15-21)apps/web/core/components/pages/navigation-pane/index.ts (2)
PAGE_NAVIGATION_PANE_VERSION_QUERY_PARAM(10-10)PAGE_NAVIGATION_PANE_WIDTH(7-7)
apps/web/core/components/pages/editor/editor-body.tsx (2)
apps/web/ce/hooks/use-editor-flagging.ts (1)
useEditorFlagging(23-39)apps/web/core/components/pages/editor/header/root.tsx (1)
PageEditorHeaderRoot(17-71)
apps/web/core/components/pages/editor/page-root.tsx (2)
apps/web/ce/hooks/pages/use-pages-pane-extensions.ts (1)
usePagesPaneExtensions(19-53)apps/web/core/components/pages/navigation-pane/root.tsx (1)
PageNavigationPaneRoot(39-117)
⏰ 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 (13)
packages/types/src/page/core.ts (1)
24-24: Aligndeleted_atwith API and ensure correct handling
In packages/types/src/page/core.ts (line 24), change
deleted_at: Date | undefined;
to
deleted_at: string | null | undefined;Verify in apps/web/core/store/pages/base-page.ts (line 96) and in BasePage.asJSON that
deleted_atis treated as a string (and/or converted from string) to avoid leakingDateobjects.apps/web/ce/components/pages/modals/index.ts (1)
2-2: Barrel exports are safe: no name collisions
Both modules export distinct symbols (TMovePageModalProps,MovePageModalvsTPageModalsProps,PageModals).apps/web/ce/hooks/pages/index.ts (1)
1-2: Barrel index looks good.Single entry-point for CE pages hooks improves DX and keeps imports tidy.
apps/web/core/components/pages/version/main-content.tsx (1)
9-11: Propagating storeType into VersionEditor looks correct.Props surface and usage are consistent.
Also applies to: 22-22, 26-35, 121-121
apps/web/core/components/pages/navigation-pane/types/index.ts (1)
1-6: Type re-exports are clean and scoped.Good centralization of extension types.
packages/editor/src/core/types/editor.ts (1)
163-164: Extended editor props wiring looks good.Optional prop maintains backward compatibility.
apps/web/core/components/pages/navigation-pane/root.tsx (1)
34-37: Extension plumbing and dynamic width look good.Defaulting extensions to [] avoids null checks; passing storeType to the extension component aligns with the store-aware design.
Please confirm that INavigationPaneExtensionComponent’s props include storeType: EPageStoreType so this call site is type-safe. You can verify by opening apps/web/core/components/pages/navigation-pane/types/extensions.ts and checking the component props.
Also applies to: 40-41, 105-114
apps/web/core/components/pages/editor/editor-body.tsx (4)
109-110: Flagging now respects storeType.Passing
storeTypeintouseEditorFlaggingmatches the new store-aware editor stack.
218-219: Header now needs projectId — OK.Prop is correctly threaded; no further action.
256-257: Extended editor props wired.
extendedEditorPropsforwarded to the collab editor — LGTM.
47-50: New handler shape acknowledged.Adding
getRedirectionLinktoTEditorBodyHandlersis fine; confirm all providers pass it (PageRoot does).If there are other
PageEditorBodycall sites, run:apps/web/core/components/pages/editor/page-root.tsx (2)
149-156: Prop threading is correct and stable.
isNavigationPaneOpen,storeType,projectId, andextendedEditorPropsare correctly passed to children.
158-170: Modal wiring looks sane.
PageModalsreceivespage,storeType, and the currenteditorRef. No issues spotted.
...l)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx
Show resolved
Hide resolved
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 (1)
packages/editor/src/core/components/editors/document/collaborative-editor.tsx (1)
53-66: Bug: unsafe dependencyembedHandler.issueinuseMemo.Accessing
embedHandler.issuewill throw whenembedHandleris undefined. Use optional chaining.- }, [externalExtensions, embedHandler.issue]); + }, [externalExtensions, embedHandler?.issue]);
🧹 Nitpick comments (2)
packages/editor/src/ce/components/editor-side-effects.ts (1)
1-3: Add React return type import for stricter typing.Prepares for future hook usage and makes the component signature explicit.
import { type Editor } from "@tiptap/core"; import type { IEditorPropsExtended } from "@/types"; +import type { ReactElement } from "react";packages/editor/src/core/components/editors/document/collaborative-editor.tsx (1)
103-119: MountEditorSideEffectsonly when needed.Avoid mounting a no-op component on every render if no extended behavior is provided.
- <> - <EditorSideEffects editor={editor} id={id} extendedEditorProps={extendedEditorProps} /> + <> + {extendedEditorProps && ( + <EditorSideEffects editor={editor} id={id} extendedEditorProps={extendedEditorProps} /> + )} <PageRenderer
📜 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/ce/components/editor-side-effects.ts(1 hunks)packages/editor/src/core/components/editors/document/collaborative-editor.tsx(3 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 (2)
packages/editor/src/core/components/editors/document/collaborative-editor.tsx (2)
16-16: Path alias for@/plane-editoris correctly configured
@/plane-editor/components/editor-side-effectsresolves topackages/editor/src/ce/components/editor-side-effects.ts, so the import will work as expected.
32-32: extendedEditorProps is already included on ICollaborativeDocumentEditorProps
TheextendedEditorPropsfield is declared onIEditorProps(packages/editor/src/core/types/editor.ts, line 163) and theOmitinICollaborativeDocumentEditorPropsonly removesinitialValue,onEnterKeyPress, andvalue, soextendedEditorPropsremains part of the props interface. TS will not error here.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/types/src/editor/editor-content.ts (1)
5-16: Tighten typing: prefer unknown over any for attrs and index signatures.Reduces unsoundness without adding dependencies.
export type JSONContent = { type?: string; - attrs?: Record<string, any>; + attrs?: Record<string, unknown>; content?: JSONContent[]; marks?: { type: string; - attrs?: Record<string, any>; - [key: string]: any; + attrs?: Record<string, unknown>; + [key: string]: unknown; }[]; text?: string; - [key: string]: any; + [key: string]: unknown; };packages/types/src/index.ts (1)
31-31: Type-only re-export to avoid accidental value re-exports.Harmless now (types only), but safer with TS emit/isolatedModules.
-export * from "./editor"; +export type * from "./editor";
📜 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 (7)
apps/space/core/components/editor/lite-text-editor.tsx(1 hunks)apps/space/helpers/string.helper.ts(0 hunks)packages/types/src/editor/editor-content.ts(1 hunks)packages/types/src/editor/index.ts(1 hunks)packages/types/src/index.ts(1 hunks)packages/types/src/issues/activity/issue_comment.ts(2 hunks)packages/utils/src/string.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/space/helpers/string.helper.ts
✅ Files skipped from review due to trivial changes (1)
- apps/space/core/components/editor/lite-text-editor.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/types/src/issues/activity/issue_comment.ts (1)
packages/types/src/editor/editor-content.ts (1)
JSONContent(5-16)
packages/utils/src/string.ts (2)
packages/types/src/editor/editor-content.ts (2)
JSONContent(5-16)Content(20-20)apps/space/helpers/string.helper.ts (1)
isEmptyHtmlString(53-58)
⏰ 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/types/src/editor/index.ts (1)
1-1: LGTM — focused type-only surface.packages/utils/src/string.ts (1)
170-193: Align JSON and HTML emptiness semantics: in isJSONContentEmpty, includehardBreakin the empty-case check and excludeembed-componentfrom the “empty” list so embed-only nodes are treated as non-empty. Verify thatJSONContentactually usesembed-componentnodes and that they should indeed count as content.
packages/editor/src/core/components/editors/document/collaborative-editor.tsx
Outdated
Show resolved
Hide resolved
...l)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/components/pages/editor/page-root.tsx (1)
118-123: Close action should also clear custom extension query param.Removing only the built-in params leaves
paneTabbehind, keeping extension state sticky.Apply this diff:
-const updatedRoute = updateQueryParams({ - paramsToRemove: [PAGE_NAVIGATION_PANE_TABS_QUERY_PARAM, PAGE_NAVIGATION_PANE_VERSION_QUERY_PARAM], -}); +const updatedRoute = updateQueryParams({ + paramsToRemove: [PAGE_NAVIGATION_PANE_TABS_QUERY_PARAM, PAGE_NAVIGATION_PANE_VERSION_QUERY_PARAM, "paneTab"], +});
♻️ Duplicate comments (1)
apps/web/core/components/pages/editor/page-root.tsx (1)
87-93: Pane open-state still ignores custom extensions.
isNavigationPaneOpenfromusePagesPaneExtensionsdoesn’t account for extensiontriggerParams; extensions render off-canvas. Fix in the hook by OR-ing with “active extension exists”.Run:
#!/bin/bash sed -n '1,200p' apps/web/ce/hooks/pages/use-pages-pane-extensions.ts rg -nP 'isNavigationPaneOpen' apps/web/ce/hooks/pages/use-pages-pane-extensions.ts -C3 rg -nP 'triggerParam|navigationPaneExtensions' apps/web/ce/hooks/pages/use-pages-pane-extensions.ts -C3
🧹 Nitpick comments (3)
apps/web/ce/hooks/pages/use-extended-editor-extensions.ts (2)
6-14: Unify naming: use “EditorProps” across hook/types (and file).For consistency with the exported hook, prefer
TExtendedEditorPropsParamsandTExtendedEditorProps. Consider renaming the file touse-extended-editor-props.ts.Apply this diff:
-export type TExtendedEditorExtensionsHookParams = { +export type TExtendedEditorPropsParams = { workspaceSlug: string; page: TPageInstance; storeType: EPageStoreType; fetchEntity: (payload: TSearchEntityRequestPayload) => Promise<TSearchResponse>; getRedirectionLink: (pageId?: string) => string; extensionHandlers?: Map<string, unknown>; projectId?: string; }; -export type TExtendedEditorExtensionsConfig = IEditorPropsExtended; +export type TExtendedEditorProps = IEditorPropsExtended;And update usage:
-export const useExtendedEditorProps = (params: TExtendedEditorExtensionsHookParams): TExtendedEditorExtensionsConfig => ({} as unknown as TExtendedEditorExtensionsConfig); +export const useExtendedEditorProps = (_params: TExtendedEditorPropsParams): TExtendedEditorProps => + ({} as unknown as TExtendedEditorProps);Also applies to: 16-16
12-12: Tighten handler typing.Replace
Map<string, unknown>with a concrete handler type (e.g.,Map<string, EditorExtensionHandler>orRecord<string, EditorExtensionHandler>) to retain type safety across the extension surface.apps/web/core/components/pages/editor/page-root.tsx (1)
169-170: Guard possibly-null editorRef when passing to PageModals.Avoid passing null if the prop isn’t nullable.
Apply this diff:
-<PageModals page={page} storeType={storeType} editorRef={editorRef.current} /> +<PageModals page={page} storeType={storeType} editorRef={editorRef.current ?? undefined} />
📜 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)
apps/web/ce/hooks/pages/use-extended-editor-extensions.ts(1 hunks)apps/web/core/components/pages/editor/page-root.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/ce/hooks/pages/use-extended-editor-extensions.ts (2)
apps/web/core/store/pages/base-page.ts (1)
TPageInstance(69-72)packages/editor/src/ce/types/editor-extended.ts (1)
IEditorPropsExtended(3-3)
apps/web/core/components/pages/editor/page-root.tsx (4)
apps/web/ce/hooks/pages/use-pages-pane-extensions.ts (1)
usePagesPaneExtensions(19-53)apps/web/ce/hooks/pages/use-extended-editor-extensions.ts (1)
useExtendedEditorProps(18-18)apps/web/core/components/pages/navigation-pane/root.tsx (1)
PageNavigationPaneRoot(39-117)apps/web/ce/components/pages/modals/modals.tsx (1)
PageModals(17-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)
apps/web/core/components/pages/editor/page-root.tsx (2)
42-45: Propagating storeType/projectId across the editor stack looks good.Wiring through PageVersionsOverlay, PageEditorBody, PageNavigationPaneRoot, and PageModals is consistent.
Also applies to: 49-49, 134-135, 152-153, 159-159
99-101: No changes needed—handlers are declared via TEditorBodyHandlers
TPageRootHandlers is defined as& TEditorBodyHandlers, and TEditorBodyHandlers declares bothfetchEntityandgetRedirectionLink.
Palanikannan1437
changed the title
There was a problem hiding this comment.
Actionable comments posted: 6
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/components/editors/document/collaborative-editor.tsx (1)
65-65: Bug: unsafe dependency access in useMemo (possible runtime crash).Dependency array references embedHandler.issue without optional chaining, but embedHandler can be undefined. This will throw at render time.
- }, [externalExtensions, embedHandler.issue]); + }, [externalExtensions, embedHandler?.issue]); + // Alternatively, depend on the exact value used: + // }, [externalExtensions, embedHandler?.issue?.widgetCallback]);apps/web/core/components/editor/document/editor.tsx (1)
84-87: Bug: spread on possibly undefined embedHandler throws at runtime....embedHandler will crash if embedHandler is undefined.
Fix via defaulting in destructure (preferred):
- embedHandler, + embedHandler = {},or locally:
- embedHandler={{ - issue: issueEmbedProps, - ...embedHandler, - }} + embedHandler={{ issue: issueEmbedProps, ...(embedHandler ?? {}) }}
♻️ Duplicate comments (3)
packages/editor/src/core/types/editor.ts (1)
9-9: CE placeholder fix is correctly consumed here.Importing IEditorPropsExtended and TExtendedEditorCommands from the plane-editor entry looks good; with CE exporting TExtendedEditorCommands = never, this no longer widens TEditorCommands with an empty string.
apps/web/core/components/pages/editor/page-root.tsx (2)
54-55: Fix ref type to include null (strict TS).-const editorRef = useRef<EditorRefApi>(null); +const editorRef = useRef<EditorRefApi | null>(null);
87-93: Pane won’t open for custom extensions (hook returns empty extensions and ignores paneTab).
usePagesPaneExtensionscurrently returnsnavigationPaneExtensions = []and setsisNavigationPaneOpenonly for built-in tabs. When?paneTab=<extension>is present, the aside stays off-canvas. Update the hook to populate extensions and treat an active extension as “open.”Suggested change (in apps/web/ce/hooks/pages/use-pages-pane-extensions.ts):
- const navigationPaneExtensions: INavigationPaneExtension[] = []; - - return { - editorExtensionHandlers, - navigationPaneExtensions, - handleOpenNavigationPane, - isNavigationPaneOpen, - }; + // TODO: load registered extensions instead of empty array + const navigationPaneExtensions: INavigationPaneExtension[] = getRegisteredNavigationPaneExtensions?.() ?? []; + const paneTabValue = searchParams.get("paneTab"); + const hasActiveExtension = !!navigationPaneExtensions.find((e) => e.triggerParam === paneTabValue); + + return { + editorExtensionHandlers, + navigationPaneExtensions, + handleOpenNavigationPane, + isNavigationPaneOpen: isNavigationPaneOpen || hasActiveExtension, + };Verification: navigate to
?paneTab=<your-extension-trigger>and ensure the aside animates in.
🧹 Nitpick comments (11)
apps/web/ce/types/pages/pane-extensions.ts (1)
6-7: Make the data map augmentable via declaration merging (future-proof CE/EE).Switching from Record to an interface enables external augmentation without touching this file.
Apply:
-// EE Union/map of extension data types (keyed by extension id) -export type TNavigationPaneExtensionData = Record<string, unknown>; +// EE Union/map of extension data types (keyed by extension id) +// Interface form allows module augmentation by EE/plugins. +export interface CEPaneExtensionDataMap { + [key: string]: unknown; +} +export type TNavigationPaneExtensionData = CEPaneExtensionDataMap;packages/editor/src/core/types/editor.ts (1)
163-164: Mandatory extendedEditorProps is a breaking API — verify all call sites are updated.IEditorProps now requires extendedEditorProps. Internal wrappers pass {} which is fine for CE’s unknown type. Please confirm no external consumers (outside this repo) rely on the old optional surface.
apps/space/core/components/editor/rich-text-editor.tsx (1)
59-59: Avoid new-object prop churn for extendedEditorProps.Passing {} creates a new reference each render and can cause unnecessary re-renders downstream. Memoize a stable empty object.
export const RichTextEditor = forwardRef<EditorRefApi, RichTextEditorWrapperProps>((props, ref) => { const { @@ const { richText: richTextEditorExtensions } = useEditorFlagging(anchor); + const emptyExtendedEditorProps = React.useMemo(() => ({} as const), []); + return ( <RichTextEditorWithRef @@ - extendedEditorProps={{}} + extendedEditorProps={emptyExtendedEditorProps}packages/editor/src/core/components/editors/document/collaborative-editor.tsx (1)
16-16: Minor: incorrect comment section.The "// constants" label doesn’t match the import (component). Consider correcting for readability.
-// constants +// ce side-effects import { DocumentEditorSideEffects } from "@/plane-editor/components/editor-side-effects";apps/space/core/components/editor/lite-text-editor.tsx (1)
65-65: Memoize empty extendedEditorProps to reduce re-renders.export const LiteTextEditor = React.forwardRef<EditorRefApi, LiteTextEditorWrapperProps>((props, ref) => { @@ const { liteText: liteTextEditorExtensions } = useEditorFlagging(anchor); + const emptyExtendedEditorProps = React.useMemo(() => ({} as const), []); @@ - extendedEditorProps={{}} + extendedEditorProps={emptyExtendedEditorProps}apps/web/core/components/editor/lite-text/editor.tsx (1)
131-131: Memoize empty extendedEditorProps to avoid prop identity churn.export const LiteTextEditor = React.forwardRef<EditorRefApi, LiteTextEditorWrapperProps>((props, ref) => { @@ const editorRef = isMutableRefObject<EditorRefApi>(ref) ? ref.current : null; + const emptyExtendedEditorProps = React.useMemo(() => ({} as const), []); @@ - extendedEditorProps={{}} + extendedEditorProps={emptyExtendedEditorProps}packages/editor/src/ce/components/editor-side-effects.ts (1)
5-10: Narrow the callback type; export it for reuse across CE/EE.Avoid unknown for updatePageProperties; define a specific callback type and use it.
Apply:
export type DocumentEditorSideEffectsProps = { editor: Editor; id: string; - updatePageProperties?: unknown; + /** + * Callback to update page metadata/properties. + * Keep broad but non-any to enable cross-package reuse. + */ + updatePageProperties?: UpdatePageProperties; extendedEditorProps?: IEditorPropsExtended; }; +export type UpdatePageProperties = (changes: Record<string, unknown>) => void;apps/web/core/components/editor/document/editor.tsx (1)
48-50: Nit: unnecessary toString/empty fallback.workspaceSlug is already a string in props; pass it directly.
- const { document: documentEditorExtensions } = useEditorFlagging({ - workspaceSlug: workspaceSlug?.toString() ?? "", - }); + const { document: documentEditorExtensions } = useEditorFlagging({ workspaceSlug });apps/web/core/components/editor/rich-text/editor.tsx (1)
45-47: Nit: unnecessary toString/empty fallback.- const { richText: richTextEditorExtensions } = useEditorFlagging({ - workspaceSlug: workspaceSlug?.toString() ?? "", - }); + const { richText: richTextEditorExtensions } = useEditorFlagging({ workspaceSlug });apps/web/core/components/editor/sticky-editor/editor.tsx (1)
54-56: Nit: unnecessary toString/empty fallback.- const { liteText: liteTextEditorExtensions } = useEditorFlagging({ - workspaceSlug: workspaceSlug?.toString() ?? "", - }); + const { liteText: liteTextEditorExtensions } = useEditorFlagging({ workspaceSlug });apps/web/core/components/pages/editor/editor-body.tsx (1)
109-112: Nit: avoid defaulting storeType to an empty string.storeType is typed; passing "" weakens type guarantees.
- const { document: documentEditorExtensions } = useEditorFlagging({ - workspaceSlug: workspaceSlug?.toString() ?? "", - storeType: storeType ?? "", - }); + const { document: documentEditorExtensions } = useEditorFlagging({ + workspaceSlug, + storeType, + });
📜 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 (17)
apps/space/core/components/editor/lite-text-editor.tsx(2 hunks)apps/space/core/components/editor/rich-text-editor.tsx(2 hunks)apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx(8 hunks)apps/web/ce/hooks/pages/use-extended-editor-extensions.ts(1 hunks)apps/web/ce/hooks/pages/use-pages-pane-extensions.ts(1 hunks)apps/web/ce/hooks/use-editor-flagging.ts(2 hunks)apps/web/ce/types/pages/pane-extensions.ts(1 hunks)apps/web/core/components/editor/document/editor.tsx(3 hunks)apps/web/core/components/editor/lite-text/editor.tsx(3 hunks)apps/web/core/components/editor/rich-text/editor.tsx(3 hunks)apps/web/core/components/editor/sticky-editor/editor.tsx(3 hunks)apps/web/core/components/pages/editor/editor-body.tsx(7 hunks)apps/web/core/components/pages/editor/page-root.tsx(5 hunks)packages/editor/src/ce/components/editor-side-effects.ts(1 hunks)packages/editor/src/ce/types/editor-extended.ts(1 hunks)packages/editor/src/core/components/editors/document/collaborative-editor.tsx(3 hunks)packages/editor/src/core/types/editor.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/editor/src/ce/types/editor-extended.ts
- apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx
- apps/web/ce/hooks/pages/use-extended-editor-extensions.ts
- apps/web/ce/hooks/pages/use-pages-pane-extensions.ts
- apps/web/ce/hooks/use-editor-flagging.ts
🧰 Additional context used
🧬 Code graph analysis (12)
apps/web/ce/types/pages/pane-extensions.ts (1)
apps/web/core/components/pages/navigation-pane/types/extensions.ts (2)
INavigationPaneExtension(15-21)INavigationPaneExtensionComponent(11-13)
packages/editor/src/core/components/editors/document/collaborative-editor.tsx (2)
packages/editor/src/ce/components/editor-side-effects.ts (1)
DocumentEditorSideEffects(12-12)packages/editor/src/core/components/editors/document/page-renderer.tsx (1)
PageRenderer(25-73)
apps/space/core/components/editor/rich-text-editor.tsx (1)
packages/editor/src/core/types/editor.ts (1)
IRichTextEditorProps(168-170)
apps/web/core/components/editor/rich-text/editor.tsx (3)
packages/editor/src/core/types/editor.ts (1)
IRichTextEditorProps(168-170)apps/web/ce/hooks/use-editor-flagging.ts (1)
useEditorFlagging(28-41)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/components/editor/document/editor.tsx (3)
packages/editor/src/core/types/editor.ts (1)
IDocumentEditorProps(183-188)apps/web/ce/hooks/use-editor-flagging.ts (1)
useEditorFlagging(28-41)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
packages/editor/src/ce/components/editor-side-effects.ts (1)
packages/editor/src/ce/types/editor-extended.ts (1)
IEditorPropsExtended(3-3)
packages/editor/src/core/types/editor.ts (1)
packages/editor/src/ce/types/editor-extended.ts (2)
TExtendedEditorCommands(5-5)IEditorPropsExtended(3-3)
apps/space/core/components/editor/lite-text-editor.tsx (1)
packages/editor/src/core/types/editor.ts (1)
ILiteTextEditorProps(166-166)
apps/web/core/components/editor/lite-text/editor.tsx (3)
packages/editor/src/core/types/editor.ts (1)
ILiteTextEditorProps(166-166)apps/web/ce/hooks/use-editor-flagging.ts (1)
useEditorFlagging(28-41)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/components/editor/sticky-editor/editor.tsx (2)
packages/editor/src/core/types/editor.ts (1)
ILiteTextEditorProps(166-166)apps/web/ce/hooks/use-editor-flagging.ts (1)
useEditorFlagging(28-41)
apps/web/core/components/pages/editor/editor-body.tsx (3)
apps/web/ce/hooks/pages/use-extended-editor-extensions.ts (1)
TExtendedEditorExtensionsConfig(16-16)apps/web/ce/hooks/use-editor-flagging.ts (1)
useEditorFlagging(28-41)apps/web/core/components/pages/editor/header/root.tsx (1)
PageEditorHeaderRoot(17-71)
apps/web/core/components/pages/editor/page-root.tsx (4)
apps/web/ce/hooks/pages/use-pages-pane-extensions.ts (1)
usePagesPaneExtensions(19-52)apps/web/ce/hooks/pages/use-extended-editor-extensions.ts (1)
useExtendedEditorProps(18-18)apps/web/core/components/pages/navigation-pane/root.tsx (1)
PageNavigationPaneRoot(39-117)apps/web/ce/components/pages/modals/modals.tsx (1)
PageModals(17-17)
🔇 Additional comments (20)
apps/web/ce/types/pages/pane-extensions.ts (1)
1-4: Type-only imports via the barrel look good.Using type-only imports keeps runtime clean and avoids circular deps at emit time. LGTM.
packages/editor/src/core/types/editor.ts (1)
55-56: Add extra-props mapping if external-embed requires parameters.You’ve added "external-embed" to TEditorCommands. If this command needs runtime params (e.g., url, provider), add a corresponding entry to TCommandExtraProps to preserve type-safety for executeMenuItemCommand and isMenuItemActive.
apps/space/core/components/editor/rich-text-editor.tsx (1)
15-16: Good: tightened public API by omitting extendedEditorProps.This aligns the wrapper with the new required prop on the inner editor while keeping the wrapper surface stable.
packages/editor/src/core/components/editors/document/collaborative-editor.tsx (1)
103-119: Side-effects mount looks correct.Rendering DocumentEditorSideEffects alongside PageRenderer cleanly injects lifecycle hooks without coupling. No functional issues spotted.
apps/space/core/components/editor/lite-text-editor.tsx (2)
5-5: Good: isCommentEmpty import relocation.Switching to @plane/utils keeps helpers centralized.
14-15: Good: wrapper omits extendedEditorProps.Keeps external API lean while satisfying inner editor’s required prop.
apps/web/core/components/editor/lite-text/editor.tsx (2)
21-23: Good: wrapper omits extendedEditorProps.Matches the new core types and internalizes the concern.
71-73: Good: useEditorFlagging now takes a config object.Passing workspaceSlug via an object aligns with the updated hook signature.
packages/editor/src/ce/components/editor-side-effects.ts (2)
1-3: Good: typed component and public props export.Imports and explicit ReactElement | null return are correct. Props type is exported for reuse.
12-12: All DocumentEditorSideEffects calls use JSX with object props
Search across the codebase did not find any non-JSX invocations; no further changes required.apps/web/core/components/editor/rich-text/editor.tsx (1)
14-16: LGTM: internalizing extendedEditorProps in the wrapper.Public API tightened; defaulted internally to {}. Matches the document editor pattern.
Also applies to: 78-79
apps/web/core/components/editor/sticky-editor/editor.tsx (1)
18-20: LGTM: removed extendedEditorProps from public props.Keeps the wrapper lean; forwards a stable default internally.
apps/web/core/components/pages/editor/editor-body.tsx (4)
28-31: LGTM: typed extension config and store type imports.These align the editor with the extended-editor and store-type plumbing.
259-260: LGTM: forwarding extendedEditorProps to the collaborative editor.Keeps extended-editor wiring consistent end-to-end.
49-50: Ensure implementers supply getRedirectionLink
No occurrences of<PageEditorBody handlers={…}>were found—manually review all consumers ofTEditorBodyHandlersto implement the newgetRedirectionLinkhandler.
221-221: Props include projectId in PageEditorHeaderRoot
ThePropstype inapps/web/core/components/pages/editor/header/root.tsxalready declaresprojectId: string, so no changes are needed.apps/web/core/components/pages/editor/page-root.tsx (4)
11-14: LGTM: CE integration wiring looks correct.Imports for PageModals, pane/extended-editor hooks, and EPageStoreType are appropriate for the new architecture.
42-45: LGTM: storeType/projectId propagation is consistent.Passing storeType and projectId through PageVersionsOverlay, PageEditorBody, and PageNavigationPaneRoot aligns with the new extensibility model.
Also applies to: 137-138, 154-156, 162-165, 170-170
172-172: Confirm PageModals editorRef prop type.You pass
editorRef={editorRef.current}(instance). If PageModals expects a ref object, passeditorRef={editorRef}instead. Verify the prop type and adjust accordingly.
121-127: Ignore redundant paneTab removal ThePAGE_NAVIGATION_PANE_TABS_QUERY_PARAMconstant is defined as"paneTab", so the existingparamsToRemovealready clears the extension pane tab. No change needed.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
apps/web/core/components/pages/navigation-pane/root.tsx (1)
56-57: Good: using query-param constants instead of string literals.Prevents drift and keeps routing consistent across components.
Also applies to: 76-78
apps/web/core/components/pages/editor/page-root.tsx (4)
105-111: Restore action updates UI only; persist to server and accept versionId.Wire
handlers.restoreVersionso the server state is updated, then refresh editor content.- const handleRestoreVersion = useCallback( - async (descriptionHTML: string) => { - editorRef.current?.clearEditor(); - editorRef.current?.setEditorValue(descriptionHTML); - }, - [editorRef] - ); + const handleRestoreVersion = useCallback( + async (versionId: string, descriptionHTML: string) => { + try { + await handlers.restoreVersion(page.id ?? "", versionId); + editorRef.current?.clearEditor(); + editorRef.current?.setEditorValue(descriptionHTML); + } catch (e) { + // surface/pipe to toast as needed + console.error("Failed to restore version", e); + throw e; + } + }, + [editorRef, handlers, page.id] + );Also update
PageVersionsOverlayprop signature and calls accordingly (outside this file):// apps/web/core/components/pages/version/root.tsx (prop type) export type TPageVersionsOverlayProps = { handleRestore: (versionId: string, descriptionHTML: string) => Promise<void>; // ... }; // wherever PageVersionsOverlay invokes the callback props.handleRestore(version.id, version.descriptionHTML);
54-55: Fix strict TS ref typing.Include
nullin the ref type to match the initial value.- const editorRef = useRef<EditorRefApi>(null); + const editorRef = useRef<EditorRefApi | null>(null);
95-103: Pass potentially-absent handlers conditionally to avoid hard dependency.Avoid passing undefined fields unconditionally; keeps CE hooks flexible.
- const extendedEditorProps = useExtendedEditorProps({ - workspaceSlug, - page, - storeType, - fetchEntity: handlers.fetchEntity, - getRedirectionLink: handlers.getRedirectionLink, - extensionHandlers: editorExtensionHandlers, - projectId, - }); + const extendedEditorProps = useExtendedEditorProps({ + workspaceSlug, + page, + storeType, + extensionHandlers: editorExtensionHandlers, + projectId, + ...(handlers.fetchEntity && { fetchEntity: handlers.fetchEntity }), + ...(handlers.getRedirectionLink && { getRedirectionLink: handlers.getRedirectionLink }), + } as any);
27-34: Type drift: optional handlers referenced below aren’t declared on TPageRootHandlers.
handlers.fetchEntityandhandlers.getRedirectionLinkare passed later but not part of this type. Add them as optional to keep the build green and reflect reality.export type TPageRootHandlers = { create: (payload: Partial<TPage>) => Promise<Partial<TPage> | undefined>; fetchAllVersions: (pageId: string) => Promise<TPageVersion[] | undefined>; fetchDescriptionBinary: () => Promise<any>; fetchVersionDetails: (pageId: string, versionId: string) => Promise<TPageVersion | undefined>; restoreVersion: (pageId: string, versionId: string) => Promise<void>; updateDescription: (document: TDocumentPayload) => Promise<void>; + // optional wiring used by CE/editor extensions + getRedirectionLink?: (...args: any[]) => string; + fetchEntity?: (...args: any[]) => Promise<any>; } & TEditorBodyHandlers;
🧹 Nitpick comments (2)
apps/web/core/components/pages/editor/page-root.tsx (2)
11-13: ImportEPageStoreTypeas a type-only import (if it’s not a runtime enum).Prevents bundling an unnecessary runtime import; align with navigation-pane/root.tsx. Validate first since enums require runtime.
-import { PageModals } from "@/plane-web/components/pages"; -import { usePagesPaneExtensions, useExtendedEditorProps } from "@/plane-web/hooks/pages"; -import { EPageStoreType } from "@/plane-web/hooks/store"; +import { PageModals } from "@/plane-web/components/pages"; +import { usePagesPaneExtensions, useExtendedEditorProps } from "@/plane-web/hooks/pages"; +import { type EPageStoreType } from "@/plane-web/hooks/store";
87-93: Pane open-state likely ignores custom extensions; ensure hook treats extension triggers as open.
usePagesPaneExtensions(CE) returns an emptynavigationPaneExtensionsand sets open-state only for built-ins. With?paneTab=<extension-trigger>, the pane may remain closed. Update the hook to populate extensions and mark open when the trigger matches an extension.Suggested patch (in apps/web/ce/hooks/pages/use-pages-pane-extensions.ts):
const paneTab = searchParams.get(PAGE_NAVIGATION_PANE_TABS_QUERY_PARAM); const isBuiltin = PAGE_NAVIGATION_PANE_TAB_KEYS.includes(paneTab as any); const hasActiveExtension = navigationPaneExtensions.some(e => e.triggerParam === paneTab); const isNavigationPaneOpen = !!paneTab && (isBuiltin || hasActiveExtension);If you want, I can open a follow-up PR to wire extension registration and tests.
Also applies to: 161-173
📜 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 (4)
apps/web/ce/components/pages/modals/modals.tsx(1 hunks)apps/web/core/components/pages/editor/editor-body.tsx(8 hunks)apps/web/core/components/pages/editor/page-root.tsx(5 hunks)apps/web/core/components/pages/navigation-pane/root.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/ce/components/pages/modals/modals.tsx
- apps/web/core/components/pages/editor/editor-body.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/core/components/pages/editor/page-root.tsx (4)
apps/web/ce/hooks/pages/use-pages-pane-extensions.ts (1)
usePagesPaneExtensions(19-52)apps/web/ce/hooks/pages/use-extended-editor-extensions.ts (1)
useExtendedEditorProps(18-20)apps/web/core/components/pages/navigation-pane/root.tsx (1)
PageNavigationPaneRoot(39-117)apps/web/ce/components/pages/modals/modals.tsx (1)
PageModals(15-15)
apps/web/core/components/pages/navigation-pane/root.tsx (3)
apps/web/ce/types/pages/pane-extensions.ts (1)
INavigationPaneExtension(10-16)apps/web/core/components/pages/navigation-pane/types/extensions.ts (1)
INavigationPaneExtension(15-21)apps/web/core/components/pages/navigation-pane/index.ts (3)
PAGE_NAVIGATION_PANE_TABS_QUERY_PARAM(9-9)PAGE_NAVIGATION_PANE_VERSION_QUERY_PARAM(10-10)PAGE_NAVIGATION_PANE_WIDTH(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: Analyze (javascript)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/editor/src/ce/components/document-editor-side-effects.ts (1)
2-2: Optional: add a minimal editor “update” hook skeleton.Keeps UI output null while demonstrating where page-prop side effects will wire in. Adjust the event name if your Tiptap Editor instance differs.
-import type { ReactElement } from "react"; +import { useEffect, type ReactElement } from "react"; -export const DocumentEditorSideEffects = (_props: DocumentEditorSideEffectsProps): ReactElement | null => null; +export const DocumentEditorSideEffects = ({ editor, id, updatePageProperties }: DocumentEditorSideEffectsProps): ReactElement | null => { + useEffect(() => { + if (!updatePageProperties) return; + const onUpdate = () => { + // Example payload; replace with your actual page props patch + updatePageProperties({ lastEditedAt: new Date().toISOString() }); + }; + // Verify the event name against your Tiptap version (e.g., "update" or "transaction"). + // @ts-expect-error - event signature depends on Editor version; confirm at call sites. + editor.on?.("update", onUpdate); + return () => { + // @ts-expect-error - see note above. + editor.off?.("update", onUpdate); + }; + }, [editor, id, updatePageProperties]); + return null; +};Also applies to: 12-12
📜 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/ce/components/document-editor-side-effects.ts(1 hunks)packages/editor/src/core/components/editors/document/collaborative-editor.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/editor/src/core/components/editors/document/collaborative-editor.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/editor/src/ce/components/document-editor-side-effects.ts (1)
packages/editor/src/ce/types/editor-extended.ts (1)
IEditorPropsExtended(3-3)
🔇 Additional comments (2)
packages/editor/src/ce/components/document-editor-side-effects.ts (2)
1-1: Good use of type-only import for Editor.Avoids pulling runtime code and keeps bundle clean.
12-12: No default import usage detected—named export is correct. Verified all call sites importDocumentEditorSideEffectsas a named export; no default export needed.
| export type DocumentEditorSideEffectsProps = { | ||
| editor: Editor; | ||
| id: string; | ||
| updatePageProperties?: unknown; | ||
| extendedEditorProps?: IEditorPropsExtended; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Type the updatePageProperties callback (avoid unknown).
Use an explicit callable signature so consumers get type-safety and IntelliSense.
export type DocumentEditorSideEffectsProps = {
editor: Editor;
id: string;
- updatePageProperties?: unknown;
+ updatePageProperties?: (patch: Record<string, unknown>) => void | Promise<void>;
extendedEditorProps?: IEditorPropsExtended;
};📝 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.
| export type DocumentEditorSideEffectsProps = { | |
| editor: Editor; | |
| id: string; | |
| updatePageProperties?: unknown; | |
| extendedEditorProps?: IEditorPropsExtended; | |
| }; | |
| export type DocumentEditorSideEffectsProps = { | |
| editor: Editor; | |
| id: string; | |
| updatePageProperties?: (patch: Record<string, unknown>) => void | Promise<void>; | |
| extendedEditorProps?: IEditorPropsExtended; | |
| }; |
🤖 Prompt for AI Agents
In packages/editor/src/ce/components/document-editor-side-effects.ts around
lines 5 to 10, replace the updatePageProperties field typed as unknown with a
concrete callable signature (for example: updatePageProperties?: (id: string,
properties: Partial<PageProperties>) => void | Promise<void>); import or define
the PageProperties type appropriate for your domain (or use a clearly named
interface if it doesn't exist yet) so consumers get type-safety and
IntelliSense, and update any call sites to match the new signature.
3 participants
Description
Refactoring page root and editor body
Summary by CodeRabbit
New Features
Refactor
Chores