-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix for 'Enter' key double submission issue #6356
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
Fix for 'Enter' key double submission issue #6356
Conversation
WalkthroughModified the Modal component's ENTER key handler to detect when the event target is a submit button and skip calling Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-app/src/components/Modal/index.js (1)
88-90: ENTER guard on focused submit button correctly prevents double-submitThe new
isSubmitButtonguard does exactly what the PR describes: when the Save/Create button (typesubmit) is focused, the document-level ENTER handler no longer callshandleConfirm, so only the native button click path fires once. Existing behavior for ENTER in other focused elements is preserved, and the implementation follows the existing style and coding guidelines.If you want to harden this further (optional), you could also skip when the event has already been handled elsewhere:
case ENTER_KEY_CODE: { // Skip if a submit button is focused - let native button click handle it to avoid double-fire const isSubmitButton = event.target?.type === 'submit'; if (!event.defaultPrevented && !shiftKey && !ctrlKey && !altKey && !metaKey && handleConfirm && !isSubmitButton) { return handleConfirm(); } }But as-is, this change cleanly fixes the reported duplicate submission issue without regressions in the Modal’s ENTER behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-app/src/components/Modal/index.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/Modal/index.js
⏰ 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). (6)
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
fixes: #6355
Description
Fix:
When users navigate to a modal's Save/Create button using Tab and press Enter, the form action was being triggered twice.
This caused:
Root Cause:
The Modal component has a document-level keydown listener that calls handleConfirm() on Enter. When a type="submit" button is focused, pressing Enter also triggers the button's native click event, resulting in handleConfirm() being called twice.
Fix:
Skip the document-level Enter handler when a type="submit" button is focused, allowing the native button click to handle submission (single execution).
Very small code change:
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Recording
No longer causing duplicate environment creation:
fix-dupe-enter-bug.mp4