fix: automatic scroll to wrong element #301
Conversation
…erent item key although same value
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough为每个 Mentions 组件实例引入基于 React.useId() 的唯一 key（并允许通过可选 Changes
Sequence Diagram(s)（本次变更仅为组件内 key/id 生成与测试验证，未引入新的控制流或跨组件交互，故省略序列图。） Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 分钟
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that React keys for dropdown menu items are unique across different Mentions component instances, preventing potential key collision issues when multiple Mentions components use the same options array.
Key Changes
- Added a
generateUniqueKeyutility function that generates random alphanumeric strings - Modified the
getOptionscallback to append a unique key suffix to each option's key - Added a test case to verify that two Mentions components with shared options generate different keys
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/util.ts | Introduces generateUniqueKey utility function for generating random unique identifiers |
| src/Mentions.tsx | Updates key generation in getOptions to append unique suffixes, ensuring keys are unique across component instances |
| tests/FullProcess.spec.tsx | Adds test case validating unique key generation across multiple Mentions instances |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/util.ts (1)
137-139: 考虑使用更可靠的唯一标识符生成方式。当前使用
Math.random()生成唯一键存在以下潜在问题：
- 理论上存在碰撞风险，虽然概率较低
- 如果 React 19 可用，建议使用
React.useId()hook（需要在组件内部调用）- 或者使用单调递增的计数器来保证唯一性
如果项目使用 React 18+，可以考虑在
Mentions.tsx中使用useIdhook 替代：// 在 Mentions 组件内部 const menuUniqueKey = React.useId();或者使用计数器方案：
+let uniqueKeyCounter = 0; + export const generateUniqueKey = () => { - return Math.random().toString(36).substring(2, 11); + return `key-${++uniqueKeyCounter}`; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Mentions.tsx(3 hunks)src/util.ts(1 hunks)tests/FullProcess.spec.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/Mentions.tsx (1)
src/util.ts (1)
generateUniqueKey(137-139)
tests/FullProcess.spec.tsx (2)
docs/examples/basic.tsx (1)
Mentions(17-40)tests/util.ts (1)
simulateInput(3-24)
⏰ 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: Cleanup artifacts
🔇 Additional comments (2)
src/Mentions.tsx (2)
27-27: 导入语句正确。成功导入了
generateUniqueKey工具函数。
232-232: 键值拼接逻辑正确，但依赖于修复 Line 228 的问题。使用
${item?.key ?? item.value}-${menuUniqueKey}和${key || optionProps.value}-${menuUniqueKey}的拼接方式正确地为每个菜单项生成了唯一键。但是，由于 Line 228 的问题，这些键值会在每次渲染时变化，请先修复该问题。
Also applies to: 245-245
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Mentions.tsx(4 hunks)tests/Mentions.spec.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Mentions.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Mentions.spec.tsx (1)
tests/util.ts (1)
simulateInput(3-24)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #301 +/- ##
=======================================
Coverage 98.06% 98.06%
=======================================
Files 8 8
Lines 258 259 +1
Branches 62 61 -1
=======================================
+ Hits 253 254 +1
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/Mentions.spec.tsx (1)
380-404: 建议添加显式断言以提高测试清晰度。测试逻辑正确，成功验证了跨实例的菜单项 ID 唯一性。以下是可选的改进建议：
- 在检查唯一性之前，显式断言总项数：
const menuItemKeys = allMenuItems.map(item => item.getAttribute('data-menu-id'), ); + // Verify we have 6 items total (3 from each instance) + expect(menuItemKeys.length).toBe(6); + const uniqueMenuItemKeys = Array.from(new Set(menuItemKeys));
- 第 402 行的注释可以更清晰地说明测试目标：
- // As all input options items have different values, so there's no case that a Mentions instance has duplicated menu item keys. + // Verify that menu items from different instances have unique keys (no collision across instances) expect(uniqueMenuItemKeys.length).toBe(menuItemKeys.length);这些改进可以让测试意图更加明确，但当前实现已经足够验证核心功能。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Mentions.tsx(10 hunks)tests/Mentions.spec.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Mentions.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Mentions.spec.tsx (1)
tests/util.ts (1)
simulateInput(3-24)
🔇 Additional comments (1)
tests/Mentions.spec.tsx (1)
12-16: Now let me check the test case to verify if the mock is working correctly with the test logic:Now let me examine the actual test code to understand what it's testing:
验证 Jest mock 的ES6模块导出处理。
根据Jest官方文档，当使用jest.mock工厂参数处理ES6模块的默认导出时，需要指定__esModule: true属性。这个属性通常由Babel/TypeScript生成，但在这里需要手动设置。
目前的mock代码缺少__esModule标记：
jest.mock('@rc-component/util/lib/hooks/useId', () => { const { useId } = jest.requireActual('react'); return useId; // 直接返回函数，缺少__esModule });正确的做法应该是返回一个包含__esModule和default属性的对象：
jest.mock('@rc-component/util/lib/hooks/useId', () => ({ __esModule: true, default: jest.requireActual('react').useId }));请运行测试确认当前mock是否正常工作。若测试通过，说明你的Jest配置支持该语法；若失败，应按上述方式调整以符合ES6模块的标准mock模式。
2 participants
中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
Fixing bug related to automatic scroll to the wrong item issue (detail here)
💡 Background and Solution
The unexpected scrolling comes from this block of code which scrolls to the currently active item.
However, seems the way Menu component searches for the selected item across the entire document instead of only within the Menu container - as seen in this line of code. So in scenarios where there are two Menu items with the same key at the same time, it always finds and scrolls to the last one.
Updating menu to let it search only within the container may not be a solution, as I saw that there's already a react-component/menu#671 (comment) about that before
So I fixed the issue by generating a unique key for each Menu component. So the same value item across different Menu components always has a different key.
📝 Change Log
Summary by CodeRabbit
新功能
错误修复
测试