fix: Prevent auto-scrolling when input is not focused #307
Conversation
|
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此 PR 在 MentionsContext 中新增并传递 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分钟
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 @ug-hero, 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 user experience issue in the mentions component where the dropdown menu would automatically scroll its selected item into view even when the user was not actively interacting with the input field. The fix ensures that the auto-scrolling behavior is now conditional on the input's focus state, leading to a more predictable and less intrusive user interface by preventing unwanted visual shifts. 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
|
![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 aims to prevent auto-scrolling in the mentions dropdown when the input is not focused. The current implementation has a high chance of causing a runtime error because it tries to access a nativeElement property on a ref object where it doesn't exist. I've provided a series of comments with code suggestions to refactor this using the existing isFocus state from the Mentions component, which is a more robust and safer approach. This will achieve the desired functionality correctly.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/MentionsContext.ts (1)
4-15:TextAreaRefimport should useimport typeandtextareaReftype should match actualuseRefreturn typeTwo type issues need correction:
Use type-only import for
TextAreaRef-import { TextAreaRef } from '@rc-component/textarea'; +import type { TextAreaRef } from '@rc-component/textarea';
TextAreaRefis a type interface, not a runtime value. Using a value import can generate bundling warnings or runtime errors in strict configurations.
textareaReftype declaration mismatches actualuseRefbehaviorIn
Mentions.tsx,useRef<TextAreaRef>(null)returnsReact.RefObject<TextAreaRef>with a readonly, nullablecurrentproperty. The current declaration asMutableRefObject<TextAreaRef>is incompatible and doesn't reflect the actual nullablecurrent.export interface MentionsContextProps { // ... - textareaRef: React.MutableRefObject<TextAreaRef>; + textareaRef: React.RefObject<TextAreaRef>; }This matches the actual ref type from
useRefand correctly signals to context consumers thatcurrentis readonly and may benull.
🧹 Nitpick comments (1)
tests/DropdownMenu.spec.tsx (1)
2-3: 测试用例基本覆盖了新行为，可再做少量健壮性增强这一组改动整体很不错，验证了：
- 文本框聚焦 + 输入触发下拉后，键盘
ArrowDown能正确触发scrollIntoView；- 文本框失焦后，通过菜单
mouseEnter改变activeIndex不会再触发scrollIntoView，避免页面被强制滚动到底部。有几处可以考虑顺手增强一下测试的健壮性（均为非必须建议）：
ResizeObservermock 建议恢复原值
beforeAll里直接覆盖了global.ResizeObserver，但没有在afterAll恢复，后续其他测试文件如果依赖真实实现，可能受影响。可以存一份原始引用并在afterAll里还原，例如：let originResizeObserver: typeof ResizeObserver; beforeAll(() => {
- global.ResizeObserver = class ResizeObserver {
- originResizeObserver = (global as any).ResizeObserver;
- (global as any).ResizeObserver = class ResizeObserver {
observe() {}
unobserve() {}
disconnect() {}
};
});+afterAll(() => {
- (global as any).ResizeObserver = originResizeObserver;
+});
textarea 查询最好加非空保障
目前直接：
const textarea = container.querySelector('textarea'); textarea.focus();在 TypeScript 严格模式或未来 DOM 结构调整时容易埋雷，建议显式断言或使用 Testing Library 的查询 API，例如：
const textarea = container.querySelector('textarea') as HTMLTextAreaElement; expect(textarea).toBeTruthy();或：
const textarea = screen.getByRole('textbox') as HTMLTextAreaElement;ArrowDown 事件可显式携带 keyCode/which
目前：
fireEvent.keyDown(textarea, { key: 'ArrowDown', code: 'ArrowDown' });组件内部用的是
event.which与KeyCode.DOWN比较，为了减少对测试环境实现细节的依赖，可以显式补充：-fireEvent.keyDown(textarea, { key: 'ArrowDown', code: 'ArrowDown' }); +fireEvent.keyDown(textarea, { + key: 'ArrowDown', + code: 'ArrowDown', + keyCode: 40, + which: 40, +});这些都不影响当前 PR 的主要目标，只是让测试在类型检查和未来依赖升级时更稳一些。
Also applies to: 12-18, 24-27, 29-58, 60-94
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/DropdownMenu.tsx(3 hunks)src/Mentions.tsx(1 hunks)src/MentionsContext.ts(2 hunks)tests/DropdownMenu.spec.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/DropdownMenu.spec.tsx (1)
tests/util.ts (1)
simulateInput(3-24)
🔇 Additional comments (2)
src/Mentions.tsx (1)
523-533: 将textareaRef下放到 MentionsContext 的做法是合理的这里把
textareaRef一起塞进MentionsContext.Provider，让下游（比如DropdownMenu）能够感知当前 textarea 的真实焦点状态，用于决定是否触发滚动，和 PR 目标高度一致，且不会改变现有公开 API 行为。逻辑本身没问题。类型细节我在src/MentionsContext.ts里单独提了一点建议，可一并参考。src/DropdownMenu.tsx (1)
17-26: Verify TextAreaRef property access pattern against rc-textarea APIThe focus-based scroll guard logic is conceptually sound, but the implementation depends critically on how TextAreaRef is accessed. According to rc-textarea's type definitions, the ref type exposes a
resizableTextAreafield rather than anativeElementproperty. If the code checksdocument.activeElement !== textareaRef.current.nativeElement, this references an undefined property, breaking the guard entirely.Confirm:
- Whether the actual code uses
.nativeElement(does not exist on TextAreaRef) or.resizableTextArea?.textArea(the correct path)- If
.nativeElementis used, this is a critical compatibility issue that would make the guard ineffective and require correcting the property access pattern
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/DropdownMenu.tsx (1)
53-53: 依赖数组应使用isFocus而非textareaRef。为确保焦点状态变化时副作用能够重新运行，依赖数组应包含
isFocus而不是textareaRef。这与上述建议使用isFocus的修复方案一致。应用此差异更新依赖数组：
- }, [activeIndex, activeOption.key, opened, textareaRef]); + }, [activeIndex, activeOption.key, opened, isFocus]);
|
使用ref相比使用isFocus(state)可以避免不必要的重渲染，每次用户失焦/聚焦都会更新Context，导致所有消费该Context的组件（甚至整个树）重新渲染。 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #307 +/- ##
==========================================
- Coverage 98.51% 98.17% -0.35%
==========================================
Files 8 8
Lines 270 274 +4
Branches 66 69 +3
==========================================
+ Hits 266 269 +3
- Misses 4 5 +1 ☔ 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 (2)
tests/DropdownMenu.spec.tsx (2)
2-3: 测试初始化整体合理，但可以更安全地处理全局 ResizeObserver 与 mocks当前在
beforeAll里直接覆盖global.ResizeObserver，以及在afterEach里只做jest.clearAllMocks()，对本文件是够用的，但对其他测试文件会有一些全局副作用风险。建议：
- 在
beforeAll/afterAll中保存并恢复原始的ResizeObserver，避免污染其它用例运行环境。- 如果后续会有更多
jest.spyOn，可以考虑在afterEach中统一使用jest.restoreAllMocks()，这样即使某个用例中间抛错，也能保证原型方法被恢复，不需要每个用例里都手动mockRestore()。示例调整（类型细节可以按你们的 ts 配置调整）：
- beforeAll(() => { - global.ResizeObserver = class ResizeObserver { - observe() {} - unobserve() {} - disconnect() {} - }; - }); + let originResizeObserver: any; + beforeAll(() => { + originResizeObserver = (global as any).ResizeObserver; + (global as any).ResizeObserver = class ResizeObserver { + observe() {} + unobserve() {} + disconnect() {} + }; + }); + + afterAll(() => { + (global as any).ResizeObserver = originResizeObserver; + }); afterEach(() => { jest.useRealTimers(); - jest.clearAllMocks(); + jest.restoreAllMocks(); });这样可以让这个测试文件对全局环境更加“自清理”。
Also applies to: 12-18, 24-27
60-90: 建议加强第二个用例的前置条件断言，避免“菜单未打开时也能通过”这个用例很好地模拟了“输入后 blur + 鼠标移入菜单项”的场景，并断言
scrollIntoView不应被调用，逻辑上和本次修复的 bug 非常贴合。不过这里有个小空隙：
- 当前写法中：
如果将来某个改动导致菜单根本没渲染出来（const menuItems = document.querySelectorAll('.rc-mentions-menu-item'); if (menuItems.length > 1) { fireEvent.mouseEnter(menuItems[1]); }menuItems.length <= 1），mouseEnter分支不会执行，此时即使 DropdownMenu 在“菜单打开”状态下又误调用了scrollIntoView，该用例也依然可能通过。建议：
- 显式断言菜单项数量，确保测试在“菜单确实打开且存在多个选项”的前提下运行。
- 可以顺便将查询范围限定在
container，避免未来页面上存在多组 Mentions 时串场。示例修改：
- const menuItems = document.querySelectorAll('.rc-mentions-menu-item'); - if (menuItems.length > 1) { - fireEvent.mouseEnter(menuItems[1]); - } + const menuItems = container.querySelectorAll('.rc-mentions-menu-item'); + expect(menuItems.length).toBeGreaterThan(1); + fireEvent.mouseEnter(menuItems[1]);这样一来，如果菜单行为有任何回归（比如根本没打开），测试会明确失败，不会“假阳性”通过。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/DropdownMenu.spec.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/DropdownMenu.spec.tsx (1)
tests/util.ts (1)
simulateInput(3-24)
🔇 Additional comments (1)
tests/DropdownMenu.spec.tsx (1)
29-58: 键盘导航 + 聚焦场景的测试覆盖良好，能有效防止回归该用例在聚焦 textarea 后，通过
simulateInput(container, '@')打开下拉，再发出ArrowDown并跑完定时器，最终断言scrollIntoView被带着{ block: 'nearest', inline: 'nearest' }调用，和业务期望（仅在输入框已聚焦时允许自动滚动）对齐。整体 arrange‑act‑assert 结构清晰，能很好地覆盖“正常聚焦下滚动应发生”的正向路径。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tests/DropdownMenu.spec.tsx (4)
12-24: ResizeObserver 全局 mock 合理，注意与其他测试的交互在
beforeAll/afterAll中挂接全局ResizeObserver，可以稳定依赖它的布局逻辑，避免 JSDOM 环境差异导致测试抖动，这里做法合理。
唯一需要留意的是：如果其他测试文件也 mockResizeObserver，要确保它们同样在各自afterAll中恢复，避免相互污染；目前本文件内是对称恢复的，没有问题。如后续有更多测试依赖
ResizeObserver，可以考虑提取一个通用的测试 helper 统一管理此 mock。
26-33: Jest 计时器与 mock 清理基本到位，可精简重复恢复
beforeEach中启用 fake timers，afterEach中恢复 real timers 并调用jest.restoreAllMocks()，整体生命周期管理是健康的。当前每个用例内还手动调用了
scrollIntoViewMock.mockRestore()，在已有jest.restoreAllMocks()的情况下略显多余，可以考虑删掉这些手动 restore，让清理逻辑集中在afterEach，减少重复与潜在遗漏。请确认你的 Jest 版本下
jest.restoreAllMocks()能覆盖jest.spyOn(HTMLElement.prototype, 'scrollIntoView')的场景。
35-64: 聚焦场景测试覆盖到位，建议增加一点健壮性这个用例很好地模拟了：textarea 聚焦 → 输入
@打开菜单 → 键盘ArrowDown导航 → 期望scrollIntoView以{ block: 'nearest', inline: 'nearest' }被调用，和新的“仅在 textarea 聚焦时滚动”的逻辑吻合。有两点可以考虑（都属可选增强）：
const textarea = container.querySelector('textarea');后直接使用textarea.focus()/fireEvent.keyDown(textarea, ...)，在类型上和运行时都假设不为 null。可以在断言前加一行expect(textarea).not.toBeNull();并在 TS 层用as HTMLTextAreaElement断言，避免未来 JSX 结构调整导致空指针。- 如需进一步防止多次滚动，可选加一句
expect(scrollIntoViewMock).toHaveBeenCalledTimes(1);明确只触发一次。请确认在当前 RTL + React 版本下，将
jest.runAllTimers()放在await act(async () => { ... })里不会触发额外的act警告。
66-95: 失焦但菜单仍交互时不滚动的回归用例设计合理第二个用例很好地锁定了本 PR 的关键回归场景：
- 聚焦 textarea 并输入
@打开菜单；- 触发
blur，再对第二个菜单项触发mouseEnter；- 断言菜单项仍然存在（
length > 1），但scrollIntoView没有被调用，验证“未聚焦时走return提前退出”的分支。同样有两点可以考虑的小改进：
- 和上一个测试类似，为
textarea做一次非空断言 + 类型断言，提高可维护性。scrollIntoViewMock.mockRestore()在有jest.restoreAllMocks()的前提下可以删除，交给统一清理。整体上，这个测试非常贴合 issue 场景和新逻辑，能够有效防止回归。
建议在本地多跑几次带
--runInBand和默认模式的测试，确认在不同执行顺序下此用例不会出现 flakiness。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/DropdownMenu.spec.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/DropdownMenu.spec.tsx (1)
tests/util.ts (1)
simulateInput(3-24)
🔇 Additional comments (1)
tests/DropdownMenu.spec.tsx (1)
2-4: 导入更新方向正确使用
@testing-library/react的render/act/fireEvent并直接从../src引入Mentions，与当前测试栈和组件入口保持一致，看起来是合理收敛，不需要额外改动。请确认当前 Jest / RTL 配置下没有关于
act的额外警告。
|
ci 挂了 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tests/DropdownMenu.spec.tsx (4)
12-24: ResizeObserver mock 实现正确，可考虑抽到公共测试初始化这段在
beforeAll/afterAll里接管global.ResizeObserver的做法是安全的，也能保证测试结束后还原环境。不过如果其他测试文件也需要ResizeObserver的 stub，建议考虑统一放到测试环境初始化（例如 jest setup 文件或一个共享 util）里，避免多处重复实现、后续改动时容易不一致。
30-33: 全局jest.restoreAllMocks与每个用例里的mockRestore有些重复
afterEach里已经调用了jest.restoreAllMocks()，它会把scrollIntoView的 spy 一并还原，因此两处用例中额外的scrollIntoViewMock.mockRestore()实际上是冗余的。可以考虑只保留一种方式（更推荐保留afterEach里的全局恢复逻辑），让每个用例更简洁一些。Also applies to: 63-64, 102-103
35-64: 聚焦场景的滚动用例覆盖了核心行为，建议补一个非空断言以提升健壮性这个测试很好地模拟了“输入触发下拉 + 键盘
ArrowDown导致滚动”的完整链路，并精确断言了scrollIntoView的参数，能有效锁住回归行为。小建议：
textarea来自querySelector('textarea')，类型上可能为HTMLTextAreaElement | null，后续直接调用textarea.focus()/fireEvent.keyDown，如果渲染结构变动导致找不到节点，会在运行期抛 NPE。可以在获取后立刻加一行断言／非空断言，例如：const textarea = container.querySelector('textarea'); expect(textarea).not.toBeNull(); // 或者 const textarea = container.querySelector('textarea') as HTMLTextAreaElement;这样在渲染结构变化时，失败信息会更直接，也更符合 TypeScript 的类型预期。
66-103: blur 场景用例能覆盖“菜单仍开但不应滚动”的逻辑，但需确认与 fake timers +waitFor的配合是否稳定这个用例很好地还原了“下拉菜单已渲染、hover 第二项、输入框 blur 后不应触发滚动”的场景，并通过
expect(scrollIntoViewMock).not.toHaveBeenCalled()验证修复点，方向是对的。有两个实现层面的小点建议确认一下：
fake timers 与
waitFor的配合
整个describe使用了jest.useFakeTimers()，而waitFor内部依赖定时器轮询断言。不同版本的 Jest/React Testing Library 对 fake timers 的支持略有差异，有的会自动切换为 real timers，有的则需要你手动runAllTimers或在waitFor外包一层 real timers。建议在本地确认下：
- 如果不手动再跑一次定时器，
waitFor能否稳定通过而不出现超时？- 如有不稳定，可考虑：
- 在本用例前后暂时切回 real timers，或
- 用额外的
act(() => jest.runAllTimers())来驱动waitFor轮询。
blur对document.activeElement的影响
当前生产代码的逻辑是依据document.activeElement是否为 textarea 来决定是否调用scrollIntoView。这里通过fireEvent.blur(textarea)来模拟失焦，按多数环境实现会同步更新document.activeElement，但不同测试环境／JSDOM 版本的细节实现可能略有差别。建议在调试时确认一下用例中blur之后document.activeElement的值，确保与生产逻辑的判断条件一致。整体来说逻辑和覆盖面都不错，只是这两个点关系到用例是否在 CI 中稳定执行，建议再验证一下以避免隐性 flakiness。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/DropdownMenu.spec.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/DropdownMenu.spec.tsx (1)
tests/util.ts (1)
simulateInput(3-24)
🔇 Additional comments (1)
tests/DropdownMenu.spec.tsx (1)
2-3: 测试依赖导入合理，覆盖了后续用到的 API这里新增的
render/act/fireEvent/waitFor以及直接渲染Mentions都与下方用例保持一致，用法和范围看起来是正确的，没有额外多余依赖。
|
覆盖率掉了。 |
这个覆盖率检测我应该如何触发呢？现在改了一下，不知道如何触发 |
2 participants
close ant-design/ant-design#56134
Summary by CodeRabbit
新功能
修复
测试
✏️ Tip: You can customize this high-level summary in your review settings.