-
Notifications
You must be signed in to change notification settings - Fork 0
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(): rename illustrations.error-message to eo-illustration-message, and support status variants #1395
Conversation
Walkthrough此拉取请求包含多个更改,主要涉及组件文档和实现的更新。新增了 Changes
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 (
|
…, and support status variants
c647987
to
5a72981
Compare
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: 2
🧹 Outside diff range and nitpick comments (8)
bricks/illustrations/src/illustration-message/styles.shadow.css (2)
9-11
: 建议限制通用选择器的作用范围通用选择器
*
可能会影响到所有嵌套的子组件。建议将其限制在特定的作用域内,以避免样式污染。-* { +:host * { box-sizing: border-box; }
39-57
: 建议使用响应式字体大小状态图标使用固定的字体大小(72px)可能在不同屏幕尺寸下显示不合适。建议考虑使用相对单位或响应式设计方案。
颜色变量的使用符合设计系统规范,这很好。
.status { - font-size: 72px; + font-size: clamp(48px, 8vw, 72px); }bricks/illustrations/package.json (1)
73-74
: 建议限制图标包的版本范围虽然添加
@next-bricks/icons
作为 peer dependency 是合理的,但使用*
作为版本范围可能会导致兼容性问题。建议指定一个主版本号范围,例如^1.0.0
。"peerDependencies": { - "@next-bricks/icons": "*" + "@next-bricks/icons": "^1.0.0" }bricks/illustrations/src/illustration-message/index.spec.tsx (1)
132-181
: 建议扩展状态变体测试覆盖范围当前测试验证了图标的变化,建议添加以下测试项:
- 验证每个状态变体对应的 class 变化
- 验证不同状态下图标的 theme 属性是否正确
建议添加如下测试代码:
// 验证 class 变化 expect( element.shadowRoot?.querySelector(".status")?.classList.contains("success") ).toBe(true); await act(async () => { element.variant = "error"; }); expect( element.shadowRoot?.querySelector(".status")?.classList.contains("error") ).toBe(true); // 验证 theme 属性 expect( element.shadowRoot?.querySelector("eo-icon")?.getAttribute("theme") ).toBe("filled");bricks/illustrations/src/illustration-message/index.tsx (4)
83-86
: [优化建议] 考虑移除对已弃用属性errorTitle
的依赖在
render
方法中,heading
的值为this.heading || this.errorTitle
,其中errorTitle
已被标记为弃用。建议逐步移除对errorTitle
的依赖,确保新代码只使用heading
属性。
54-57
: [一致性建议] 更新别名以反映组件重命名在
@defineElement
装饰器中,alias
包含了旧的组件名称"illustrations.error-message"
。为了保持命名一致性,建议更新或移除别名,以避免混淆。
69-70
: [弃用警告] 属性errorTitle
已弃用
errorTitle
属性已被标记为弃用,建议在文档和代码中明确指出,并鼓励使用新的heading
属性。
165-167
: [代码优化] 检查<slot />
的使用是否必要在 JSX 中使用
<slot />
可能并非必要,或者可能需要替换为适当的 React 插槽实现。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
bricks/illustrations/docs/eo-illustration-message.md
(1 hunks)bricks/illustrations/docs/error-message.md
(0 hunks)bricks/illustrations/package.json
(1 hunks)bricks/illustrations/src/bootstrap.ts
(1 hunks)bricks/illustrations/src/error-message/index.tsx
(0 hunks)bricks/illustrations/src/illustration-message/index.spec.tsx
(8 hunks)bricks/illustrations/src/illustration-message/index.tsx
(1 hunks)bricks/illustrations/src/illustration-message/styles.shadow.css
(2 hunks)shared/common-bricks/common-bricks.json
(1 hunks)
💤 Files with no reviewable changes (2)
- bricks/illustrations/docs/error-message.md
- bricks/illustrations/src/error-message/index.tsx
✅ Files skipped from review due to trivial changes (1)
- bricks/illustrations/src/bootstrap.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- bricks/illustrations/docs/eo-illustration-message.md
- shared/common-bricks/common-bricks.json
🔇 Additional comments (6)
bricks/illustrations/src/illustration-message/styles.shadow.css (1)
26-26
: 命名更改符合语义化原则
将 .title
更改为 .heading
更好地体现了元素的语义,这是一个很好的改进。
bricks/illustrations/package.json (1)
73-74
: 验证图标包的使用情况
需要确认新组件中图标的使用是否符合预期。
✅ Verification successful
图标使用验证完成
根据源代码分析,illustration-message 组件中的图标使用符合预期:
- 正确引入了
@next-bricks/icons
包中的GeneralIcon
组件 - 使用
wrapBrick
正确封装了图标组件 - 仅在状态类型(success/error/warning/info)的消息中使用图标
- 图标的使用场景明确且合理
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证 @next-bricks/icons 的导入和使用情况
# 测试:搜索代码库中对图标包的引用
echo "检查对 @next-bricks/icons 的引用:"
rg "@next-bricks/icons" --type ts --type tsx
# 测试:检查新组件中的图标使用
echo "检查 illustration-message 组件中的图标使用:"
fd "illustration-message" --type f --exec rg "Icon|icon" {}
Length of output: 276
Script:
#!/bin/bash
# 修正搜索命令,使用更通用的文件类型
# 检查对图标包的引用
echo "检查对 @next-bricks/icons 的引用:"
rg "@next-bricks/icons"
# 检查新组件中的图标使用
echo "检查 illustration-message 组件中的图标使用:"
fd "illustration-message" --type f --exec cat {} \;
Length of output: 48279
Script:
#!/bin/bash
# 检查 illustration-message 组件的源代码
echo "检查 illustration-message 组件源代码:"
fd "illustration-message.*\.(tsx|ts)$" --type f --exec cat {} \;
Length of output: 140
Script:
#!/bin/bash
# 使用更精确的路径搜索
echo "检查 illustration-message 组件源代码:"
cat bricks/illustrations/src/illustration-message/index.tsx
echo -e "\n检查组件中的图标引用:"
rg -l "general-icon" bricks/illustrations/src/illustration-message/
Length of output: 4426
bricks/illustrations/src/illustration-message/index.spec.tsx (4)
4-4
: 类型导入更新正确!
类型从 ErrorMessage
更新为 IllustrationMessage
符合组件重命名的要求。
Line range hint 19-56
: 基础用法测试更新合理,并改进了可访问性!
- 组件名称更新为
eo-illustration-message
- 为图片添加了
alt
属性,提高了可访问性 - 类名从
title
更新为更语义化的heading
Line range hint 70-90
: 未找到场景测试更新正确!
测试用例正确反映了组件重命名和图片属性的更新。
Line range hint 103-130
: 未知错误场景测试更新合理!
测试结构简化且保持了功能完整性。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1395 +/- ##
==========================================
+ Coverage 85.18% 85.19% +0.01%
==========================================
Files 507 507
Lines 14906 14921 +15
Branches 2220 2222 +2
==========================================
+ Hits 12697 12712 +15
Misses 1775 1775
Partials 434 434
|
🚀 Deployed on https://docs-preview-1395--next-bricks.netlify.app |
📐🤏 Size check result (9781ac1...c896e3e): Load all bricks together
Critical changes: None. See full changes
Load bricks by each packageCritical changes:
See full changes
Critical details for package illustrations
Load by each brickCritical changes:
See full changes
Critical details for brick illustrations.error-message
|
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat
作为提交类型。BREAKING CHANGE: 你的变更说明
。新特性:
feat
作为提交类型。问题修复:
fix
作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore
,docs
,test
等作为提交类型。Summary by CodeRabbit
新功能
eo-illustration-message
组件,支持多种消息展示和配置。文档
eo-illustration-message
组件的使用文档,包含 YAML 格式的示例。error-message
组件的文档。样式
.success
、.error
、.warning
和.info
)。.title
类重命名为.heading
,以提高一致性。