-
Notifications
You must be signed in to change notification settings - Fork 4.9k
feat: 删除原有H5 lego模式,react button重构支持dynamic模式 #18576
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
功能概述本次更改涉及两个主要组件的修改:Button 组件增加了 变更文件
代码审查工作量估计🎯 3 (中等难度) | ⏱️ ~20-25 分钟 需要重点关注的区域:
可能相关的 PR
建议审查人员
兔子的庆祝诗
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/taro-components-react/src/components/button/index.tsx (1)
119-135: 严重的可访问性和语义问题:使用 View 替代原生 button 元素。将原生
<button>元素替换为<View>会导致严重的可访问性和功能问题:
- 失去语义化:View 不是按钮,屏幕阅读器无法识别其为可交互元素
- 键盘导航失效:原生按钮支持 Enter 和 Space 键触发,View 需要手动实现
- 焦点管理缺失:View 默认不可聚焦,需要添加
tabIndex={0}和role="button"- 表单集成失效:
- 第 121 行将
type传递给 View 无意义(View 没有 type 属性)- 第 131 行的
form-type={formType as any}作为自定义属性不会触发原生表单提交/重置- 第 122-123 行的
size和disabled属性 View 也不支持建议的修复方案:
如果必须使用 View(例如跨平台兼容性考虑),需要添加完整的可访问性支持:
- <View + <View + role="button" + tabIndex={disabled ? -1 : 0} + aria-disabled={disabled} {...omit(restProps, [...])} - type={type} - size={size} - disabled={disabled} + data-type={type} + data-size={size} + aria-label={children} ref={forwardedRef} className={cls} style={style} onClick={handleClick} + onKeyDown={(e) => { + if ((e.key === 'Enter' || e.key === ' ') && !disabled) { + e.preventDefault() + handleClick(e as any) + } + }} onTouchStart={_onTouchStart} onTouchEnd={_onTouchEnd} plain={plain.toString()} - form-type={formType as any} + data-form-type={formType} >或者考虑保留原生
<button>元素以获得完整的原生支持。相关文档:
🧹 Nitpick comments (1)
packages/taro-components-react/src/components/button/index.tsx (1)
133-133: 为加载状态添加可访问性支持。Loading 状态应该通过 ARIA 属性通知辅助技术:
建议在父 View 元素上添加
aria-busy属性:在 packages/taro-components-react/src/components/button/index.tsx 第 119-135 行的 View 元素上:
<View + aria-busy={loading} {...omit(restProps, [...])}这样屏幕阅读器会通知用户按钮正在加载中。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/taro-components-react/src/components/button/index.tsx(5 hunks)packages/taro-components-react/src/components/button/style/index.scss(4 hunks)packages/taro-components-react/src/components/button/style/weui-btn_loading.scss(1 hunks)packages/taro-components-react/src/components/image/index.tsx(0 hunks)packages/taro-components/src/components/image/image.tsx(1 hunks)packages/taro-components/src/components/image/readme.md(0 hunks)
💤 Files with no reviewable changes (2)
- packages/taro-components-react/src/components/image/index.tsx
- packages/taro-components/src/components/image/readme.md
🧰 Additional context used
🧬 Code graph analysis (1)
packages/taro-components-react/src/components/button/index.tsx (1)
packages/taro-components-react/src/utils/index.tsx (1)
omit(35-42)
⏰ 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). (9)
- GitHub Check: Build Rust Binding / stable - aarch64-apple-darwin
- GitHub Check: Build Rust Binding / stable - x86_64-unknown-linux-gnu
- GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
- GitHub Check: Build Rust Binding / stable - x86_64-apple-darwin
- GitHub Check: Build Rust WASM / stable - wasm32-wasi
- GitHub Check: Build Rust Binding / stable - aarch64-apple-darwin
- GitHub Check: Build Rust Binding / stable - x86_64-pc-windows-msvc
- GitHub Check: Build Rust Binding / stable - x86_64-apple-darwin
- GitHub Check: Build Rust WASM / stable - wasm32-wasi
🔇 Additional comments (10)
packages/taro-components/src/components/image/image.tsx (3)
43-57: 懒加载实现正确!IntersectionObserver 的使用符合最佳实践:
- 300px 的 rootMargin 提供了合理的预加载距离
- 正确地在触发后取消观察以避免内存泄漏
- 逻辑清晰,性能良好
79-117: 渲染逻辑实现正确!渲染方法的实现简洁且功能完整:
- 懒加载条件判断正确(第 109 行):当启用懒加载且未加载时,
src设为undefined- 类名生成逻辑覆盖了所有 mode 类型
- 事件处理器绑定正确
nativeProps的展开提供了良好的扩展性
1-118: 代码清理和重构验证完成 ✓已验证以下项目,确认清理彻底完整:
- ✓ 无任何 lego 相关函数引用(isLegoScriptLoaded、insertLegoScript、parseLegoUrl、legoData)
- ✓ 无任何 lego CDN URL 引用
- ✓ Image 组件中无 lang 属性使用
- ✓ 类型定义中无 Image 组件的 lang 属性定义
该组件已成功简化为标准的图片组件实现,移除了 Lego 模式依赖和 lang 属性,代码清洁、无遗留引用。
packages/taro-components-react/src/components/button/style/index.scss (2)
97-103: 验证 mini 按钮与 flex 布局的兼容性。mini 按钮的变更包括:
- 内边距从
0 1.32em改为0 26px(同样失去字体大小自适应能力)- 新增
vertical-align: middle需要注意的是,mini 按钮使用
display: inline-block(第 98 行),而父容器.taro-button-core使用display: flex。请验证:
- 当 mini 按钮应用 flex 布局时,
vertical-align: middle是否仍然有效(flex 子元素通常不受 vertical-align 影响)- mini 按钮在行内排列时的对齐表现
52-58: 需要手动验证代码变更情况,以确认原始代码状态。根据当前文件内容验证,发现以下情况:
当前
.taro-button-core类(52-58行)已采用display: flex和align-items: center+justify-content: center实现居中,这是正确的现代做法。但在核实审核意见时发现了问题:
- 当前代码使用的已经是
padding: 0 14px(px 单位),无法验证是否曾使用 em 单位- 当前
.taro-button-core类中不存在text-align或line-height属性,无法验证这些属性是否被移除- 整个组件样式中没有找到 em 单位的内边距用法
由于缺少变更前的代码对比,无法确认审核意见中关于"从 em 改为 px"和"属性被移除"的具体变更。建议:
- 检查 git diff 确认原始代码状态
- 如果确实存在这些变更,需验证在各种内容场景下的视觉效果
packages/taro-components-react/src/components/button/index.tsx (3)
23-23: formType 属性定义正确。新增的
formType?: 'submit' | 'reset'属性类型定义清晰,符合表单按钮的标准类型。
79-84: 审查意见基于对 Taro 框架架构的误解,实际不存在问题。代码中的自定义事件实现是正确的。经验证:
Form 组件已正确监听事件:Form 组件(
packages/taro-components/src/components/form/form.tsx)使用@Listen('tarobuttonsubmit')和@Listen('tarobuttonreset')装饰器,正确捕获 Button 组件触发的自定义事件。框架为 Stencil.js 而非原生 HTML:Form 组件使用
@stencil/core的@Component和@Listen装饰器,这是 Stencil.js 框架的标准模式,支持自定义事件的声明式监听。事件处理完整:
onButtonSubmit处理器收集表单值并触发标准submit事件onButtonReset处理器调用form.reset()Likely an incorrect or invalid review comment.
92-98: 建议手动验证禁用按钮的事件阻止行为。该变更引入了
stopPropagation()来阻止禁用按钮的事件冒泡。虽然代码搜索未发现显式依赖此事件的父组件,但存在以下风险:
- React Native 版本未使用此行为(仅用
!disabled && onClick()判断),产生行为差异- 事件委托或捕获阶段的事件监听可能在业务代码中存在
- 组件库代码(pull-down-refresh)曾注释掉
stopPropagation()以避免事件委托问题建议在合并前验证:是否存在依赖禁用按钮点击事件冒泡的业务逻辑或第三方代码。
packages/taro-components-react/src/components/button/style/weui-btn_loading.scss (2)
1-19: 加载动画定义正确。
weuiLoading关键帧动画定义清晰,同时提供了-webkit-前缀版本以兼容旧版浏览器,使用rotate3d实现旋转效果符合最佳实践。
46-53: mini 加载指示器尺寸合理。为 mini 按钮提供 16px 的加载指示器尺寸是合理的,
vertical-align: middle确保了在行内布局中的正确对齐。不过需要注意,根据之前对
index.scss的评论,当按钮使用display: flex时,vertical-align可能不会生效。请验证 mini 按钮在 flex 布局下的表现。
| // Button Mini | ||
| $weuiBtnMiniFontSize: 13px; | ||
| $weuiBtnMiniHeight: 2.3; | ||
| $weuiBtnMiniHeight: 2; |
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.
🧩 Analysis chain
确认 mini 按钮高度变量的单位。
$weuiBtnMiniHeight: 2; 是一个无单位的值,这看起来不太符合预期。通常高度变量应该有明确的单位(如 px、em 等)。请确认:
- 这个变量在代码中的实际使用方式
- 是否应该是
2em或20px等带单位的值
另外,从 2.3 降低到 2 是 13% 的高度减少,请确保这不会影响 mini 按钮的视觉效果和可用性。
运行以下脚本查找该变量的使用位置:
🏁 Script executed:
#!/bin/bash
# 查找 weuiBtnMiniHeight 变量的使用位置
rg -n '\$weuiBtnMiniHeight' --type scssLength of output: 28
让我用更合适的方式重新搜索该变量的使用位置。
---
🏁 Script executed:
```shell
#!/bin/bash
# 使用 glob 模式搜索 SCSS 文件中的变量
rg -n '\$weuiBtnMiniHeight' --glob '*.scss' -C 3
Length of output: 3915
🏁 Script executed:
#!/bin/bash
# 同时查看源文件内容,了解变量定义和使用上下文
cat packages/taro-components-react/src/components/button/style/index.scss | head -40Length of output: 1137
🏁 Script executed:
#!/bin/bash
# 寻找所有在 button 样式目录中的 SCSS 文件
find packages/taro-components-react/src/components/button/style -name '*.scss' -type fLength of output: 223
🏁 Script executed:
#!/bin/bash
# 检查 index.scss 是否导入 variable.scss,以及变量定义的覆盖顺序
head -20 packages/taro-components-react/src/components/button/style/index.scssLength of output: 558
🏁 Script executed:
#!/bin/bash
# 检查 variable.scss 中的变量定义
cat packages/taro-components-react/src/components/button/style/variable.scss | head -30Length of output: 977
更新 variable.scss 中的 $weuiBtnMiniHeight 变量以保持一致性
无单位的 line-height 值实际上是正确的 SCSS/CSS 用法(作为相对倍数)。但存在一个不一致之处:
packages/taro-components-react/src/components/button/style/variable.scss第21行仍为$weuiBtnMiniHeight: 2.3;packages/taro-components-react/src/components/button/style/index.scss第1行导入了variable.scss,随后第13行将其覆盖为$weuiBtnMiniHeight: 2;
应将 variable.scss 中的定义也更新为 2,以避免混淆并保持代码一致性。
🤖 Prompt for AI Agents
packages/taro-components-react/src/components/button/style/variable.scss around
line 21: the `$weuiBtnMiniHeight` value is 2.3 which conflicts with the override
in index.scss (line 13) set to 2; update the variable here to 2 (no unit) to
match index.scss and remove confusion — optionally remove the redundant override
in index.scss if you want a single source of truth.
| .taro-button-core.taro-btn-loading { | ||
| .weui-loading { | ||
| display: inline-block; | ||
| width: 22px; | ||
| height: 22px; | ||
| animation: weuiLoading 1s steps(12, end) infinite; | ||
| background: transparent url("https://img13.360buyimg.com/imagetools/jfs/t1/353674/27/5761/14397/690ac1cfFf6bf0e2c/77727d0791bacf8e.png") no-repeat; | ||
| background-size: cover; | ||
|
|
||
| &.weui-btn_primary, | ||
| &.weui-btn_warn { | ||
| color: $weuiBtnActiveFontColor; | ||
| } | ||
| &.weui-btn_primary { | ||
| // color: var(--weui-WHITE); | ||
| background-color: $weuiBtnPrimaryActiveBg; | ||
| } | ||
|
|
||
| &.weui-btn_warn { | ||
| background-color: $weuiBtnWarnActiveBg; | ||
| } | ||
| } | ||
|
|
||
| } |
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.
🧩 Analysis chain
修复选择器嵌套错误和外部资源依赖。
当前实现存在以下问题:
1. 选择器嵌套错误(关键问题):
第 30-41 行的嵌套选择器逻辑有误:
.taro-button-core.taro-btn-loading {
.weui-loading {
&.weui-btn_primary, // ❌ 错误:.weui-loading 元素不会有这些类
&.weui-btn_warn {
color: $weuiBtnActiveFontColor;
}.weui-loading 是加载图标元素,不会有 .weui-btn_primary 或 .weui-btn_warn 类。这些类应该在父元素 .taro-button-core 上。
正确的写法应该是:
.taro-button-core.taro-btn-loading {
.weui-loading {
display: inline-block;
width: 22px;
height: 22px;
animation: weuiLoading 1s steps(12, end) infinite;
background: transparent url("...") no-repeat;
background-size: cover;
}
// 颜色和背景应该在按钮级别控制
&.taro-btn-primary,
&.taro-btn-warn {
color: $weuiBtnActiveFontColor;
}
&.taro-btn-primary {
background-color: $weuiBtnPrimaryActiveBg;
}
&.taro-btn-warn {
background-color: $weuiBtnWarnActiveBg;
}
}2. 外部资源依赖(重大问题):
第 27 行使用了 360buyimg.com(京东)的 CDN 图片:
- 如果 CDN 不可用,加载动画将失效
- 跨域请求可能影响性能
- 对外部服务的依赖增加了不确定性
建议:
- 将图片资源内联为 base64 或 SVG data URL
- 或将图片放到项目静态资源目录中
运行以下脚本检查是否有其他外部资源依赖:
🏁 Script executed:
#!/bin/bash
# 查找所有外部 URL 引用
rg -n 'url\(["\x27]?https?://' packages/taro-components-react/src/components/button/style/ --type scssLength of output: 28
Now I'll run corrected scripts to verify the issues:
Now let me run scripts to check the file and verify the actual code structure:
🏁 Script executed:
#!/bin/bash
# Find the SCSS file and display its content
fd 'weui-btn_loading.scss' --exec cat {}Length of output: 1735
🏁 Script executed:
#!/bin/bash
# Search for external URL in CSS/SCSS files
rg -n 'https?://' packages/taro-components-react/src/components/button/style/ -g '*.scss'Length of output: 235
修复选择器嵌套逻辑错误和外部 CDN 依赖。
验证确认以下问题:
1. 选择器嵌套错误(关键问题)
第 30-41 行的嵌套选择器逻辑有误。当前代码:
.taro-button-core.taro-btn-loading {
.weui-loading {
&.weui-btn_primary, // ❌ 错误:编译后为 .weui-loading.weui-btn_primary
&.weui-btn_warn {
color: $weuiBtnActiveFontColor;
}这表示 .weui-loading 元素本身应具有 .weui-btn_primary 或 .weui-btn_warn 类,但这些类实际上应该在按钮元素上。应改为:
.taro-button-core.taro-btn-loading {
&.weui-btn_primary,
&.weui-btn_warn {
.weui-loading {
color: $weuiBtnActiveFontColor;
}
}
&.weui-btn_primary .weui-loading {
background-color: $weuiBtnPrimaryActiveBg;
}
&.weui-btn_warn .weui-loading {
background-color: $weuiBtnWarnActiveBg;
}
}2. 外部 CDN 依赖
第 27 行使用了 360buyimg.com 的 CDN 图片,存在单点失效风险。建议改为 Base64 或本地资源。
🤖 Prompt for AI Agents
packages/taro-components-react/src/components/button/style/weui-btn_loading.scss
lines 21-44: the selector nesting is wrong — classes like .weui-btn_primary and
.weui-btn_warn are applied to the button, not the .weui-loading element — and
there's an external CDN image used. Fix by moving the
.weui-btn_primary/.weui-btn_warn selectors to the parent
.taro-button-core.taro-btn-loading (so they wrap .weui-loading) and adjust the
background-color rules to target .weui-loading inside those parent selectors;
also replace the external URL background-image with a local asset or embedded
Base64 resource and update the background declaration accordingly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #18576 +/- ##
========================================
Coverage 55.97% 55.97%
========================================
Files 416 416
Lines 21563 21563
Branches 5302 5277 -25
========================================
Hits 12070 12070
+ Misses 8006 7875 -131
- Partials 1487 1618 +131
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
这个 PR 做了什么? (简要描述所做更改)
Feat:删除原有H5 lego模式,react button重构支持dynamic模式
这个 PR 是什么类型? (至少选择一个)
这个 PR 涉及以下平台:
Summary by CodeRabbit
发布说明
新功能
Bug 修复
样式
重构