refactor: update TextMenu and Variable nodes to ensure copy action wo… #285
Conversation
|
@patelajay745 is attempting to deploy a commit to the kmkoushik's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughChanges touch three files. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (4)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,md}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{tsx,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (3)
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. Comment |
|
@KMKoushik Can you please check ? |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.env.example(1 hunks)packages/email-editor/src/menus/TextMenu.tsx(12 hunks)packages/email-editor/src/nodes/variable.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
packages/email-editor/src/menus/TextMenu.tsxpackages/email-editor/src/nodes/variable.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript-first: use .ts/.tsx for source code (avoid JavaScript source files)
Use 2-space indentation and semicolons (Prettier 3 enforces these)
Adhere to @usesend/eslint-config; fix all ESLint warnings (CI fails on warnings)
Do not use dynamic imports; always place imports at the top of the module
Files:
packages/email-editor/src/menus/TextMenu.tsxpackages/email-editor/src/nodes/variable.tsx
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier 3 (run pnpm format)
Files:
packages/email-editor/src/menus/TextMenu.tsxpackages/email-editor/src/nodes/variable.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase (e.g., AppSideBar.tsx)
Files:
packages/email-editor/src/menus/TextMenu.tsxpackages/email-editor/src/nodes/variable.tsx
🧬 Code graph analysis (1)
packages/email-editor/src/menus/TextMenu.tsx (1)
packages/email-editor/src/editor.tsx (1)
Editor(74-116)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 25-25: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/email-editor/src/menus/TextMenu.tsx">
<violation number="1" location="packages/email-editor/src/menus/TextMenu.tsx:258">
Scheduling shouldShow through setTimeout immediately breaks the BubbleMenu. The timeout handler destructures {editor, state, from, to}, but setTimeout invokes it with no arguments, so it throws at runtime; even if it didn’t, lastShowResult is never updated, so shouldShow always returns false.</violation>
</file>
<file name="packages/email-editor/src/nodes/variable.tsx">
<violation number="1" location="packages/email-editor/src/nodes/variable.tsx:177">
Wiring onOpenChange directly to setIsEditing means the Radix trigger still opens the popover on the first click, so single-click copy is still blocked. Guard the handler so only close events update state.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
|
i don't think double click is a solution here, if multiple nodes are selected should just should not show the popup. this way we can apply the same logic to buttons / other nodes |
2 participants
#277 Solved this copying issue.
Now it will open fallback popup on double click.
Please check Video for reference.
Screen.Recording.2025-11-01.at.11.02.28.AM.mov
Summary by cubic
Refactored the email editor’s TextMenu and Variable node behavior to fix the copy/select issue so users can reliably select and copy text and variables. Addresses issue #277.
Written for commit 1f9274f. Summary will update automatically on new commits.
Summary by CodeRabbit
New Features
Improvements