[WIKI-539] refactor: remove lite text read only editor#7481
[WIKI-539] refactor: remove lite text read only editor#7481
Conversation
WalkthroughThis change removes all code, types, and exports related to "read-only" editor components and APIs across the codebase. The editor's editable state is now managed via a single Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component
participant Editor as LiteTextEditor
participant FileHandler as FileHandler
participant MentionHandler as MentionHandler
UI->>Editor: Render LiteTextEditor with editable={true|false}
alt editable = true
Editor->>FileHandler: Use provided uploadFile for file uploads
else editable = false
Editor->>FileHandler: Use no-op uploadFile (uploads disabled)
end
Editor->>MentionHandler: Use unified mention handler for mentions
Editor-->>UI: Rendered editor (editable or read-only as per prop)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 1
🧹 Nitpick comments (4)
packages/editor/src/core/extensions/callout/extension.tsx (2)
66-68: Simplify NodeView renderer – avoid the manual type-cast
ReactNodeViewRenderercan accept the component directly; the extra wrapper and cast are unnecessary and lose type-safety.- return ReactNodeViewRenderer((props) => ( - <CustomCalloutBlock {...props} node={props.node as CustomCalloutNodeViewProps["node"]} /> - )); + // The component already satisfies NodeViewProps; no additional cast required. + return ReactNodeViewRenderer(CustomCalloutBlock);
42-60: Backspace shortcut: narrow thetry / catchand surface unexpected errorsCatching every error and merely logging it risks masking genuine editor bugs.
Consider restricting thetryblock to only the line that can throw (thefindParentNodeClosestToPoscall) or re-throw after logging so upstream error handlers (e.g. Sentry) are triggered.apps/web/core/components/comments/card/root.tsx (1)
6-6: LGTM: Editor API unification implemented correctly.The change from
EditorReadOnlyRefApitoEditorRefApialigns with the unified editor API approach. Consider renamingreadOnlyEditorReftoeditorRefin a future refactor to better reflect its unified nature.Also applies to: 40-40
packages/editor/src/core/helpers/editor-ref.ts (1)
74-94: Consider performance optimization for getSelectedTextThe current implementation creates DOM elements for each selected node, which could impact performance for large selections. Consider caching or using a more efficient serialization approach.
For better performance with large selections, consider:
- Using a document fragment instead of individual div elements
- Implementing debouncing if this method is called frequently
- Caching the serializer instance
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
apps/space/core/components/editor/index.ts(0 hunks)apps/space/core/components/editor/lite-text-editor.tsx(3 hunks)apps/space/core/components/editor/lite-text-read-only-editor.tsx(0 hunks)apps/space/core/components/issues/peek-overview/comment/add-comment.tsx(1 hunks)apps/space/core/components/issues/peek-overview/comment/comment-detail-card.tsx(3 hunks)apps/web/core/components/comments/card/display.tsx(3 hunks)apps/web/core/components/comments/card/edit-form.tsx(3 hunks)apps/web/core/components/comments/card/root.tsx(2 hunks)apps/web/core/components/comments/comment-create.tsx(1 hunks)apps/web/core/components/editor/lite-text/editor.tsx(5 hunks)apps/web/core/components/editor/lite-text/index.ts(0 hunks)apps/web/core/components/editor/lite-text/read-only-editor.tsx(0 hunks)apps/web/core/components/editor/rich-text/editor.tsx(3 hunks)apps/web/core/components/editor/sticky-editor/editor.tsx(2 hunks)apps/web/core/hooks/editor/use-editor-config.ts(3 hunks)packages/editor/src/ce/extensions/core/index.ts(0 hunks)packages/editor/src/ce/extensions/core/read-only-extensions.ts(0 hunks)packages/editor/src/ce/extensions/rich-text/read-only-extensions.tsx(0 hunks)packages/editor/src/core/components/editors/index.ts(0 hunks)packages/editor/src/core/components/editors/lite-text/editor.tsx(1 hunks)packages/editor/src/core/components/editors/lite-text/index.ts(0 hunks)packages/editor/src/core/components/editors/lite-text/read-only-editor.tsx(0 hunks)packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx(0 hunks)packages/editor/src/core/components/editors/rich-text/editor.tsx(1 hunks)packages/editor/src/core/extensions/callout/extension.tsx(1 hunks)packages/editor/src/core/extensions/callout/index.ts(0 hunks)packages/editor/src/core/extensions/callout/read-only-extension.tsx(0 hunks)packages/editor/src/core/extensions/custom-image/extension.tsx(1 hunks)packages/editor/src/core/extensions/image/extension.tsx(2 hunks)packages/editor/src/core/extensions/index.ts(0 hunks)packages/editor/src/core/extensions/read-only-extensions.ts(0 hunks)packages/editor/src/core/extensions/utility.ts(2 hunks)packages/editor/src/core/helpers/editor-ref.ts(2 hunks)packages/editor/src/core/hooks/use-editor.ts(2 hunks)packages/editor/src/core/hooks/use-read-only-editor.ts(0 hunks)packages/editor/src/core/plugins/file/root.ts(1 hunks)packages/editor/src/core/props.ts(1 hunks)packages/editor/src/core/props/index.ts(0 hunks)packages/editor/src/core/props/read-only.tsx(0 hunks)packages/editor/src/core/types/config.ts(1 hunks)packages/editor/src/core/types/editor.ts(3 hunks)packages/editor/src/core/types/hook.ts(1 hunks)packages/editor/src/core/types/mention.ts(1 hunks)packages/editor/src/index.ts(0 hunks)
💤 Files with no reviewable changes (19)
- packages/editor/src/core/components/editors/lite-text/index.ts
- packages/editor/src/core/extensions/index.ts
- packages/editor/src/index.ts
- packages/editor/src/core/components/editors/index.ts
- apps/web/core/components/editor/lite-text/index.ts
- apps/space/core/components/editor/index.ts
- packages/editor/src/ce/extensions/core/index.ts
- packages/editor/src/core/props/read-only.tsx
- packages/editor/src/ce/extensions/core/read-only-extensions.ts
- packages/editor/src/core/extensions/callout/read-only-extension.tsx
- packages/editor/src/core/components/editors/lite-text/read-only-editor.tsx
- packages/editor/src/core/extensions/callout/index.ts
- packages/editor/src/core/props/index.ts
- packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx
- apps/web/core/components/editor/lite-text/read-only-editor.tsx
- packages/editor/src/core/hooks/use-read-only-editor.ts
- apps/space/core/components/editor/lite-text-read-only-editor.tsx
- packages/editor/src/ce/extensions/rich-text/read-only-extensions.tsx
- packages/editor/src/core/extensions/read-only-extensions.ts
⏰ 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). (4)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
- GitHub Check: Build and lint web apps
🔇 Additional comments (37)
packages/editor/src/core/extensions/callout/extension.tsx (1)
3-3: No stale imports from@/extensions/calloutremainI searched all
.ts/.tsxfiles for any imports of the old barrel path and found none. The updated import to@/extensions/callout/blockis correctly applied everywhere.packages/editor/src/core/props.ts (1)
5-7: No action needed:TArgsis a local, consistent alias
TheTArgstype in packages/editor/src/core/props.ts is a module‐local alias (not exported) and follows the same pattern used across multiple files (e.g. helpers, extensions, hooks). There is noTCoreEditorPropsin the codebase and no downstream imports to break. Please disregard the earlier suggestion to rename or re-export—this change aligns with existing conventions.Likely an incorrect or invalid review comment.
packages/editor/src/core/components/editors/rich-text/editor.tsx (1)
8-8: LGTM! Import path updated correctly.The import path change for
RichTextEditorAdditionalExtensionsaligns with the refactoring to consolidate editor extensions.packages/editor/src/core/components/editors/lite-text/editor.tsx (1)
22-22: Good simplification of prop handling.Removing the explicit
editableprop and relying on spread props is a clean approach that maintains functionality while allowing more flexible prop passing. This aligns well with the unified editor approach.apps/web/core/components/comments/comment-create.tsx (1)
116-116: Correct addition ofeditableprop.Adding
editable={true}to theLiteTextEditoris the right approach for comment creation, ensuring users can edit when creating new comments. This properly adapts to the unified editor API.apps/space/core/components/issues/peek-overview/comment/add-comment.tsx (1)
78-78: Consistent implementation of unified editor pattern.Adding
editable={true}for the comment addition functionality is correct and maintains the expected user experience while adapting to the consolidated editor API.packages/editor/src/core/types/hook.ts (1)
5-5: Good type consolidation aligns with refactoring goals.Removing
IReadOnlyEditorPropsfrom the import and eliminatingTReadOnlyEditorHookPropssimplifies the type system by consolidating read-only and editable editor functionality into unified types. This change supports the broader effort to use a single editor component with expliciteditablecontrol.packages/editor/src/core/extensions/custom-image/extension.tsx (1)
9-9: LGTM: Type consolidation aligns with refactoring goal.The removal of
TReadOnlyFileHandlerimport and narrowing thefileHandlerprop type to onlyTFileHandlercorrectly implements the unified file handler approach. The existing conditional checks for optional properties (upload,validation) ensure backward compatibility.Also applies to: 16-16
packages/editor/src/core/extensions/image/extension.tsx (1)
5-5: LGTM: Consistent type consolidation.The changes follow the same pattern as other extensions, removing
TReadOnlyFileHandlerand unifying the file handler type. The conditional property checks maintain compatibility with different file handler configurations.Also applies to: 15-15
packages/editor/src/core/plugins/file/root.ts (1)
4-4: LGTM: File handler type unification maintains flexibility.The consolidation to
TFileHandleris consistent with the refactoring goal. The conditional logic checking for thedeletemethod ensures that different file handler implementations can still provide varying capabilities.Also applies to: 11-11
apps/web/core/components/comments/card/edit-form.tsx (2)
6-6: LGTM: Editor API type unification.The change from
EditorReadOnlyRefApitoEditorRefApicorrectly implements the unified editor reference API approach.Also applies to: 17-17
78-78: LGTM: Explicit editability control.The addition of the explicit
editableprop makes the editor's behavior clear and aligns with the unified editor approach where editability is controlled via props rather than separate components.apps/web/core/components/editor/rich-text/editor.tsx (2)
3-4: LGTM! Improved type imports and type safety.The explicit
typeimports clarify these are TypeScript type-only imports, and the added default value fordisabledExtensionsprevents potential undefined issues.
71-75: LGTM! Cleaner mentionHandler implementation.The simplification from an inline arrow function to direct component reference is more readable, and removing the explicit type annotation is fine since TypeScript can infer the parameter type.
apps/web/core/components/comments/card/display.tsx (3)
6-6: LGTM! Correct type consolidation.The import change from
EditorReadOnlyRefApitoEditorRefApialigns with the unified editor API refactoring.
20-20: LGTM! Prop type updated correctly.The
readOnlyEditorRefprop type is correctly updated to use the unifiedEditorRefApiinterface.
70-71: LGTM! Proper migration to unified editor.The replacement of
LiteTextReadOnlyEditorwithLiteTextEditorpluseditable={false}correctly maintains the read-only behavior while using the unified component.apps/space/core/components/issues/peek-overview/comment/comment-detail-card.tsx (3)
11-11: LGTM! Import correctly updated.The import change from
LiteTextReadOnlyEditortoLiteTextEditoraligns with the unified editor component approach.
104-105: LGTM! Correct editable mode configuration.The
LiteTextEditoris properly configured witheditable(which defaults totruein JSX) for the editing form, and theuploadFilehandler is correctly provided.
142-143: LGTM! Proper read-only mode configuration.The replacement of
LiteTextReadOnlyEditorwithLiteTextEditorpluseditable={false}correctly maintains the read-only display behavior.packages/editor/src/core/types/mention.ts (1)
21-25: LGTM! Proper type consolidation.The
TMentionHandlertype correctly consolidates mention handling by merging therenderComponentproperty from the removedTReadOnlyMentionHandler, maintaining functionality while simplifying the type system.apps/web/core/components/editor/sticky-editor/editor.tsx (2)
17-20: LGTM! Proper interface design.Omitting the
editableproperty from the inherited props ensures the sticky editor's editability is controlled internally rather than externally, which is appropriate for this component's purpose.
73-73: LGTM! Consistent editable behavior.Explicitly setting
editable={true}ensures the sticky editor is always editable, which aligns with its intended functionality as an input component.packages/editor/src/core/extensions/utility.ts (1)
13-13: LGTM! Type consolidation aligns with the unified editor approach.The removal of
TReadOnlyFileHandlerfrom imports and the simplification of thefileHandlerprop type are consistent with the PR's objective to consolidate read-only and editable editor functionality.Also applies to: 41-41
packages/editor/src/core/types/config.ts (1)
4-20: LGTM! Clean type consolidation.The unified
TFileHandlertype successfully merges all properties from the previous split types while maintaining type safety and clarity. This simplification aligns well with the removal of separate read-only editor components.apps/web/core/components/editor/lite-text/editor.tsx (2)
35-43: Excellent use of discriminated union for type safety!The discriminated union type ensures that
uploadFileis only required when the editor is editable, preventing runtime errors and improving developer experience.
103-103: Good defensive programming with the no-op upload handler.Providing an async no-op function when
editableis false prevents potential runtime errors in the file handler logic.packages/editor/src/core/hooks/use-editor.ts (1)
120-120: Clean refactoring! Good separation of concerns.Delegating the imperative handle logic to
getEditorRefHelpersreduces code complexity in this hook and centralizes the editor API implementation. This improves maintainability without changing functionality.apps/web/core/hooks/editor/use-editor-config.ts (1)
26-91: Excellent consolidation of file handler logic!The unified
getEditorFileHandlerssuccessfully merges all functionality from the previous split handlers while maintaining the same behavior. The implementation correctly handles both legacy HTTP paths and new asset paths.apps/space/core/components/editor/lite-text-editor.tsx (3)
3-3: Good use of type importsUsing
typeimports for type-only imports is a TypeScript best practice that improves tree-shaking and code clarity.
12-28: Excellent type safety improvement with discriminated unionThe discriminated union type correctly enforces that
uploadFileis only required wheneditableis true. This prevents potential runtime errors and makes the component's API contract clearer.
55-60: Empty-string no-op matches TFileHandler.upload signatureThe
TFileHandler.uploadmethod is defined as(blockId: string, file: File) => Promise<string>. Returning an empty string viaasync () => ""still satisfies the requiredPromise<string>return type. No changes are needed here.packages/editor/src/core/types/editor.ts (2)
84-113: Well-designed API consolidationThe unified
EditorRefApiinterface successfully consolidates read-only and editable functionality into a single, cohesive API. The new methods for blur, real-time updates, command execution, and cursor position enhance the editor's capabilities without breaking existing functionality.
122-122: Good design decision for explicit editabilityMaking
editablea required property at the base interface level ensures explicit declaration of editor state and prevents ambiguity. This aligns well with the unified editor architecture.packages/editor/src/core/helpers/editor-ref.ts (3)
3-16: Appropriate imports for enhanced functionalityThe new imports properly support the expanded editor capabilities, including DOM serialization for text selection, menu command execution, and content manipulation helpers.
120-167: Excellent event subscription patternThe event subscription methods follow a consistent pattern with proper cleanup functions, preventing memory leaks and adhering to React best practices. The null checks and unsubscribe returns ensure robust event handling.
58-58: Add null check for editor in blur methodThe
blurmethod should check if the editor exists before calling commands to prevent potential runtime errors.- blur: () => editor?.commands.blur(), + blur: () => { + if (!editor) return; + editor.commands.blur(); + },Likely an incorrect or invalid review comment.
* refactor: remove lite text read only editor * chore: update types
4 participants
Description
This PR gets rid of the lite text read only editor.
Type of Change
Summary by CodeRabbit
New Features
editableprop to control editability, allowing both editable and read-only modes within the same component.Refactor
Bug Fixes
Chores