-
Notifications
You must be signed in to change notification settings - Fork 287
Feat v3 cpp td #3364
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 v3 cpp td #3364
Conversation
|
Warning Rate limit exceeded@xiaoyatong has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Walkthrough本次变更包含版本与依赖分辨率更新;Taro 事件机制改为使用 eventCenter;Button(Taro) 改为用 View 渲染并简化类型;Swipe 重构宽度测量与拖拽状态更新并调整公开 close 签名;Toast 文本容器样式与渲染标签调整;若干处 ENV 常量处理本地化。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as 用户
participant S as Swipe 组件
participant M as getWidth(测量)
participant ST as 组件状态
U->>S: touchstart
alt 未禁用
S->>M: 计算左右动作区宽度
M-->>S: 返回宽度
S->>ST: 更新宽度/进入拖拽准备状态
else 禁用
S-->>U: 忽略拖拽
end
U->>S: touchmove
S->>ST: 根据位移更新 offset/dragging
U->>S: touchend
S->>ST: 依据阈值打开/关闭并设置过渡
note over S,ST: useMemo 固定 transform/transition
sequenceDiagram
autonumber
participant P as 发布方
participant EC as Taro eventCenter
participant S as 订阅方
S->>EC: on(event, handler)
P->>EC: trigger(event, payload)
EC-->>S: 调用 handler(payload)
S->>EC: off(event, handler)
note over EC: 由本地 Events 实例切换为 eventCenter
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x_cpp #3364 +/- ##
================================================
Coverage ? 89.11%
================================================
Files ? 291
Lines ? 19226
Branches ? 2705
================================================
Hits ? 17134
Misses ? 2082
Partials ? 10 ☔ 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.
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 (4)
src/packages/button/button.taro.tsx (1)
157-181: 恢复使用 TaroButton,避免小程序能力失效这里把组件主体从
<TaroButton>改成了<View>,同时把formType传递逻辑注释掉了。这样会导致以下回归:在小程序端,只有原生 Button 支持formType、openType、表单提交/重置以及相关无障碍行为;换成<View>后,这些能力完全丢失,<NutButton formType="submit">等常见用法会直接失效。请恢复使用<TaroButton>并继续透传表单相关属性,以避免破坏已有业务。示例修复:
- // <TaroButton - <View + <TaroButton {...rest} ref={ref} - // formType={formType || nativeType} + formType={formType || nativeType} className={buttonClassNames} style={{ ...getStyle, ...style }} onClick={(e) => handleClick(e as any)} > <View className="nut-button-wrap"> … - </View> - // </TaroButton> + </TaroButton>src/packages/swipe/swipe.taro.tsx (3)
84-98: setActionWidth 未向回调传递“前一状态”,会导致调用方解构/扩展 undefined 直接报错此处
const res = fn()未传入actionWidth.current,而上方getWidth里以(v) => ({ ...v, left: ... })的形式调用。运行时{...v}在 v 为undefined时会抛 TypeError。请将“当前值”传给回调,或改为合并 Partial。建议最小修复(向回调传入 prev):
- const setActionWidth = (fn: any) => { - const res = fn() + const setActionWidth = (fn: (prev?: { left: number; right: number }) => any) => { + const res = fn(actionWidth.current) if (res.left !== undefined) { updateState({ ...actionWidth.current, left: res.left, }) } if (res.right !== undefined) { updateState({ ...actionWidth.current, right: res.right, }) } }更佳实现(一次性合并 Partial,避免多次 set):
- const setActionWidth = (fn: any) => { - const res = fn(actionWidth.current) - if (res.left !== undefined) { - updateState({ ...actionWidth.current, left: res.left }) - } - if (res.right !== undefined) { - updateState({ ...actionWidth.current, right: res.right }) - } - } + const setActionWidth = ( + updater: (prev: { left: number; right: number }) => Partial<{ left: number; right: number }> + ) => { + const prev = actionWidth.current + const patch = updater(prev) || {} + updateState({ ...prev, ...patch }) + }
151-156: 逻辑错误:toggle里同样将opened作为布尔使用,应为opened.current当前写法会使
baseNum恒为1 - base,改变打开/关闭阈值,造成交互异常。请修复如下:
- const baseNum = opened ? 1 - base : base + const baseNum = opened.current ? 1 - base : base
233-253: document 事件监听需做运行环境守卫,避免小程序端潜在异常Taro 小程序端通常不存在
document;建议在 H5 下再注册监听,并在可行处使用{ passive: true }。建议修改:
- useEffect(() => { + useEffect(() => { // 并没有生效 - const handler: any = (event: { target: Node | null }) => { + const handler: any = (event: { target: Node | null }) => { const targets = [root] if ( targets.some((targetItem) => { const targetElement = targetItem.current || targetItem return !targetElement || targetElement?.contains(event.target) }) ) { return } close() } - - document.addEventListener('touchstart', handler) - return () => { - document.removeEventListener('touchstart', handler) - } - }, [close]) + if (typeof document !== 'undefined' && document.addEventListener) { + document.addEventListener('touchstart', handler, { passive: true } as any) + return () => { + document.removeEventListener('touchstart', handler) + } + } + return () => {} + }, [close])
🧹 Nitpick comments (3)
src/packages/swipe/swipe.taro.tsx (3)
99-105: 细节优化:使用 translate3d 提升移动端动画性能
translate3d(x,0,0)更易触发合成层,拖拽更顺滑。可作为低风险优化。- return { - transform: `translate(${state.offset}px, 0)`, + return { + transform: `translate3d(${state.offset}px, 0, 0)`, transitionDuration: state.dragging ? '0.01s' : '.6s', }
106-113: 去掉多余的 async 或显式 await
onTouchStart被标记为async但未await,可去掉async以避免产生未使用的 Promise;或根据交互需要在首帧前await getWidth()。- const onTouchStart = async (event: BaseEventOrig<HTMLDivElement>) => { + const onTouchStart = (event: BaseEventOrig<HTMLDivElement>) => { if (!props.disabled) { getWidth()
125-129: 可读性提升:加括号避免一元负号与||的优先级歧义当前逻辑正确,但不直观。加括号更清晰。
- -actionWidth.current.right || 0, + -(actionWidth.current.right || 0),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
package.json(3 hunks)src/hooks/taro/use-custom-event.ts(1 hunks)src/packages/button/button.taro.tsx(5 hunks)src/packages/input/input.taro.tsx(1 hunks)src/packages/swipe/swipe.taro.tsx(6 hunks)src/packages/toast/toast.scss(1 hunks)src/packages/toast/toast.taro.tsx(1 hunks)src/utils/taro/get-rect-by-id.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/packages/swipe/swipe.taro.tsx (2)
src/utils/taro/get-rect.ts (1)
getRectInMultiPlatform(29-55)src/types/base/atoms.ts (1)
PositionX(42-42)
src/packages/button/button.taro.tsx (2)
src/packages/button/button.tsx (1)
ButtonProps(18-32)src/utils/typings.ts (1)
BasicComponent(3-8)
⏰ 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: build
🔇 Additional comments (4)
src/packages/toast/toast.scss (1)
95-100: 文本容器改为自适应高度,方向正确
height: auto;解决了多行内容被截断/溢出的风险;与 TSX 中由 View 承载文本的变更一致。建议回归检查极端长文案与有无图标两种布局下的垂直对齐。建议用 Story 或示例页快速验证以下场景:
- 多行 content(中英文、长单词);
- 搭配 icon 与不带 icon;
- size=small/base/large 不同字号。
src/packages/swipe/swipe.taro.tsx (2)
49-62: 与上条相关:getWidth 中回调签名与 setActionWidth 当前实现不匹配在未修复
setActionWidth之前,这里的{ ...v, ... }会因 v 未定义而报错。若采用“传 prev”的最小修复,本段可保留现状;否则请改为返回 Partial,避免扩展未定义。可选精简(配合“合并 Partial”的实现):
- leftRect && setActionWidth((v: any) => ({ ...v, left: leftRect.width })) + leftRect && setActionWidth(() => ({ left: leftRect.width })) ... - rightRect && - setActionWidth((v: any) => ({ ...v, right: rightRect.width })) + rightRect && setActionWidth(() => ({ right: rightRect.width }))另外可并行测量以减少等待:
- if (leftWrapper.current) { await ... } - if (rightWrapper.current) { await ... } + await Promise.all([ + leftWrapper.current + ? getRectInMultiPlatform(leftWrapper.current, leftId).then(r => r && setActionWidth(() => ({ left: r.width }))) + : Promise.resolve(), + rightWrapper.current + ? getRectInMultiPlatform(rightWrapper.current, rightId).then(r => r && setActionWidth(() => ({ right: r.width }))) + : Promise.resolve(), + ])Likely an incorrect or invalid review comment.
228-231: 忽略close参数透出建议
SwipeRef.close在类型定义(src/types/spec/swipe/base.ts:7)中签名为() => void,仓库内也未发现任何带参调用,当前useImperativeHandle(instanceRef, () => ({ open, close: () => close() }))不会破坏兼容性,无需修改。Likely an incorrect or invalid review comment.
src/packages/toast/toast.taro.tsx (1)
184-186: content 从 Text 切换为 View:支持多行和复杂内容,配合 height: auto 避免 Text 的换行/嵌套限制。测试仅通过
.nut-toast-text类断言内容,未依赖具体标签;样式同理,无需更新测试或快照。
| if (touch.isHorizontal()) { | ||
| lockClick.current = true | ||
| const newState = { ...state, dragging: true } | ||
| const isEdge = !opened || touch.deltaX.current * startOffset.current < 0 | ||
| if (isEdge) { | ||
| preventDefault(event, true) | ||
| } | ||
|
|
||
| newState.offset = rangeCalculation( | ||
| const offset = rangeCalculation( | ||
| touch.deltaX.current + startOffset.current, |
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.
逻辑错误:使用了 !opened 而非 !opened.current,导致边界检测异常
opened 是 useRef 返回的对象,始终为 truthy,!opened 永远为 false,影响 isEdge 的判断及随后的阻止默认行为。
请修复如下:
- const isEdge = !opened || touch.deltaX.current * startOffset.current < 0
+ const isEdge = !opened.current || touch.deltaX.current * startOffset.current < 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (touch.isHorizontal()) { | |
| lockClick.current = true | |
| const newState = { ...state, dragging: true } | |
| const isEdge = !opened || touch.deltaX.current * startOffset.current < 0 | |
| if (isEdge) { | |
| preventDefault(event, true) | |
| } | |
| newState.offset = rangeCalculation( | |
| const offset = rangeCalculation( | |
| touch.deltaX.current + startOffset.current, | |
| if (touch.isHorizontal()) { | |
| lockClick.current = true | |
| const isEdge = !opened.current || touch.deltaX.current * startOffset.current < 0 | |
| if (isEdge) { | |
| preventDefault(event, true) | |
| } | |
| const offset = rangeCalculation( | |
| touch.deltaX.current + startOffset.current, |
🤖 Prompt for AI Agents
In src/packages/swipe/swipe.taro.tsx around lines 119 to 126, the code
incorrectly checks `!opened` where `opened` is a useRef object; change the
condition to use the ref value (`!opened.current`) so the edge detection works
correctly; update any other places in this block that reference `opened` for
boolean logic to use `opened.current` to ensure preventDefault and isEdge behave
as intended.
| const close = useCallback( | ||
| (position?: PositionX) => { | ||
| if (opened.current) { | ||
| opened.current = false | ||
| props.onClose?.({ | ||
| name: props.name as number | string, | ||
| position: position || 'left', | ||
| }) | ||
| } | ||
| setState((prevState) => ({ | ||
| ...prevState, | ||
| offset: 0, | ||
| })) | ||
| }, | ||
| [props] | ||
| ) |
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.
close() 的 position 默认值不准确且会吞掉外部传入的 position
- 当右侧打开时,
close()默认position: 'left'会错误上报关闭位置。 useImperativeHandle中暴露为close: () => close(),即便调用方传入了close('right'),也会被忽略(静默破坏既有用法)。
建议:
- 记录上一次打开的侧别,用于
close()无参时的 fallback。 useImperativeHandle直接暴露原始close,保留参数。
示例修复:
const opened = useRef(false)
+ const lastSideRef = useRef<PositionX | null>(null)
...
const open = (side: PositionX) => {
opened.current = true
+ lastSideRef.current = side
...
- const close = useCallback(
- (position?: PositionX) => {
+ const close = useCallback(
+ (position?: PositionX) => {
if (opened.current) {
opened.current = false
props.onClose?.({
name: props.name as number | string,
- position: position || 'left',
+ position: position ?? lastSideRef.current ?? (state.offset > 0 ? 'left' : 'right'),
})
}
setState((prevState) => ({
...prevState,
offset: 0,
}))
},
[props]
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const close = useCallback( | |
| (position?: PositionX) => { | |
| if (opened.current) { | |
| opened.current = false | |
| props.onClose?.({ | |
| name: props.name as number | string, | |
| position: position || 'left', | |
| }) | |
| } | |
| setState((prevState) => ({ | |
| ...prevState, | |
| offset: 0, | |
| })) | |
| }, | |
| [props] | |
| ) | |
| const opened = useRef(false) | |
| const lastSideRef = useRef<PositionX | null>(null) | |
| const open = (side: PositionX) => { | |
| opened.current = true | |
| lastSideRef.current = side | |
| // …rest of open logic… | |
| } | |
| const close = useCallback( | |
| (position?: PositionX) => { | |
| if (opened.current) { | |
| opened.current = false | |
| props.onClose?.({ | |
| name: props.name as number | string, | |
| position: position ?? lastSideRef.current ?? (state.offset > 0 ? 'left' : 'right'), | |
| }) | |
| } | |
| setState((prevState) => ({ | |
| ...prevState, | |
| offset: 0, | |
| })) | |
| }, | |
| [props] | |
| ) |
🤖 Prompt for AI Agents
In src/packages/swipe/swipe.taro.tsx around lines 174-189, close() currently
defaults position to 'left' and ignores any passed-in position when invoked via
the imperative handle; fix by tracking the last opened side (e.g., store
lastPosition when opening) and use that as the fallback when close() is called
without an argument, change the close signature to accept an optional position
and use position ?? lastPosition ?? 'left' when calling props.onClose, and
update useImperativeHandle to expose the original close function (not a wrapper
that drops parameters) so external callers can pass a position like
close('right') and have it respected.
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
重构
样式
杂务