fix: enable spacebar to be typed in opened mode #1204
Conversation
|
@claytonlin1110 is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
Summary of ChangesHello @claytonlin1110, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the keyboard interaction within the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Walkthrough此更改调整了 BaseSelect 组件中 Enter/Space 键的键盘处理：仅在下拉尚未打开（mergedOpen 为 false）时阻止默认提交，其他行为（按键触发展开）保持不变。 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 分钟 Possibly related PRs
Suggested reviewers
诗句
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
claytonlin1110
force-pushed
the
fix/spacebar-enable-opened
branch
from
a2df78d to
f366628
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1204 +/- ##
=======================================
Coverage 99.43% 99.43%
=======================================
Files 31 31
Lines 1230 1230
Branches 441 441
=======================================
Hits 1223 1223
Misses 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request aims to address an issue where the spacebar cannot be typed in an opened select box. However, the provided code changes in src/BaseSelect/index.tsx modify the behavior of the Enter key. The change to the Enter key's event.preventDefault() logic appears to introduce a regression: it now only prevents default form submission when the select dropdown is not already open. This means if a user presses Enter while the dropdown is open (e.g., to confirm a selection), the default browser behavior, such as form submission, might occur instead of the intended action within the select component. Please clarify if the intention was to modify the Enter key or if the spacebar issue needs to be addressed in a different part of the codebase, and consider reverting the Enter key change to maintain existing functionality.
| const isSpaceKey = key === ' '; | ||
|
|
||
| // Enter or Space opens dropdown (ARIA combobox: spacebar should open) | ||
| if (isEnterKey || isSpaceKey) { | ||
| // Do not submit form when type in the input; prevent Space from scrolling page | ||
| if (mode !== 'combobox') { | ||
| event.preventDefault(); | ||
| } | ||
|
|
||
| if (isEnterKey || isSpaceKey) { | ||
| // We only manage open state here, close logic should handle by list component |
There was a problem hiding this comment.
The event.preventDefault() call for the Enter key has been moved inside the if (!mergedOpen) block. This means that when the select dropdown is already open (mergedOpen is true), pressing the Enter key will no longer prevent the default browser behavior (e.g., form submission) if mode !== 'combobox'. This is a regression, as Enter key typically confirms selection and prevents form submission when the dropdown is active. The original placement ensured event.preventDefault() was always called for Enter key in non-combobox modes.
// Do not submit form when type in the input
if (mode !== 'combobox') {
event.preventDefault();
}
// We only manage open state here, close logic should handle by list component
if (!mergedOpen) {
triggerOpen(true);
}
|
@afc163 Please review |
|
感觉移动 event.preventDefault() 的位置影响有点大 |
|
I think moving the position of event.preventDefault() (so it runs only when !mergedOpen) is the right fix. PR #1203 is feasible for typing space in the input but not sufficient for space-to-open without scroll; keeping the “by dropdown state” condition is the better approach. |
2 participants
🔗 Related Issues
#56939
#56943
Background & Solution
Background
Due to this PR, space bar is not typed in opened select box.
Solution
Open select box by space only when it's not opened yet so that user can still type a space in opened box.
Summary by CodeRabbit