-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix: prevent prematurely triggered IME comment creation #8425
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
base: preview
Are you sure you want to change the base?
Conversation
check composition event
|
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. 📝 WalkthroughWalkthroughAdds IME composition checks to Enter-key handlers in comment input components to prevent form submission while an input method editor (IME) is composing. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
check composition event
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/components/comments/card/edit-form.tsx (1)
74-91: TheEnterKeyExtensionalso needs IME composition filtering.The
onKeyDownhandler on line 75 correctly checks!e.nativeEvent.isComposing, but theEnterKeyExtension(used byonEnterKeyPresson line 88) does not perform this check. SinceEnterKeyExtensiondoesn't pass the event object to the callback, it cannot be checked at the callback level. The extension itself must be updated to filter composition events before invokingonEnterKeyPress, similar to the DOM-level handler. Without this fix, IME composition could still trigger comment submission through the TipTap keyboard shortcut handler.
🧹 Nitpick comments (1)
apps/web/core/components/comments/card/edit-form.tsx (1)
75-75: Consider extracting the Enter key condition for readability.The Enter key condition has grown to six checks. For improved maintainability, consider extracting this logic:
🔎 Optional refactor suggestion
+ const shouldSubmitOnEnter = (e: React.KeyboardEvent, isEmpty: boolean): boolean => { + return ( + e.key === "Enter" && + !e.shiftKey && + !e.ctrlKey && + !e.metaKey && + !e.nativeEvent.isComposing && + !isEmpty + ); + }; <div onKeyDown={(e) => { - if (e.key === "Enter" && !e.shiftKey && !e.ctrlKey && !e.metaKey && !e.nativeEvent.isComposing && !isEmpty) handleSubmit(onEnter)(e); + if (shouldSubmitOnEnter(e, isEmpty)) handleSubmit(onEnter)(e); }} >
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/core/components/comments/card/edit-form.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/comments/card/edit-form.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/core/components/comments/card/edit-form.tsx
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
apps/web/core/components/comments/card/edit-form.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
apps/web/core/components/comments/card/edit-form.tsx
|
Hi @vamsikrishnamathala @prateekshourya29 , could you please help review this PR? |
Description
When using IME for multi-language input (e.g., typing Chinese followed by English), pressing Enter during composition incorrectly submitted the comment. This happened because the Enter key event did not properly check for native composition state.
Type of Change
Screenshots and Media (if applicable)
Pressing Enter during IME composition prematurely submitted the comment:
Expected Behavior (After Fix):
Test Scenarios
References
#7084
#7022
#7496
#5485
Note
Fixes premature comment submission when using IME by making Enter-to-submit respect composition state.
!e.nativeEvent.isComposingcheck to Enter key handlers incomment-create.tsxandcomments/card/edit-form.tsxWritten by Cursor Bugbot for commit a5ac382. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.