Skip to content

feat: add focusTrap prop to preview config#507

Merged
zombieJ merged 1 commit intoreact-component:masterfrom
aojunhao123:feat/focus-trap-prop
Apr 8, 2026
Merged

feat: add focusTrap prop to preview config#507
zombieJ merged 1 commit intoreact-component:masterfrom
aojunhao123:feat/focus-trap-prop

Conversation

@aojunhao123
Copy link
Copy Markdown
Contributor

@aojunhao123 aojunhao123 commented Apr 8, 2026

Summary

Add focusTrap option to InternalPreviewConfig (default true). When set to false, the preview will not trap focus via useLockFocus.

Problem

When a preview is rendered with open: true inline (e.g. in antd's semantic documentation demos), useLockFocus activates a global focusin listener that prevents all other elements on the page from receiving keyboard focus. This breaks keyboard accessibility for the entire page.

Solution

Add a focusTrap prop (consistent with rc-dialog and rc-drawer naming) to allow consumers to opt out of focus trapping:

<Image.PreviewGroup
  preview={{ open: true, focusTrap: false }}
/>

rc-dialog already guards focus lock with isFixedPos and focusTrap:

useLockFocus(visible && isFixedPos && focusTrap !== false, ...)

rc-drawer uses focusTrap prop:

const mergedFocusTrap = focusTrap ?? mask !== false;
useLockFocus(open && mergedFocusTrap, ...)

rc-image previously had no guard:

useLockFocus(open && portalRender, ...)

Changes

  • InternalPreviewConfig: add focusTrap?: boolean (default true)
  • Preview component: pass focusTrap condition to useLockFocus
  • Add test case for focusTrap: false

Test plan

  • Existing focus trap tests still pass
  • New test: focus is not trapped when focusTrap is false
  • Verified on antd docs page: keyboard navigation works after setting focusTrap: false on the semantic demo's always-open preview

Summary by CodeRabbit

发布说明

  • 新功能

    • 添加可配置的焦点锁定选项。用户现在可以选择启用或禁用预览打开时的焦点锁定行为,默认启用。
  • 测试

    • 添加测试用例以验证禁用焦点锁定时的行为。

Add `focusTrap` option to `InternalPreviewConfig` (default `true`).
When set to `false`, the preview will not trap focus via `useLockFocus`.

This aligns with rc-dialog and rc-drawer which both support controlling
focus trapping behavior. The main use case is rendering an always-open
preview inline (e.g. for documentation semantic demos), where the global
focus lock incorrectly prevents keyboard interaction with the rest of
the page.
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 8, 2026

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

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f615c006-d30e-4739-a248-25f49341ec1c

📥 Commits

Reviewing files that changed from the base of the PR and between f6ffd6b and 17d963c.

📒 Files selected for processing (2)
  • src/Preview/index.tsx
  • tests/preview.test.tsx

概览

此变更向 Preview 组件添加可选的 focusTrap 配置属性,允许在打开预览时禁用焦点捕获。修改了焦点锁定逻辑以根据该标志有条件地启用焦点限制,并添加了相应的测试用例验证禁用焦点捕获时的行为。

变更

内聚组件 / 文件 摘要
焦点捕获配置
src/Preview/index.tsx
InternalPreviewConfig 接口中添加可选的 focusTrap?: boolean 属性(默认为 true),更新焦点锁定条件逻辑以根据该标志有条件地应用焦点限制。
焦点行为测试
tests/preview.test.tsx
添加新的测试用例,验证当禁用 focusTrap 时焦点保持在包装器元素上,不被重定向回预览组件。

估计代码审查工作量

🎯 2 (Simple) | ⏱️ ~8 分钟

可能相关的 PR

建议的审查者

  • zombieJ
  • yoyo837

诗歌

🐰 焦点驾驭兔子来,
陷阱开关轻轻摆,
预览打开自在选,
束缚约束说拜拜,
测试验证显威能,✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题清晰准确地概括了此次变更的主要内容——为预览配置添加focusTrap属性。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.44%. Comparing base (8514cd1) to head (17d963c).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #507   +/-   ##
=======================================
  Coverage   99.44%   99.44%           
=======================================
  Files          17       17           
  Lines         538      541    +3     
  Branches      161      165    +4     
=======================================
+ Hits          535      538    +3     
  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
Copy Markdown
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

This pull request introduces a focusTrap property to the Preview component, allowing users to toggle focus trapping behavior, which defaults to true. The changes include updating the useLockFocus hook and adding a test case to verify that focus is not trapped when the property is false. Feedback suggests that for a consistent non-modal experience, other behaviors like the aria-modal attribute and scroll locking should also respect the focusTrap setting.

}, [open]);

useLockFocus(open && portalRender, () => wrapperRef.current);
useLockFocus(focusTrap && open && portalRender, () => wrapperRef.current);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

While this change correctly disables the focus lock via useLockFocus, the Preview component still maintains other modal-like behaviors that should ideally respect the focusTrap prop for a consistent non-modal experience:

  1. Accessibility: The aria-modal="true" attribute (line 450) should be set to false (or match focusTrap) to correctly inform assistive technologies that the rest of the page remains interactive when the preview is open.
  2. Scroll Locking: The scroll lock logic (line 368) should be bypassed when focusTrap is false, allowing users to scroll the page while the preview is open (especially useful for the "inline" documentation use case mentioned in the PR).

Since these lines are not part of the current diff hunks, they couldn't be included in a code suggestion, but they are important for the completeness of this feature.

@zombieJ zombieJ merged commit 51fbd23 into react-component:master Apr 8, 2026
5 of 6 checks passed
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.

2 participants