fix: defaultOpen not working #1187
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
功能概览修改 变更详情
预估代码审查工作量🎯 2 (Simple) | ⏱️ ~12 分钟
可能相关的 PR
庆祝诗篇
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
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 resolves an issue where the 'defaultOpen' prop was not correctly initializing the open state of select components. The underlying 'useOpen' hook has been updated to properly consume and apply this prop, ensuring that components render with the expected initial open state. The change is validated with a new dedicated test case. 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
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1187 +/- ##
=======================================
Coverage 99.42% 99.42%
=======================================
Files 31 31
Lines 1215 1216 +1
Branches 432 433 +1
=======================================
+ Hits 1208 1209 +1
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
This pull request correctly fixes an issue where the defaultOpen prop was not working. The fix involves passing the defaultOpen prop to the useOpen hook, which then correctly initializes the component's open state. The implementation is clean and effective. I also appreciate the addition of a new test case to verify the functionality, which is great for preventing future regressions. I've added one comment regarding a minor type inconsistency for improved type safety and code clarity.
| const [mergedOpen, triggerOpen] = useOpen( | ||
| defaultOpen || false, | ||
| open, | ||
| onPopupVisibleChange, | ||
| (nextOpen) => (disabled || emptyListContent ? false : nextOpen), | ||
| ); |
There was a problem hiding this comment.
The signature of the useOpen hook in src/hooks/useOpen.ts has a type inconsistency. The propOpen parameter is typed as boolean, but it receives the open prop from here, which is of type boolean | undefined. To ensure type safety and correctness, the hook's signature should be updated to propOpen?: boolean in src/hooks/useOpen.ts. This will accurately reflect that the open prop is optional.
![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/BaseSelect/index.tsx (1)
361-366: 正确实现了 defaultOpen 支持！将
defaultOpen作为第一个参数传递给useOpen钩子的修改是正确的，这使得初始打开状态能够正确设置。可选的小优化：
defaultOpen || false中的|| false可以省略，因为useControlledState应该能够正确处理undefined值。不过当前的写法更加明确，也完全可以接受。可选的简化建议：
const [mergedOpen, triggerOpen] = useOpen( - defaultOpen || false, + defaultOpen, open, onPopupVisibleChange, (nextOpen) => (disabled || emptyListContent ? false : nextOpen), );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/BaseSelect/index.tsx(1 hunks)src/hooks/useOpen.ts(2 hunks)tests/Select.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/Select.test.tsx (1)
tests/utils/common.ts (1)
expectOpen(4-20)
src/BaseSelect/index.tsx (1)
src/hooks/useOpen.ts (1)
useOpen(41-99)
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
tests/Select.test.tsx (1)
69-77: 测试用例正确验证了 defaultOpen 功能！该测试用例简洁明了，正确验证了
defaultOpen属性使下拉菜单初始时处于打开状态的功能。测试逻辑清晰，符合项目的测试模式。src/hooks/useOpen.ts (1)
42-42: 正确实现了 defaultOpen 参数支持！函数签名更新和状态初始化的修改都是正确的：
- Line 42：将
defaultOpen作为第一个参数添加到函数签名中，符合 React 约定（defaultValue在value之前）- Line 54：将
defaultOpen传递给useControlledState，使得在非受控模式下能够使用defaultOpen初始化打开状态这个修改正确地解决了
defaultOpen不生效的问题，同时保持了 SSR 安全性和受控/非受控状态的逻辑。Also applies to: 54-54
https://codesandbox.io/p/sandbox/ji-ben-shi-yong-antd-6-1-1-forked-n83xdl?file=%2Fdemo.tsx%3A14%2C18
Summary by CodeRabbit
发布说明
新功能
defaultOpen属性，允许指定下拉菜单的初始打开状态。测试
defaultOpen功能的正确性。✏️ Tip: You can customize this high-level summary in your review settings.