-
Notifications
You must be signed in to change notification settings - Fork 59
fix: #297 disabled option can be selected by pressing Enter key #298
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: master
Are you sure you want to change the base?
Conversation
Walkthrough在示例中新增一个被禁用的选项“Dark”;在核心组件中为选择逻辑增加禁用校验,遇到禁用项时直接返回,不再替换文本或触发回调。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant UI as Mentions 下拉
participant IM as InternalMentions.selectOption
participant ED as Editor/TextArea
participant CB as onSelect Callback
U->>UI: 点击某个选项
UI->>IM: selectOption(option)
alt 选项被禁用
IM-->>UI: return (不做任何操作)
note right of IM: 禁用项被忽略<br/>不替换文本、不触发回调
else 选项可用
IM->>ED: 替换提及文本
IM->>UI: 更新内部状态
IM->>CB: 触发 onSelect(option)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Summary of Changes
Hello @doublelam, 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 addresses a bug where disabled options within the Mentions component could still be selected by pressing the Enter key. The changes ensure that disabled options are correctly unselectable, improving the component's robustness and user experience.
Highlights
- Bug Fix: Prevents disabled options in the Mentions component from being selected when the Enter key is pressed.
- Component Logic: Introduced a check within the "selectOption" function to immediately return if the selected option is marked as disabled.
- Example Update: Added a new disabled option to the "basic.tsx" example file to demonstrate and test the fix.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request addresses an issue where disabled options could be selected using the Enter key. The fix is straightforward and effective, adding a check for the disabled
status in the selectOption
function. A corresponding example with a disabled option has been added to the documentation. The changes are correct and resolve the reported bug. I have added one comment suggesting a potential usability enhancement for future consideration.
const { value: mentionValue = '', disabled } = option; | ||
if (disabled) { | ||
return; | ||
} |
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 change correctly prevents the selection of a disabled option.
For an improved user experience, you might also consider preventing keyboard navigation to disabled options. Currently, a user can still highlight a disabled option using the arrow keys, and when they press Enter, nothing happens, which could be confusing.
A potential enhancement would be to update the onInternalKeyDown
handler to skip over disabled options when navigating with the UP or DOWN arrow keys. While this is outside the scope of the current changes, it's a related improvement that would make the component more intuitive.
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 (3)
src/Mentions.tsx (2)
334-341
: 键盘导航与回车应跳过禁用项,避免“高亮在禁用项 + 回车无反应”的卡顿体验当前上下键与回车均依据
activeIndex
操作mergedOptions
,当高亮项为禁用项时,回车直接被早返回吞掉,面板保持开启且无反馈。建议:
- 上下键跳过禁用项;
- 回车在高亮为禁用项时自动跳转到下一个可用项;若均为禁用项,则关闭面板(与“无匹配项”一致)。
可在不改变渲染列表(仍显示禁用项)的前提下,仅调整导航与选择逻辑:
@@ - if (which === KeyCode.UP || which === KeyCode.DOWN) { - // Control arrow function - const optionLen = mergedOptions.length; - const offset = which === KeyCode.UP ? -1 : 1; - const newActiveIndex = (activeIndex + offset + optionLen) % optionLen; - setActiveIndex(newActiveIndex); - event.preventDefault(); - } else if (which === KeyCode.ESC) { + if (which === KeyCode.UP || which === KeyCode.DOWN) { + // 控制上下键,跳过禁用项 + const optionLen = mergedOptions.length; + const hasEnabled = mergedOptions.some(opt => !opt?.disabled); + if (!hasEnabled) { + // 全禁用则保持现状 + event.preventDefault(); + return; + } + const offset = which === KeyCode.UP ? -1 : 1; + let newActiveIndex = activeIndex; + for (let i = 0; i < optionLen; i += 1) { + newActiveIndex = (newActiveIndex + offset + optionLen) % optionLen; + if (!mergedOptions[newActiveIndex]?.disabled) break; + } + setActiveIndex(newActiveIndex); + event.preventDefault(); + } else if (which === KeyCode.ESC) { stopMeasure(); } else if (which === KeyCode.ENTER) { // Measure hit event.preventDefault(); // loading skip if (silent) { return; } - if (!mergedOptions.length) { + if (!mergedOptions.length) { stopMeasure(); return; } - const option = mergedOptions[activeIndex]; - selectOption(option); + const enabledOptions = mergedOptions.filter(opt => !opt?.disabled); + if (!enabledOptions.length) { + // 全部禁用时与“无选项”一致处理 + stopMeasure(); + return; + } + let option = mergedOptions[activeIndex]; + if (option?.disabled) { + // 回车时若高亮为禁用项,选中第一个可用项并同步高亮 + option = enabledOptions[0]; + const idx = mergedOptions.findIndex(o => o === option); + if (idx >= 0) setActiveIndex(idx); + } + selectOption(option); }这样用户不会陷入“回车无任何效果”的状态,交互更连贯、可预期。
请在本地用键盘上下键与回车对比如下场景:
- 仅中间有禁用项;
- 所有候选均为禁用;
- 第一项为禁用、第二项可用;
确保高亮跳转与回车行为符合预期。Also applies to: 351-357
299-319
: 选择拦截虽正确,但建议补充单元测试覆盖关键路径建议新增用例覆盖:
- 高亮禁用项时按 Enter,不应触发
onSelect
、不应替换文本;- 上下键导航会跳过禁用项(若采纳上条建议);
- 候选全禁用时按 Enter 的行为(若采纳上条建议,期望与无候选一致关闭面板)。
我可以基于现有测试框架补充测试,是否需要我起一个跟进 PR?
docs/examples/basic.tsx (1)
34-38
: 示例加入禁用项“Dark”很有帮助,建议在说明中点明“不可通过回车选择”演示层面已覆盖禁用态。为便于用户理解新行为,建议在示例附近增加一句文案或注释,强调:禁用项会显示但不可通过键盘回车或点击选中,也不会触发
onSelect
。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/examples/basic.tsx
(1 hunks)src/Mentions.tsx
(1 hunks)
🔇 Additional comments (1)
src/Mentions.tsx (1)
300-303
: 在选择入口统一拦截 disabled,修复方向正确在
selectOption
增加对option.disabled
的早返回,能从根源阻止回车/点击选中禁用项,同时集中到单点,避免键盘和鼠标路径分叉处理,简洁且可维护。
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #298 +/- ##
==========================================
- Coverage 98.04% 97.67% -0.38%
==========================================
Files 8 8
Lines 256 258 +2
Branches 60 61 +1
==========================================
+ Hits 251 252 +1
- Misses 5 6 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit