feat: add scrollLock prop to control body scroll lock#564
Conversation
|
Someone is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughDialog 组件新增 scrollLock 属性，允许开发者控制打开对话框时是否锁定 body 滚动。实现通过在 DialogWrap 中解构该属性并根据其值调整 Portal 的 autoLock 逻辑，配合类型定义、测试用例和文档说明完成。 ChangesscrollLock 功能开关
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/DialogWrap.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/IDialogPropTypes.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. tests/scroll.spec.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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 introduces a new scrollLock prop (defaulting to true) to control whether the body scroll is locked when the dialog opens. The feedback suggests extracting scrollLock from the props passed down to the underlying Dialog component to prevent potential React console warnings about unknown DOM attributes. Additionally, improvements to the new test cases are recommended, such as removing unused destructured variables, correcting test descriptions, and adding a test case to verify dynamic updates of the scrollLock prop.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/IDialogPropTypes.ts (1)
1-1:⚠️ Potential issue | 🟠 Major修复
@rc-component/util/lib/PortalWrapper导入以消除no-restricted-importsCI 标记
src/IDialogPropTypes.ts第 1 行的导入触发no-restricted-imports。@rc-component/util根入口index.d.ts已导出GetContainer（export type { GetContainer } from './PortalWrapper';），因此应将GetContainer的导入改为从包根导入以通过检查；仓库内仅此处使用GetContainer，影响面很小。该问题虽非本次 PR 引入，但建议合并前一并修复。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/IDialogPropTypes.ts` at line 1, 在 IDialogPropTypes.ts 中将对类型 GetContainer 的导入从受限子路径 '`@rc-component/util/lib/PortalWrapper`' 改为从包根 '`@rc-component/util`' 导入以通过 no-restricted-imports 检查；具体操作是替换当前 import type { GetContainer } from '`@rc-component/util/lib/PortalWrapper`' 为 import type { GetContainer } from '`@rc-component/util`'（确保只修改导入语句，保留类型引用不变）。
🧹 Nitpick comments (1)
tests/scroll.spec.tsx (1)
83-83: 💤 Low value可选：移除未使用的
rerender变量。两个新增的测试用例中都解构了
rerender变量但未使用。可以简化为仅解构unmount。♻️ 建议的清理
- const { unmount, rerender } = render(<Dialog visible scrollLock={false} />); + const { unmount } = render(<Dialog visible scrollLock={false} />);- const { unmount, rerender } = render(<Dialog visible scrollLock={true} />); + const { unmount } = render(<Dialog visible scrollLock={true} />);Also applies to: 94-94
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/scroll.spec.tsx` at line 83, Two tests destructure an unused rerender from the render result in tests/scroll.spec.tsx; remove the unused variable to clean up the code by changing occurrences where you do const { unmount, rerender } = render(<Dialog ... />) to only destructure const { unmount } = render(<Dialog ... />). Update both instances (the one around the scrollLock=false test and the other referenced at the second occurrence) so rerender is not declared if unused.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/IDialogPropTypes.ts`:
- Line 1: 在 IDialogPropTypes.ts 中将对类型 GetContainer 的导入从受限子路径
'`@rc-component/util/lib/PortalWrapper`' 改为从包根 '`@rc-component/util`' 导入以通过
no-restricted-imports 检查；具体操作是替换当前 import type { GetContainer } from
'`@rc-component/util/lib/PortalWrapper`' 为 import type { GetContainer } from
'`@rc-component/util`'（确保只修改导入语句，保留类型引用不变）。
---
Nitpick comments:
In `@tests/scroll.spec.tsx`:
- Line 83: Two tests destructure an unused rerender from the render result in
tests/scroll.spec.tsx; remove the unused variable to clean up the code by
changing occurrences where you do const { unmount, rerender } = render(<Dialog
... />) to only destructure const { unmount } = render(<Dialog ... />). Update
both instances (the one around the scrollLock=false test and the other
referenced at the second occurrence) so rerender is not declared if unused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ee979be0-5bb2-4314-b89e-666e8f962ea9
📒 Files selected for processing (4)
README.mdsrc/DialogWrap.tsxsrc/IDialogPropTypes.tstests/scroll.spec.tsx
EmilyyyLiu
force-pushed
the
feat/scroll-lock
branch
from
b3e47e4 to
d496c6d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #564 +/- ##
==========================================
+ Coverage 98.92% 98.93% +0.01%
==========================================
Files 8 8
Lines 186 188 +2
Branches 67 68 +1
==========================================
+ Hits 184 186 +2
Misses 2 2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Add scrollLock prop (default: true) to control whether to lock body scroll when dialog opens - Add test cases for scrollLock functionality - Update README.md with new prop documentation Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
EmilyyyLiu
force-pushed
the
feat/scroll-lock
branch
from
9c45c3c to
cb61b22
Compare
zombieJ
left a comment
zombieJ
left a comment
There was a problem hiding this comment.
LGTM 👍 实现简洁正确:scrollLock={false} 时 autoLock 短路,useScrollLocker 不会给 body 注入 overflow:hidden;默认 true 保持原行为,向后兼容。+1
3 participants
Summary
Add
scrollLockprop to control whether to lock body scroll when dialog opens.Background
Related issue: #32567
Users want to be able to interact with background elements when modal is open. Currently, the modal locks body
scroll by default.
Solution
Add
scrollLockprop (default: true) to control body scroll lock behavior.API
Changelog
│ Language │ Changelog │
│ 🇺🇸 English │ Add scrollLock prop to control whether to lock body scroll when dialog opens │
│ 🇨🇳 Chinese │ 新增 scrollLock 属性，用于控制对话框打开时是否锁定 body 滚动 │
Summary by CodeRabbit
发布说明
scrollLock配置项（默认启用），允许用户控制对话框打开时是否锁定页面滚动。