Skip to content

fix: resolving first focus on elements is incorrect#735

Open
luozz1994 wants to merge 2 commits intoreact-component:masterfrom
luozz1994:fix-lock-focus
Open

fix: resolving first focus on elements is incorrect#735
luozz1994 wants to merge 2 commits intoreact-component:masterfrom
luozz1994:fix-lock-focus

Conversation

@luozz1994
Copy link

@luozz1994 luozz1994 commented Feb 13, 2026

问题点: Modal弹框首次focus 不正确,第二次才恢复正常行为

解决办法:在 lockFocus 时判断一下当前元素是否已聚焦,如果没有的话进行聚焦操作,然后再继续执行 syncFocus 方法

fix ant-design/ant-design#56963

Summary by CodeRabbit

发布说明

  • Bug修复
    • 改进了焦点锁定行为,使目标元素在需要时更快获得焦点,并在聚焦时阻止页面滚动,提升焦点管理体验。
  • 新增
    • 增强了焦点锁定的即时聚焦路径,同步聚焦至锁定元素或其首个可聚焦子项。
  • 测试
    • 添加了针对焦点锁定行为的单元测试,覆盖即时聚焦、已聚焦不重复聚焦和清理场景。

@vercel
Copy link

vercel bot commented Feb 13, 2026

Someone is attempting to deploy a commit to the React Component Team on Vercel.

A member of the Team first needs to authorize it.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @luozz1994, 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!

此拉取请求旨在解决 Modal 组件在首次打开时焦点定位不准确的问题。通过在 lockFocus 机制中引入一个前置检查,确保目标元素在焦点同步之前已被正确聚焦,从而提升了用户体验和可访问性,避免了用户需要二次交互才能使焦点正常工作的困扰。

Highlights

  • 解决 Modal 首次聚焦问题: 修复了 Modal 弹框首次聚焦不正确的问题,之前需要第二次操作才能正常聚焦。
  • 改进 lockFocus 逻辑: 在 lockFocus 函数中,增加了对当前元素是否已聚焦的判断。如果未聚焦,则会先进行聚焦操作,然后再执行 syncFocus 方法,确保焦点行为的即时性和准确性。

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • src/Dom/focus.ts
    • lockFocus 函数中,添加了检查元素是否已聚焦的逻辑。
    • 如果元素未聚焦,则调用 element.focus({ preventScroll: true }) 进行强制聚焦。
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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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

  1. 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.

@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

Walkthrough

在 lockFocus 注册 focusinkeydown 监听器后,新增代码会在元素尚未获得焦点时立即调用 focus(..., { preventScroll: true }) 强制聚焦,同步焦点到锁定元素;同时新增/更新了对应的单元测试覆盖这些行为。

Changes

Cohort / File(s) Summary
核心实现
src/Dom/focus.ts
lockFocus 中添加初始聚焦逻辑:注册监听器后若目标元素未获得焦点则调用 element.focus({ preventScroll: true }),并保留原有解锁/监听清理流程。
测试
tests/focus.test.tsx
新增对 lockFocus 的单元测试:覆盖当元素未聚焦时立即聚焦、元素已聚焦时不重复聚焦、聚焦到第一个可聚焦子元素、以及即时聚焦后的清理行为。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 分钟

Possibly related PRs

诗歌

🐰 我跳进聚焦的圈,
先抓住光,不等天边,
preventScroll 把浪潮放轻,
模态安稳梦乡甜,
小兔鼓掌,耳朵竖着看。

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题清晰明确地概括了主要改动:修复Modal首次聚焦行为不正确的问题,与代码变更(在lockFocus中添加初始聚焦逻辑)相符。
Linked Issues check ✅ Passed PR实现了#56963所述的修复需求:在lockFocus中添加元素聚焦判断和初始聚焦操作,使Modal首次打开时的聚焦行为正确。
Out of Scope Changes check ✅ Passed 所有变更都在#56963的解决范围内:lockFocus函数增强、相关单元测试以及导出调整,无超出范围的改动。
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into master

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
tests/focus.test.tsx (1)

105-168: 测试覆盖良好,验证了修复的关键场景。

三个测试用例分别覆盖了:未聚焦时立即调用 focus、已聚焦时跳过 focus、以及 syncFocus 将焦点移至首个可聚焦子元素。清理逻辑也到位(unlockremoveChildmockRestore)。

一个小建议:第三个测试(Line 150)中 wrapper 没有设置 tabIndex,这意味着 element.focus({ preventScroll: true }) 实际上不会使 wrapper 获得焦点(非可聚焦元素)。测试能通过是因为后续 syncFocus 将焦点移到了 input 子元素上。建议添加一行注释说明这个隐含的前置条件,以提高可读性。

💡 建议添加注释说明
     it('should focus element and then sync focus to first focusable child', () => {
       const wrapper = document.createElement('div');
+      // wrapper intentionally has no tabIndex, so focus() on it is a no-op;
+      // syncFocus should still move focus to the first focusable child.
       document.body.appendChild(wrapper);

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

你好,感谢你的贡献。这个修复通过在 lockFocus 时主动为元素设置焦点,解决了 Modal 弹窗首次焦点不正确的问题。这个改动确保了 syncFocus 在初始调用时能够正确记录最后一次聚焦的元素(lastFocusElement),从而使后续的焦点锁定逻辑能够正常工作。代码简洁且直接地解决了问题,注释也很清晰。做得很好!

// https://github.com/ant-design/ant-design/issues/56963
if (!hasFocus(element)) {
element.focus({ preventScroll: true });
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里会有点怪异,因为 syncFocus 就是用来聚焦的。如果这里先手工锁定了一下,那 snycFocus 还需要调用吗?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

另外要补一个 test case

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里一开始本来考虑的是在syncFocus内的else那个地方做一次focus的逻辑,不过考虑到如果写在那个函数内部的话,每次focusin事件触发都有可能执行到else导致事件重复,因此将其focus逻辑放在函数调用之前,让其更独立

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Modal 焦点的问题

2 participants

Comments