chore: adjust open order #1169
chore: adjust open order #1169
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
Summary of ChangesHello @zombieJ, 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 refactors the Highlights
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
|
综述该变更修改了 变更
序列图由于此变更主要涉及函数签名和参数传递的重构，不涉及控制流的实质性改变，因此不生成序列图。 预估代码审查工作量🎯 2 (简单) | ⏱️ ~12 分钟 变更范围聚焦在两个相关文件，改动遵循一致的模式（参数从布尔值改为配置对象），逻辑更新直观清晰，无复杂分支判断或多个独立的设计决策。 诗歌
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1169 +/- ##
=======================================
Coverage 99.41% 99.41%
=======================================
Files 31 31
Lines 1192 1194 +2
Branches 422 424 +2
=======================================
+ Hits 1185 1187 +2
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 modifies the triggerOpen function in src/hooks/useOpen.ts to accept a configuration object with ignoreNext and lazy options. This change allows for more fine-grained control over when the dropdown is opened or closed, specifically adding a delay for blur events and preventing immediate closure when clicking within the popup. The BaseSelect component in src/BaseSelect/index.tsx is updated to utilize these new options when calling triggerOpen in onInternalBlur and onInternalMouseDown.
| export type TriggerOpenType = ( | ||
| nextOpen?: boolean, | ||
| config?: { | ||
| ignoreNext?: boolean; | ||
| lazy?: boolean; | ||
| }, | ||
| ) => void; |
There was a problem hiding this comment.
The type definition for TriggerOpenType has been updated to include a config object with ignoreNext and lazy properties. Ensure that all usages of TriggerOpenType are updated to reflect this change.
| if (!disabled) { | ||
| triggerOpen(false); | ||
| triggerOpen(false, { | ||
| lazy: true, | ||
| }); |
There was a problem hiding this comment.
The triggerOpen function is now called with a lazy option set to true. This introduces a delay in closing the dropdown on blur. Ensure this delay does not negatively impact the user experience, especially in scenarios where immediate feedback is expected.
| // Tell `open` not to close since it's safe in the popup | ||
| triggerOpen(true, true); | ||
| triggerOpen(true, { | ||
| ignoreNext: true, | ||
| }); |
There was a problem hiding this comment.
The triggerOpen function is called with ignoreNext: true when clicking inside the popup. This prevents the dropdown from closing immediately. Verify that this behavior is consistent with the intended user experience and doesn't introduce unexpected side effects.
| const toggleOpen = useEvent<TriggerOpenType>((nextOpen, config = {}) => { | ||
| const { ignoreNext = false, lazy = false } = config; | ||
|
|
There was a problem hiding this comment.
The toggleOpen function now accepts a config object. The default values for ignoreNext and lazy are set to false. This is good for backward compatibility, but ensure that all call sites are reviewed to determine if these options should be explicitly set.
| if (nextOpenVal || !lazy) { | ||
| if (!taskLockRef.current) { | ||
| triggerEvent(nextOpenVal); | ||
|
|
||
| // Lock if needed | ||
| if (ignoreNext) { | ||
| taskLockRef.current = ignoreNext; | ||
|
|
||
| macroTask(() => { | ||
| taskLockRef.current = false; | ||
| }, 2); | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for opening the dropdown has been modified to include a check for the lazy option. If nextOpenVal is true or lazy is false, the dropdown is opened immediately (if not locked). This change affects the timing of dropdown opening. Consider adding a comment explaining the rationale behind this change.
| macroTask(() => { | ||
| if (id === taskIdRef.current && !taskLockRef.current) { | ||
| triggerEvent(false); | ||
| triggerEvent(nextOpenVal); |
There was a problem hiding this comment.
The logic for closing the dropdown is now executed within a macroTask. This introduces a delay in closing the dropdown. Ensure that this delay does not negatively impact the user experience, especially in scenarios where immediate feedback is expected.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/hooks/useOpen.ts (1)
79-93: 建议添加注释说明延迟逻辑。第 79 行的条件
if (nextOpenVal || !lazy)实现了正确的行为：
- 打开操作 (
nextOpenVal=true) 始终立即触发- 关闭操作 +
lazy=false立即触发- 关闭操作 +
lazy=true延迟到宏任务执行这与 PR 目标一致，但建议添加注释解释此逻辑，以提高代码可读性。
应用此差异添加说明性注释：
+ // Trigger immediately when opening or when not lazy + // Lazy mode only defers closing operations if (nextOpenVal || !lazy) { if (!taskLockRef.current) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/BaseSelect/index.tsx(2 hunks)src/hooks/useOpen.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/hooks/useOpen.ts (1)
tests/utils/common.ts (1)
toggleOpen(22-31)
🔇 Additional comments (4)
src/hooks/useOpen.ts (2)
25-31: 类型定义清晰，向后兼容性良好。新的配置对象方式比原来的布尔参数更具扩展性，且通过可选参数保持了向后兼容。
70-100: 所有调用点已正确更新，向后兼容性得到维护。验证完成。代码库中所有
toggleOpen调用都与新签名兼容：
- 单参数调用 (
toggleOpen(true),toggleOpen(false),toggleOpen()) 通过默认参数config = {}正确处理- 配置对象调用 (
toggleOpen(false, { lazy: true }),toggleOpen(true, { ignoreNext: true })) 正确传递配置- 在 src/BaseSelect/index.tsx、src/SelectInput/index.tsx、src/OptionList.tsx 等文件的所有 17+ 个调用点中未发现不兼容的用法
实现逻辑正确，
ignoreNext和lazy参数的默认值确保了向后兼容性，同时支持新的配置对象语法。src/BaseSelect/index.tsx (2)
572-578: blur 延迟关闭优化实现正确。使用
{ lazy: true }延迟 blur 触发的关闭操作，允许用户在失焦后点击下拉框而不会立即关闭，这正是 PR 目标所述的优化。
585-590: popup 内点击的防关闭逻辑正确。使用
{ ignoreNext: true }在点击 popup 内元素时设置锁，阻止延迟的 blur 关闭操作执行。与lazy选项配合良好，确保用户点击 popup 时不会意外关闭。
triggerOpen 只对 blur 造成关闭进行延迟操作，以优化相应逻辑。
Summary by CodeRabbit