[WIKI-466] refactor: remove rich text read only editor #7241
Conversation
WalkthroughThis update removes all dedicated read-only rich text editor components and consolidates editable and read-only functionality into a single Changes
Sequence Diagram(s)sequenceDiagram
participant ParentComponent
participant RichTextEditor
participant EditorWrapper
participant useEditor
ParentComponent->>RichTextEditor: Render with editable={true|false}
RichTextEditor->>EditorWrapper: Pass editable prop
EditorWrapper->>useEditor: Pass editable prop
useEditor-->>EditorWrapper: Editor instance (editable state set)
EditorWrapper-->>RichTextEditor: Rendered editor
RichTextEditor-->>ParentComponent: Editor UI (editable or read-only)
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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: 0
🧹 Nitpick comments (1)
space/core/components/editor/rich-text-editor.tsx (1)
44-44: Consider extracting the no-op function to improve performance.The conditional logic correctly handles file uploads, but the no-op function
async () => ""is recreated on every render wheneditableisfalse.Extract the no-op function to a constant outside the component:
+const NO_OP_UPLOAD_HANDLER = async () => ""; export const RichTextEditor = forwardRef<EditorRefApi, RichTextEditorWrapperProps>((props, ref) => { // ... existing code ... fileHandler={getEditorFileHandlers({ anchor, - uploadFile: editable ? props.uploadFile : async () => "", + uploadFile: editable ? props.uploadFile : NO_OP_UPLOAD_HANDLER, workspaceId, })}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
packages/editor/src/core/components/editors/editor-wrapper.tsx(3 hunks)packages/editor/src/core/components/editors/lite-text/editor.tsx(1 hunks)packages/editor/src/core/components/editors/rich-text/index.ts(0 hunks)packages/editor/src/core/components/editors/rich-text/read-only-editor.tsx(0 hunks)packages/editor/src/core/types/editor.ts(1 hunks)packages/editor/src/index.ts(0 hunks)space/core/components/editor/index.ts(1 hunks)space/core/components/editor/rich-text-editor.tsx(2 hunks)space/core/components/editor/rich-text-read-only-editor.tsx(0 hunks)space/core/components/issues/peek-overview/issue-details.tsx(2 hunks)web/ce/components/pages/editor/ai/ask-pi-menu.tsx(2 hunks)web/ce/components/pages/editor/ai/menu.tsx(2 hunks)web/core/components/core/description-versions/modal.tsx(4 hunks)web/core/components/core/modals/gpt-assistant-popover.tsx(4 hunks)web/core/components/editor/rich-text-editor/index.ts(0 hunks)web/core/components/editor/rich-text-editor/rich-text-editor.tsx(3 hunks)web/core/components/editor/rich-text-editor/rich-text-read-only-editor.tsx(0 hunks)web/core/components/inbox/modals/create-modal/issue-description.tsx(1 hunks)web/core/components/issues/description-input.tsx(2 hunks)web/core/components/issues/issue-modal/components/description-editor.tsx(1 hunks)web/core/components/profile/activity/activity-list.tsx(2 hunks)web/core/components/profile/activity/profile-activity-list.tsx(2 hunks)
💤 Files with no reviewable changes (6)
- packages/editor/src/index.ts
- packages/editor/src/core/components/editors/rich-text/index.ts
- web/core/components/editor/rich-text-editor/index.ts
- web/core/components/editor/rich-text-editor/rich-text-read-only-editor.tsx
- space/core/components/editor/rich-text-read-only-editor.tsx
- packages/editor/src/core/components/editors/rich-text/read-only-editor.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (37)
packages/editor/src/core/types/editor.ts (1)
141-144: LGTM! Type definition correctly updated for unified editor component.The change from interface to type alias is appropriate and the new required
editableprop ensures explicit control over editor behavior. The optionaldragDropEnabledprop maintains backward compatibility.Note: The required
editableprop is a breaking change that requires all consumers to explicitly specify editability, which aligns well with the PR's goal of removing implicit read-only behavior.web/core/components/inbox/modals/create-modal/issue-description.tsx (1)
70-70: LGTM! Correctly added editable prop for issue creation context.The
editable={true}prop is appropriate since this component is used for creating and editing issue descriptions. This change aligns with the unified editor pattern and type requirements.web/ce/components/pages/editor/ai/ask-pi-menu.tsx (2)
7-7: LGTM! Import correctly updated for unified editor component.
41-42: LGTM! Component migration preserves read-only behavior for AI response display.The change from
RichTextReadOnlyEditortoRichTextEditorwitheditable={false}maintains the intended read-only behavior while aligning with the unified component approach.packages/editor/src/core/components/editors/lite-text/editor.tsx (1)
22-22: Verify if editable should be configurable for LiteTextEditor.The addition of
editableprop toEditorWrapperaligns with the unified editor approach. However, consider whetherLiteTextEditorshould accept aneditableprop parameter to make it configurable, or if it should always be editable by design.Based on the component name and usage pattern, hardcoding
editable={true}seems appropriate, but please verify this aligns with the intended API design.space/core/components/issues/peek-overview/issue-details.tsx (2)
3-3: LGTM! Import correctly updated for unified editor component.
28-29: LGTM! Component migration preserves read-only behavior for issue details display.The change from
RichTextReadOnlyEditortoRichTextEditorwitheditable={false}correctly maintains read-only behavior for the issue details peek overview while adopting the unified component pattern.web/core/components/issues/issue-modal/components/description-editor.tsx (1)
181-181: LGTM: Correct implementation of unified editor component.The addition of
editableprop withtruevalue is appropriate for this issue description editor, maintaining the expected editable behavior while using the unifiedRichTextEditorcomponent.space/core/components/editor/index.ts (1)
4-4: LGTM: Correct consolidation of editor exports.The export change from read-only editor to the unified rich text editor aligns with the refactoring objective to remove dedicated read-only components.
web/ce/components/pages/editor/ai/menu.tsx (2)
11-11: LGTM: Correct import update for unified editor component.The import change from
RichTextReadOnlyEditortoRichTextEditoraligns with the refactoring to use a single editor component.
211-215: LGTM: Proper implementation of read-only behavior.The component usage correctly maintains read-only behavior by setting
editable={false}for displaying AI-generated responses, which is the appropriate configuration for this use case.web/core/components/profile/activity/activity-list.tsx (2)
12-12: LGTM: Correct import update for unified editor component.The import change from the read-only editor to the unified
RichTextEditorcomponent aligns with the refactoring objective.
76-77: LGTM: Proper read-only configuration for activity comments.The component usage correctly maintains read-only behavior with
editable={false}for displaying activity comments, which is appropriate for historical activity data.packages/editor/src/core/components/editors/editor-wrapper.tsx (3)
15-15: LGTM: Proper TypeScript typing for editable prop.The addition of the required
editable: booleanprop to the Props type correctly supports the unified editor approach.
25-25: LGTM: Correct prop destructuring.The
editableprop is properly destructured from the component props, maintaining consistency with the existing code pattern.
44-44: LGTM: Proper propagation to useEditor hook.The
editableprop is correctly passed to theuseEditorhook, enabling the foundational control over editor editability that supports the entire refactoring effort.web/core/components/profile/activity/profile-activity-list.tsx (2)
10-10: LGTM - Import updated correctly for refactoring.The import change from
RichTextReadOnlyEditortoRichTextEditoraligns with the refactoring objective.
99-100: LGTM - Component usage correctly updated.The component usage correctly switches from
RichTextReadOnlyEditortoRichTextEditorwitheditable={false}to maintain read-only behavior.web/core/components/core/description-versions/modal.tsx (4)
5-5: LGTM - Type import unified correctly.The change from
EditorReadOnlyRefApitoEditorRefApiunifies the editor API types.
22-22: LGTM - Component import updated for refactoring.The import change aligns with the consolidation of editor components.
55-55: LGTM - Ref type updated correctly.The ref type change from
EditorReadOnlyRefApitoEditorRefApimaintains consistency with the unified API.
134-135: LGTM - Component usage correctly maintains read-only behavior.The switch to
RichTextEditorwitheditable={false}preserves the read-only functionality for version descriptions.web/core/components/core/modals/gpt-assistant-popover.tsx (5)
10-10: LGTM - Type import unified correctly.The API type consolidation is consistent with the refactoring.
14-14: LGTM - Component import updated for refactoring.The import change aligns with the editor component consolidation.
58-59: LGTM - Ref types updated consistently.Both editor refs are correctly updated to use the unified
EditorRefApitype.
220-221: LGTM - Prompt editor correctly updated.The prompt editor maintains read-only behavior with
editable={false}.
235-236: LGTM - Response editor correctly updated.The response editor maintains read-only behavior with
editable={false}.web/core/components/issues/description-input.tsx (2)
15-15: LGTM - Import updated for unified editor component.The import change supports the consolidation of editor components.
113-161: LGTM - Excellent simplification of conditional rendering.The refactoring eliminates the conditional logic between
RichTextEditorandRichTextReadOnlyEditor, using a single component witheditable={!disabled}. The addition ofsearchMentionCallbackanduploadFileprops correctly supports the editable mode requirements.web/core/components/editor/rich-text-editor/rich-text-editor.tsx (5)
16-32: Excellent type safety with discriminated union.The discriminated union type based on the
editableprop ensures that wheneditable=true, the required propssearchMentionCallbackanduploadFileare enforced at compile time. This prevents runtime errors and provides clear API contracts.
37-37: LGTM - Prop destructuring updated correctly.The
editableprop is properly extracted for use in the component logic.
50-50: LGTM - Conditional search entity function.The search entity function is correctly conditionally assigned based on the
editableprop, using a no-op function when not editable.
59-59: LGTM - Editable prop passed correctly.The
editableprop is properly passed to the underlyingRichTextEditorWithRefcomponent.
63-63: LGTM - Conditional upload file function.The upload file function is correctly conditionally assigned, using a no-op function when not editable to prevent file uploads in read-only mode.
space/core/components/editor/rich-text-editor.tsx (3)
12-26: Well-implemented discriminated union type for type safety.The discriminated union pattern ensures that when
editableistrue, theuploadFilefunction is required, and wheneditableisfalse, it's properly omitted. This provides excellent compile-time type safety and prevents runtime errors.
29-29: Proper destructuring of the new requirededitableprop.The explicit destructuring of the
editableprop is necessary and correctly implemented to support the new unified component interface.
41-41: Correct implementation of the editable prop passing.The
editableprop is properly passed down to the underlyingRichTextEditorWithRefcomponent, completing the unified editor interface.
Description
This PR gets rid of the rich text read only editor, resulting in-
Type of Change
Summary by CodeRabbit
New Features
editableprop for all rich text editor components, allowing precise control over editability for each instance.Refactor
RichTextEditorcomponent, controlled via theeditableprop.Chores
editableprop, ensuring consistent behavior for editable and read-only states.