-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[WIKI-576] fix: trail node #7527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughA new hook for managing editor extension states is introduced and integrated into both lite and rich text editors, replacing previous prop-based flagged extension handling. The editor container logic for inserting paragraphs is simplified. Additionally, slash command menu items now support an optional badge, with corresponding type updates. The Changes
Sequence Diagram(s)sequenceDiagram
participant EditorComponent
participant useEditorFlagging
participant EditorExtensionState
EditorComponent->>useEditorFlagging: Call with anchor/workspaceSlug
useEditorFlagging-->>EditorComponent: Returns { document, liteText, richText } extension states
EditorComponent->>EditorExtensionState: Passes flagged/disabled extensions to editor instance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Suggested labels
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 (1)
✅ Files skipped from review due to trivial 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). (3)
✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/space/ce/hooks/use-editor-flagging.ts (2)
22-22
: Consider removing or documenting the unused parameter.The
workspaceSlug
parameter is currently unused in the hook implementation. Either remove it if not needed, or add a comment explaining its future intended use.-export const useEditorFlagging = (workspaceSlug: string): TEditorFlaggingHookReturnType => ({ +export const useEditorFlagging = (_workspaceSlug: string): TEditorFlaggingHookReturnType => ({Or add a comment:
+/** + * @param workspaceSlug - Reserved for future workspace-specific extension configuration + */ export const useEditorFlagging = (workspaceSlug: string): TEditorFlaggingHookReturnType => ({
23-35
: Consider reducing code duplication for disabled extensions.The same disabled extensions are repeated across all editor types. Consider extracting to a constant for better maintainability.
+const COMMON_DISABLED_EXTENSIONS: TExtensions[] = ["ai", "collaboration-cursor"]; + export const useEditorFlagging = (workspaceSlug: string): TEditorFlaggingHookReturnType => ({ document: { - disabled: ["ai", "collaboration-cursor"], + disabled: COMMON_DISABLED_EXTENSIONS, flagged: [], }, liteText: { - disabled: ["ai", "collaboration-cursor"], + disabled: COMMON_DISABLED_EXTENSIONS, flagged: [], }, richText: { - disabled: ["ai", "collaboration-cursor"], + disabled: COMMON_DISABLED_EXTENSIONS, flagged: [], }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/space/ce/hooks/use-editor-flagging.ts
(1 hunks)apps/space/core/components/editor/lite-text-editor.tsx
(2 hunks)apps/space/core/components/editor/rich-text-editor.tsx
(3 hunks)packages/editor/src/core/components/editors/editor-container.tsx
(1 hunks)packages/editor/src/core/extensions/slash-commands/command-menu-item.tsx
(1 hunks)packages/editor/src/core/types/slash-commands-suggestion.ts
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the plane project codebase, the path alias `@/plane-web` resolves to the `ce` directory, making i...
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/space/core/components/editor/rich-text-editor.tsx
⏰ 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: Build and lint web apps
🔇 Additional comments (12)
packages/editor/src/core/types/slash-commands-suggestion.ts (2)
3-3
: LGTM: Proper use of type-only import.Converting to a type-only import is appropriate since
TEditorCommands
is only used for type annotation on line 13.
21-21
: LGTM: Well-designed optional property addition.The optional
badge
property allows for enhanced UI elements in slash command items while maintaining backward compatibility. TheReact.ReactNode
type is appropriate for flexible content rendering.packages/editor/src/core/extensions/slash-commands/command-menu-item.tsx (2)
35-35
: LGTM: Clean implementation of badge rendering.The badge is rendered appropriately after the title. Since the
badge
property is optional, React will handleundefined
values gracefully by not rendering anything when no badge is provided.
1-39
: Note: Inconsistency between PR objectives and provided changes.The PR objectives mention fixing a trailing node issue when clicking at the end of the editor, but the provided files only show changes related to adding badge support to slash command menu items. The AI summary also mentions editor extension states and container logic changes that aren't visible in these files.
Could you verify that all relevant files for the trailing node fix are included in this review? The current changes appear to be unrelated to the stated bug fix objective.
Likely an incorrect or invalid review comment.
apps/space/core/components/editor/rich-text-editor.tsx (3)
3-3
: Import path looks correct.The import path
ce/hooks/use-editor-flagging
correctly resolves based on the project's path alias structure.
30-32
: Proper integration of the new hook.The component correctly removes the external
flaggedExtensions
prop dependency and uses the centralizeduseEditorFlagging
hook. Theanchor
parameter is appropriately passed to the hook.
50-50
: Correct usage of hook-provided flagged extensions.The
richText.flagged
property from the hook is correctly passed to the editor component, maintaining the same functionality while centralizing extension management.apps/space/core/components/editor/lite-text-editor.tsx (3)
11-11
: Import path correctly resolves to the ce directory.Based on the project's path alias configuration,
@/plane-web/hooks/use-editor-flagging
correctly resolves to thece/hooks/
directory.
48-48
: Proper hook integration matches RichTextEditor pattern.The component correctly uses the centralized
useEditorFlagging
hook with theanchor
parameter, following the same pattern as the RichTextEditor component.
55-55
: Correct usage of liteText flagged extensions.The
liteText.flagged
property is correctly passed to the editor, maintaining consistency with the centralized extension management approach.packages/editor/src/core/components/editors/editor-container.tsx (2)
51-53
: Simplified last node retrieval improves readability.The direct access to
doc.lastChild
is cleaner and more efficient than the previous position-based approach.
57-63
: Logic improvement aligns with PR objective to fix trailing node issue.The updated condition correctly checks if the last node is a paragraph before deciding to insert a new one. This prevents unnecessary paragraph insertion when the last node is already a paragraph, which should fix the trailing node behavior when clicking at the end of the editor.
packages/editor/src/core/components/editors/editor-container.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/space/ce/hooks/use-editor-flagging.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/space/ce/hooks/use-editor-flagging.ts (1)
packages/editor/src/core/types/extensions.ts (1)
TExtensions
(1-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). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
- GitHub Check: Build and lint web apps
🔇 Additional comments (2)
apps/space/ce/hooks/use-editor-flagging.ts (2)
1-2
: LGTM!The import statement correctly imports the
TExtensions
type from the editor package, which is properly used throughout the file.
4-17
: Well-structured type definition.The type definition provides a clear and consistent structure for managing extension states across different editor types. The naming convention and organization are appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/types/src/page/core.ts (1)
11-11
: Consider using a more specific type for better type safety.The new
description
property uses the genericobject
type. While this provides flexibility, consider using a more specific type (e.g.,Record<string, any>
or a dedicated interface) for better type safety and developer experience.However, if this needs to match the flexibility of editor content structures, the current
object
type may be appropriate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/types/src/page/core.ts
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/types/src/page/core.ts (1)
packages/types/src/page/extended.ts (1)
TPageExtended
(1-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). (4)
- GitHub Check: CodeQL
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
- GitHub Check: Build and lint web apps
🔇 Additional comments (2)
packages/types/src/page/core.ts (2)
5-5
: LGTM!The
TPage
type definition structure is clean and well-organized.
24-24
: LGTM!The intersection with
TPageExtended
at the end of the type definition follows TypeScript best practices and provides good extensibility while maintaining type safety for the core properties.
* fix : trail node * remove flagged * refactor : add disable flagging * refactor:update disabled extension * refactor: additional disabled * refactor: update enum * chore: add description key to page response type * chore: update base page instance --------- Co-authored-by: Aaryan Khandelwal <[email protected]>
Description
This PR fix the issue of not adding a trailing node on click of the editor end.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Refactor
Enhancements