-
-
Notifications
You must be signed in to change notification settings - Fork 194
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
feat: supports classnames & styles #484
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough本次更改涉及对 Changes
Poem
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/Popup.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
src/Popup.tsx (1)
15-16
: 属性解构合理,但建议优化代码格式props 解构的更新正确包含了新增的
overlayInnerClassName
属性。建议应用以下格式优化,使代码更易读:
- const { children, prefixCls, id, overlayInnerStyle, overlayInnerClassName, className, style } = - props; + const { + children, + prefixCls, + id, + overlayInnerStyle, + overlayInnerClassName, + className, + style, + } = props;src/Tooltip.tsx (1)
46-47
: 建议添加属性文档注释为了提高代码的可维护性和使用体验,建议为新添加的
styles
和classNames
属性添加 JSDoc 文档注释,说明其用途和使用方式。+/** 自定义样式配置对象,可分别设置根元素和内部元素的样式 */ styles?: TootipStyles; +/** 自定义类名配置对象,可分别设置根元素和内部元素的类名 */ classNames?: TooltipClassNames;tests/index.test.tsx (2)
265-269
: 建议增加更多边界测试场景当前测试用例仅验证了基本功能,建议添加以下场景:
- 样式和类名为空对象的情况
- 样式值包含特殊字符的情况
- 类名包含多个空格的情况
254-284
: 建议添加异步交互测试当前测试用例只验证了初始渲染状态,建议添加以下场景:
- 动态更新样式和类名
- 切换可见性时样式和类名的保留情况
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/Popup.tsx
(1 hunks)src/Tooltip.tsx
(5 hunks)tests/index.test.tsx
(1 hunks)
🔇 Additional comments (3)
src/Popup.tsx (1)
11-11
: 接口扩展符合预期
新增的 overlayInnerClassName
属性类型定义正确,可选参数的设计也合理。这样的扩展不会影响现有代码的兼容性。
src/Tooltip.tsx (1)
100-106
: 实现看起来不错,建议验证样式优先级
样式和类名的合并实现正确,使用了适当的展开运算符来合并样式对象。建议验证样式优先级是否符合预期,特别是在覆盖默认样式时。
Also applies to: 113-113, 130-130
✅ Verification successful
样式优先级实现已得到验证
代码中的样式优先级处理已经得到正确实现:
- 在
overlayInnerStyle
中,通过展开运算符确保tooltipStyles?.inner
在后,可以覆盖默认样式 - 测试文件中包含了样式覆盖的测试用例,验证了自定义样式可以正确覆盖默认样式
classNames
工具函数的使用确保了类名的正确组合
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查是否有相关的测试用例验证样式优先级
# 搜索测试文件中的相关测试用例
rg -l "styles.*root|classNames.*root" tests/
Length of output: 46
Script:
#!/bin/bash
# 搜索 Tooltip 相关的测试文件
fd "tooltip.*\.(test|spec)\.(ts|tsx|js|jsx)" -i
# 搜索测试文件中的样式相关测试
rg -i "style|classname" --type ts --type tsx --type js --type jsx -A 5 "tests/"
# 检查 Tooltip 组件中的样式处理
ast-grep --pattern 'overlayInnerStyle = { $$ }'
# 搜索样式优先级相关的测试用例
rg -i "tooltip.*style.*priority|style.*priority.*tooltip" --type ts --type tsx --type js --type jsx
Length of output: 337
Script:
#!/bin/bash
# 搜索 Tooltip 相关的测试文件,不指定文件类型让 fd 自动处理
fd "tooltip.*test" -i
# 搜索样式相关的代码实现
rg "overlayInnerStyle|tooltipStyles" -A 3
# 搜索样式相关的测试用例
rg "style.*test|test.*style" -A 3
# 检查是否有样式覆盖的相关实现
rg "overlayInnerClassName|tooltipClassNames" -A 3
Length of output: 4549
tests/index.test.tsx (1)
254-264
: 测试用例的设置结构清晰明确
测试数据的准备工作组织得当,通过分别定义 customClassNames
和 customStyles
对象使得测试意图明确。
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
🧹 Outside diff range and nitpick comments (2)
src/Tooltip.tsx (2)
100-106
: 建议增加样式合并的类型安全性当前的样式合并实现功能正确,但建议添加空值检查以提高代码的健壮性。
-overlayInnerStyle={{ ...overlayInnerStyle, ...tooltipStyles?.inner }} +overlayInnerStyle={{ + ...(overlayInnerStyle || {}), + ...(tooltipStyles?.inner || {}) +}}
113-113
: 保持样式处理的一致性为了保持代码一致性,建议对
popupStyle
的样式合并也添加相同的空值检查。同时,classNames
工具的使用非常恰当。-popupStyle={{ ...overlayStyle, ...tooltipStyles?.root }} +popupStyle={{ + ...(overlayStyle || {}), + ...(tooltipStyles?.root || {}) +}}Also applies to: 130-130
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/Tooltip.tsx
(5 hunks)
🔇 Additional comments (2)
src/Tooltip.tsx (2)
8-8
: 接口设计清晰且符合 React 规范!
新增的 TooltipStyles
和 TooltipClassNames
接口设计合理,类型定义准确,并且与 React 组件的样式自定义模式保持一致。
Also applies to: 46-57
86-87
: 属性解构合理!
tooltipClassNames
和 tooltipStyles
的命名清晰,解构位置恰当,与其他样式相关属性放在一起便于维护。
src/Popup.tsx
Outdated
@@ -8,14 +8,21 @@ export interface ContentProps { | |||
overlayInnerStyle?: React.CSSProperties; | |||
className?: string; | |||
style?: React.CSSProperties; | |||
overlayInnerClassName?: string; |
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.
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.
overlayInnerClassName 和 overlayInnerStyle 改为 innerStyle 和 innerClassName 吧,overlay 在这里是多余的语义。
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.
innerClassName 改好了,innerStyle 只能用别名的方式来了,毕竟popup这个组件导出在外部组件可用,不太能动了,不然可能引发break
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #484 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 26 26
Branches 10 10
=========================================
Hits 26 26 ☔ View full report in Codecov by Sentry. |
ant-design/ant-design#51814
Summary by CodeRabbit
更新后的版本说明
新功能
Popup
组件中添加了可选属性innerClassName
,增强了内层覆盖层的样式灵活性。Tooltip
组件中添加了styles
和classNames
属性,允许用户自定义工具提示的样式和类名。测试
Tooltip
组件添加了新测试用例,以验证自定义样式和类名的应用。