fix: bubble menu weird flickering #6591
Conversation
WalkthroughThe changes update multiple editor components to simplify state management. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor
participant BubbleMenu as EditorBubbleMenu
participant Align as TextAlignmentSelector
participant Color as BubbleMenuColorSelector
User->>Editor: Interact / Type text
Editor->>BubbleMenu: Update state via useEditorState
BubbleMenu->>Align: Pass updated editorState
BubbleMenu->>Color: Pass updated editorState
Align->>Editor: Check alignment state
Color->>Editor: Check color state
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Palanikannan1437
force-pushed
the
fix/bubble-menu
branch
from
83dd8ec to
dad85d8
Compare
Palanikannan1437
changed the title
Palanikannan1437
force-pushed
the
fix/bubble-menu
branch
from
dad85d8 to
d55c79d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
web/core/constants/editor.ts (1)
96-129:⚠️ Potential issueFix type signature inconsistency.
The type signature still uses "strikethrough" while the
itemKeyhas been updated to "strike". This inconsistency should be fixed.Apply this diff to update the type signature:
-const BASIC_MARK_ITEMS: ToolbarMenuItem<"bold" | "italic" | "underline" | "strikethrough">[] = [ +const BASIC_MARK_ITEMS: ToolbarMenuItem<"bold" | "italic" | "underline" | "strike">[] = [
🧹 Nitpick comments (4)
packages/editor/src/core/components/menus/bubble-menu/root.tsx (3)
7-7: Consider grouping imports for clarity.
Since you're already importing multiple menu items, consider grouping menu item imports together for improved readability.
19-20: Extending or clarifying constants.
COLORS_LISTis a convenient approach for color references. Consider adding a short comment describing its structure to aid maintainability.
104-104: Transition duration clarity.
Defining[300, 0]for the show/hide duration is fine, but consider documenting the intention or referencing design guidelines to avoid “magic numbers.”packages/editor/src/core/components/menus/menu-items.ts (1)
145-151: Consider renaming the function for consistency.While the type parameter and key have been updated to use "strike", the function name
StrikeThroughItemstill uses the old terminology. Consider renaming it toStrikeItemfor consistency.Apply this diff to rename the function:
-export const StrikeThroughItem = (editor: Editor): EditorMenuItem<"strike"> => ({ +export const StrikeItem = (editor: Editor): EditorMenuItem<"strike"> => ({Don't forget to update all references to this function in other files.
🛑 Comments failed to post (1)
packages/editor/src/core/types/editor.ts (1)
34-34: 💡 Verification agent
🧩 Analysis chain
Verify the impact of renaming the strikethrough command.
The change from
"strikethrough"to"strike"inTEditorCommandsis a breaking change that could affect dependent code.Run the following script to verify all usages of the strikethrough command:
🏁 Script executed:
#!/bin/bash # Description: Search for any remaining references to "strikethrough" in the codebase. # Search for "strikethrough" in TypeScript/JavaScript files rg -t ts -t js -i 'strikethrough' --glob '!{*.test.*,*.spec.*}'Length of output: 1345
Impact: Inconsistent Command Identifiers Detected
The change in
TEditorCommandsreplacing"strikethrough"with"strike"is a breaking change. Our search shows that several parts of the code—such as inweb/core/constants/editor.ts,packages/editor/src/core/constants/common.ts, and some UI components—still reference"strikethrough". These references will need to be updated to"strike"(or properly aliased) to ensure consistency and avoid type or runtime issues.
- Action Required: Update dependent code (constants, menu items, etc.) to use
"strike"in lieu of"strikethrough"to align with the updated public type.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/editor/src/core/components/menus/bubble-menu/root.tsx (2)
104-104: Consider adjusting tippy animation duration.The current duration setting
[300, 0]might cause the flickering issue. A shorter show duration could help.- duration: [300, 0], + duration: [150, 0],
144-147: Consider extracting className to a constant.The className string is quite long and could be reused. Consider extracting it to a constant or utility function.
+const BUBBLE_MENU_CLASS = "flex py-2 divide-x divide-custom-border-200 rounded-lg border border-custom-border-200 bg-custom-background-100 shadow-custom-shadow-rg"; <div ref={menuRef} - className="flex py-2 divide-x divide-custom-border-200 rounded-lg border border-custom-border-200 bg-custom-background-100 shadow-custom-shadow-rg" + className={BUBBLE_MENU_CLASS} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/editor/src/core/components/menus/bubble-menu/alignment-selector.tsx(4 hunks)packages/editor/src/core/components/menus/bubble-menu/color-selector.tsx(1 hunks)packages/editor/src/core/components/menus/bubble-menu/root.tsx(6 hunks)packages/editor/src/core/components/menus/menu-items.ts(1 hunks)packages/editor/src/core/constants/common.ts(2 hunks)packages/editor/src/core/types/editor.ts(1 hunks)web/core/constants/editor.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/editor/src/core/types/editor.ts
- web/core/constants/editor.ts
- packages/editor/src/core/components/menus/menu-items.ts
- packages/editor/src/core/components/menus/bubble-menu/alignment-selector.tsx
- packages/editor/src/core/components/menus/bubble-menu/color-selector.tsx
- packages/editor/src/core/constants/common.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/editor/src/core/components/menus/bubble-menu/root.tsx (4)
1-2: LGTM! Well-structured state management.The new
EditorStateTypeinterface provides a comprehensive and type-safe way to manage editor state, and the imports are properly organized.Also applies to: 28-46
48-78: LGTM! Improved state management with useEditorState hook.The refactoring to use
useEditorStatehook and consolidating formatting items into an object improves code organization and maintainability.
113-139: LGTM! Improved event handling.The addition of the menu reference check in
handleMouseDownand proper event cleanup in theuseEffecthook improves the robustness of the menu behavior.
159-185: LGTM! Clean conditional rendering.The conditional rendering based on
editorState.codeand proper prop passing to child components is well implemented.
Description
Bubble menu's weird flickering bug is now solved.
Type of Change
Test Scenarios
Summary by CodeRabbit
Refactor
Chores