Skip to content

Conversation

@zombieJ
Copy link
Member

@zombieJ zombieJ commented Jan 4, 2026

Summary by CodeRabbit

发布说明

  • 新功能

    • 为对话框面板添加焦点捕获控制选项,支持在固定位置模式下灵活启用或禁用焦点锁定。
  • 测试

    • 添加测试覆盖以验证焦点捕获禁用时的行为。

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Jan 4, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
dialog Ready Ready Preview, Comment Jan 4, 2026 10:01am

@coderabbitai
Copy link

coderabbitai bot commented Jan 4, 2026

演进概览

本变更引入了一个新的 focusTrap 属性,允许在 Panel 组件中有选择地禁用焦点锁定。同时更新了焦点锁定的激活逻辑,现在需满足可见性、固定位置和焦点陷阱未禁用三个条件。

变更详情

一致性调整 / 文件 变更摘要
公共 API 扩展
src/Dialog/Content/Panel.tsx
在 PanelProps 中添加可选的 focusTrap?: boolean 属性;修改焦点锁定条件,现在仅当 visible、isFixedPos 和 focusTrap !== false 三者同时满足时才激活焦点锁定
接口定义更新
src/IDialogPropTypes.ts
在 IDialogPropTypes 接口中添加可选的 focusTrap?: boolean 属性
测试覆盖增强
tests/focus.spec.tsx
添加新测试用例"should not lock focus when focusTrap is false",验证当 focusTrap 为 false 时焦点锁定保持禁用状态

预计代码审查工作量

🎯 2 (简单) | ⏱️ ~10 分钟

可能相关的 PR

兔子之歌

🐰 焦点困顿,不再强制
陷阱开关,掌握在手
Panel 更聪慧,Panel 更温柔
三重条件,守护用户
自由选择,尽在掌握 ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Pull request title accurately summarizes the main feature addition: a new focusTrap property enabling control over focus-trapping behavior in the Dialog component.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch focusTrap

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.

@gemini-code-assist
Copy link

Summary of Changes

Hello @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 enhances the Dialog component by introducing a focusTrap prop, providing developers with explicit control over whether the dialog should trap keyboard focus. This change allows for greater flexibility in managing user interaction and accessibility, enabling the disabling of focus trapping when it's not desired, while maintaining the default behavior where focus trapping is active under specific conditions.

Highlights

  • New focusTrap Prop: A new boolean prop named focusTrap has been introduced to the Dialog component, allowing explicit control over its focus trapping behavior.
  • Conditional Focus Locking: The useLockFocus hook has been updated to respect the new focusTrap prop. Focus trapping will now only be active if the dialog is visible, fixed position, and focusTrap is not explicitly set to false.
  • Enhanced Test Coverage: A new test case has been added to tests/focus.spec.tsx to verify that focus trapping is correctly disabled when the focusTrap prop is set to false.

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

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.

Copy link

@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

This pull request introduces a focusTrap prop to allow disabling the focus lock feature in the dialog. The implementation is sound, and a corresponding test case has been added to verify the new behavior. I have one suggestion to improve the code's readability.

const mergedRef = useComposeRef(holderRef, panelRef, internalRef);

useLockFocus(visible && isFixedPos, () => internalRef.current);
useLockFocus(visible && isFixedPos && focusTrap !== false, () => internalRef.current);

Choose a reason for hiding this comment

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

medium

For better readability and to use a more modern JavaScript/TypeScript feature, you could use the nullish coalescing operator (??). The expression focusTrap ?? true is equivalent to focusTrap !== false for boolean and undefined values, and it more clearly conveys the intent of defaulting to true when focusTrap is not provided. Note that the parentheses are important due to operator precedence.

Suggested change
useLockFocus(visible && isFixedPos && focusTrap !== false, () => internalRef.current);
useLockFocus(visible && isFixedPos && (focusTrap ?? true), () => internalRef.current);

@codecov
Copy link

codecov bot commented Jan 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.42%. Comparing base (18678e8) to head (9843e71).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #542   +/-   ##
=======================================
  Coverage   98.42%   98.42%           
=======================================
  Files           8        8           
  Lines         191      191           
  Branches       68       68           
=======================================
  Hits          188      188           
  Misses          3        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/IDialogPropTypes.ts (1)

61-61: 建议添加 JSDoc 注释说明属性用途。

新增的 focusTrap 属性缺少文档说明。建议参考同文件中 focusTriggerAfterClose 的注释风格,添加 JSDoc 注释说明该属性的作用、默认值和使用场景,以提升 API 的可发现性和开发者体验。

🔎 建议的文档补充
+ // Whether to trap focus inside the dialog when it's open and fixed position
+ // Default: true
  focusTrap?: boolean;
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18678e8 and 9843e71.

📒 Files selected for processing (3)
  • src/Dialog/Content/Panel.tsx
  • src/IDialogPropTypes.ts
  • tests/focus.spec.tsx
🔇 Additional comments (3)
tests/focus.spec.tsx (1)

68-84: 测试用例实现正确!

新增的测试用例正确验证了 focusTrap={false} 的行为:即使在 position: fixed 的情况下,焦点锁定也应该被禁用。测试结构遵循了现有测试的模式,实现清晰准确。

src/Dialog/Content/Panel.tsx (2)

49-49: 属性解构实现正确!

focusTrap 属性已正确添加到解构列表中,并且通过 PanelProps extends IDialogPropTypes 自动继承了类型定义。


57-57: 焦点锁定逻辑实现优秀!

使用 focusTrap !== false 模式是一个很好的实现选择,它提供了合理的默认行为:

  • 未指定 focusTrap 时默认启用焦点锁定(向后兼容)
  • 显式设置 focusTrap={false} 时禁用焦点锁定
  • 显式设置 focusTrap={true} 时启用焦点锁定

这种实现方式保证了向后兼容性,现有未使用 focusTrap 属性的代码将继续按原有方式工作。

@aojunhao123
Copy link
Contributor

手动 +1 🙌

@zombieJ zombieJ merged commit c228726 into master Jan 5, 2026
12 checks passed
@zombieJ zombieJ deleted the focusTrap branch January 5, 2026 03:30
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.

3 participants