-
-
Notifications
You must be signed in to change notification settings - Fork 201
feat: add className *-mask-blur #507
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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本次变更为 Dialog 新增 mask 可取字符串值 Changes
Sequence Diagram(s)sequenceDiagram
participant App as 调用方
participant Dialog as Dialog
participant Mask as Mask
participant DOM as DOM/CSS
App->>Dialog: 渲染 { mask: true | 'blur', visible }
Dialog->>Mask: 透传 mask
Mask->>DOM: 渲染遮罩并根据 mask 添加类名(-mask-blur)
DOM->>DOM: 应用 CSS backdrop-filter: blur(4px)(当类名存在)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @EmilyyyLiu, 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!
此拉取请求引入了一项新功能,允许对话框遮罩具有模糊效果。通过为mask属性添加新的blur选项并应用backdrop-filter: blur CSS属性来实现。
Highlights
- 新的mask属性选项: Dialog组件的mask属性现在除了接受boolean类型外,还接受'blur'值。
- 模糊效果的CSS: 添加了一个新的CSS类*-mask-blur,当mask属性设置为'blur'时,该类将为遮罩应用backdrop-filter: blur(4px)效果。
- 集成: Mask组件现在会检查mask === 'blur'条件,以有条件地应用新的模糊类。
- 文档更新: README.md已更新,以反映新的mask属性类型。
- 测试用例: 添加了一个新的测试用例Dialog mask blur来验证此功能。
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
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 issue 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
-
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. ↩
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #507 +/- ##
=======================================
Coverage 98.46% 98.46%
=======================================
Files 8 8
Lines 195 196 +1
Branches 68 69 +1
=======================================
+ Hits 192 193 +1
Misses 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
这个 PR 为对话框的遮罩层增加了一个很棒的 'blur' 选项。实现方式很直接,也包含了新的测试用例。我提出了一些建议,主要是为了提高模糊效果的浏览器兼容性,以及更新文档使其更清晰。
z-index: 1050; | ||
|
||
&&-blur { | ||
backdrop-filter: blur(4px); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this 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 (5)
README.md (1)
68-68
: 补充 mask='blur'
的语义描述,提升可读性当前描述仍是 “whether show mask”,未体现
'blur'
的效果。建议在表格里直接说明传入'blur'
将展示带模糊效果的遮罩,便于用户理解与检索。应用如下变更以明确语义:
-| mask | Boolean \| `blur` | true | whether show mask | | +| mask | Boolean \| `blur` | true | 控制是否展示遮罩;传入 `blur` 时为带背景模糊的遮罩 | |assets/index/Mask.less (1)
13-15
: 为 Safari 兼容性补充 -webkit-backdrop-filter 前缀
backdrop-filter
在 Safari 需加上-webkit-backdrop-filter
才能生效。建议同时声明以提升跨浏览器一致性。参考改动:
&&-blur { backdrop-filter: blur(4px); + -webkit-backdrop-filter: blur(4px); }
src/Dialog/Mask.tsx (1)
16-16
: 避免 maskProps.className 覆盖内部类名,建议显式合并目前
{...maskProps}
在 className 之后展开,若maskProps
里带className
会覆盖${prefixCls}-mask
和-mask-blur
,导致样式失效。建议拆出并合并maskProps.className
,其余属性再展开。可按以下方式调整:
- const { prefixCls, style, visible, maskProps, motionName, className, mask } = props; + const { prefixCls, style, visible, maskProps, motionName, className, mask } = props; + const { className: maskPropsClassName, ...restMaskProps } = maskProps || {}; @@ - className={classNames( - `${prefixCls}-mask`, - motionClassName, - className, - mask === 'blur' && `${prefixCls}-mask-blur`, - )} - {...maskProps} + className={classNames( + `${prefixCls}-mask`, + motionClassName, + className, + maskPropsClassName, + mask === 'blur' && `${prefixCls}-mask-blur`, + )} + {...restMaskProps}Also applies to: 28-34
src/IDialogPropTypes.tsx (1)
28-29
: 为 mask 字段补充注释,增强 IDE 悬浮提示与文档可读性明确
'blur'
的行为能减少误用。建议在类型上方添加注释:
- mask?: boolean | 'blur'; + /** true 显示遮罩;false 不显示;`'blur'` 显示并对背景应用模糊效果 */ + mask?: boolean | 'blur';tests/index.spec.tsx (1)
756-761
: 增强断言:同时校验基础遮罩类与 blur 类在同一元素上,且仅渲染一个遮罩为防止未来实现回归(例如新增独立 blur 元素或丢失基础类),建议在同一测试中校验:
- 基础遮罩元素
.rc-dialog-mask
存在;- 该元素包含
.rc-dialog-mask-blur
类;- 遮罩只渲染一个实例。
可在当前用例中追加如下断言:
it('Dialog mask blur', () => { render(<Dialog mask="blur" visible/>); jest.runAllTimers(); expect(document.querySelector('.rc-dialog-root')).toBeTruthy(); - expect(document.querySelector('.rc-dialog-mask-blur')).toBeTruthy(); + expect(document.querySelector('.rc-dialog-mask-blur')).toBeTruthy(); + // 基础遮罩元素仍存在 + const maskEl = document.querySelector('.rc-dialog-mask'); + expect(maskEl).toBeTruthy(); + // blur 类应附加在同一个遮罩元素上 + expect((maskEl as HTMLElement).className).toContain('rc-dialog-mask-blur'); + // 不应渲染重复的遮罩 + expect(document.querySelectorAll('.rc-dialog-mask')).toHaveLength(1); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md
(2 hunks)assets/index/Mask.less
(3 hunks)src/Dialog/Mask.tsx
(2 hunks)src/Dialog/index.tsx
(1 hunks)src/IDialogPropTypes.tsx
(2 hunks)tests/index.spec.tsx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
README.md (1)
107-107
: Contributing 标题与排版调整:LGTM文档结构更清晰,无功能性影响。
assets/index/Mask.less (1)
27-29
: 选择器换行与格式化:LGTM将多个选择器分行更利于维护与 diff 对比,无语义变化。
Also applies to: 39-41
src/Dialog/Mask.tsx (1)
12-12
: MaskProps 扩展 mask?: boolean | 'blur':LGTM类型与新增能力对齐,兼容已有用法。
src/IDialogPropTypes.tsx (1)
4-11
: SemanticName 多行格式化:LGTM仅格式调整,无行为变化。
src/Dialog/index.tsx (1)
201-201
: 向 Mask 组件透传 mask:LGTM与新能力对齐;同时
visible={mask && visible}
在mask === 'blur'
时会返回visible
的布尔值,类型与语义均正确。tests/index.spec.tsx (1)
755-761
: 新增用例方向正确,能验证 blur 掩膜类名注入此用例覆盖了 mask="blur" 的渲染路径,并断言了 rc-dialog-mask-blur 的存在,基本正确。
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this 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)
README.md (1)
68-68
: 类型标注与描述措辞微调建议
- Type 列建议使用 Boolean | 'blur',与代码实现
mask?: boolean | 'blur'
保持一致。- 描述中的 “Can be set to …” 是句子残片,建议合并为一句,避免语法瑕疵(LanguageTool 提示)。
建议按下方 diff 修改该行(仅文案与类型表示调整):
-| mask | Boolean \| `blur` | true | whether to show the mask. Can be set to `blur` for a blur effect. | | +| mask | Boolean \| 'blur' | true | whether to show the mask; set to 'blur' for a blur effect. | |另外,请确认是否需要在 Version 列标注该能力的首次支持版本,便于用户识别变更所属版本。需要的话我可以一并补充。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~68-~68: To form a complete sentence, be sure to include a subject.
Context: ...lur| true | whether to show the mask. Can be set to
blur` for a blur effect. | ...
(MISSING_IT_THERE)
🔇 Additional comments (1)
README.md (1)
107-107
: 文档结构优化已确认移除多余空行并规范化 “Contributing” 小节标题,排版更整洁,无需进一步改动。
| footer | React.Element | | footer of the dialog | | | ||
| closable | Boolean \| ({ closeIcon?: React.ReactNode; disabled?: boolean, afterClose:function } & React.AriaAttributes) | true | whether show close button | | | ||
| mask | Boolean | true | whether show mask | | | ||
| mask | Boolean \| `blur` | true | whether to show the mask. Can be set to `blur` for a blur effect. | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我觉得做到 next 里,然后ConfigProvider 支持一个 mask 的 className 和 style 会比较好。然后做一个语法糖:
<ConfigProvider mask={{ blur: false }} />
mask可配置为blur
Summary by CodeRabbit