-
Couldn't load subscription status.
- Fork 286
feat(popup): 增加一些新属性,顺便解决了下input的只读无法点击、焦点等问题 #3342
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
* feat: 修复属性透传 * feat: 直接删除duration
* fix(range): taro环境异步渲染useReady不会触发 * refactor: 将组件中使用的useReady替换为useLayoutEffect
* feat: inputRef获取真实input-dom * feat: 兼容小程序和h5获取input标签 * feat: 取消?问好,出错直接抛出
* feat: 支持popup 高度可以伸缩 * feat: 适配小程序 * feat: 修改文档 * fix: 默认值不需要,走样式 * feat: 增加使用的限制条件 * docs: 增加文档 * fix: 适配鸿蒙,初始值重置修改 * test: 添加单测 * fix: 增加h5 的初始值 * fix: 增加高度下限约束 * test: fix 单测
Walkthrough本次变更发布 3.0.18-cpp 版本:更新 CHANGELOG 与 package 版本;为 Popup 新增可调整高度(resizable)、最小高度、顶部插槽、触摸事件与 afterShow/afterClose,配套样式、类型、文档、示例与测试调整;优化 Taro Input 的原生输入聚焦/失焦访问;Swiper 在 Taro 端转发 duration。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 用户
participant Popup as Popup(组件)
participant DOM as Popup DOM
participant CB as 回调(Props)
rect rgba(200,220,255,0.2)
note over User,Popup: 打开弹层
User->>Popup: visible=true
Popup->>DOM: 渲染/显示(afterShow)
Popup->>CB: afterShow(el)
end
alt position=bottom AND resizable
rect rgba(200,255,200,0.2)
note over User,Popup: 触摸拖拽调整高度
User-->>Popup: onTouchStart(y0)
Popup->>CB: onTouchStart(height,e)
User-->>Popup: onTouchMove(y)
Popup->>Popup: 计算Δy与方向、限制minHeight
Popup->>DOM: 更新height样式
Popup->>CB: onTouchMove(height,e,dir)
User-->>Popup: onTouchEnd
Popup->>CB: onTouchEnd(finalHeight,e)
end
end
rect rgba(255,220,200,0.2)
note over User,Popup: 关闭弹层
User->>Popup: visible=false
Popup->>DOM: 隐藏/卸载(afterClose)
Popup->>CB: afterClose(el)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x_cpp #3342 +/- ##
================================================
Coverage ? 88.15%
================================================
Files ? 291
Lines ? 19214
Branches ? 2989
================================================
Hits ? 16939
Misses ? 2269
Partials ? 6 ☔ 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/packages/input/input.taro.tsx (1)
193-201: readOnly 未传递给 TaroInput,导致“只读”形同虚设本次将 disabled 从
disabled || readOnly改为仅disabled={disabled}是正确方向,有助于解决“只读不可点击/聚焦”的问题。但当前从 props 解构了readOnly却未向 TaroInput 透传任何只读属性,结果是输入仍可编辑,功能回归严重。建议按环境映射只读属性(WEB 用
readOnly,小程序用readonly),示例补丁:placeholderClass={`${classPrefix}-placeholder`} - disabled={disabled} + {...(getEnv() === ENV_TYPE.WEB ? { readOnly } : { readonly: readOnly })} + disabled={disabled} value={value} focus={autoFocus || focus}说明:
- 仍保持
disabled与readOnly解耦,从而允许只读态可点击/可聚焦。- 透传只读态以禁止编辑,避免业务方误以为只读已生效却能修改值的风险。
src/packages/popup/demos/h5/demo2.tsx (1)
47-51: 与文档不一致:top 弹层不应设置 resizable(文档声明仅 bottom 支持)文档明确 “resizable 当前只支持从底部弹出”,此处给 top 弹层加了
resizable,会造成认知冲突。建议移除或改为在 bottom 示例中展示。首选修复(移除 top 的 resizable,转移到底部弹层):
<Popup visible={showTop} destroyOnClose position="top" - resizable onClose={() => { setShowTop(false) }} /> <Popup visible={showBottom} position="bottom" + resizable onClose={() => { setShowBottom(false) }} />若确实已支持 top 方向的可调高,请同步修订两份文档中对
resizable的说明(去掉“当前只支持从底部弹出”),并补充相应交互的边界与高度计算规则。src/packages/popup/__tests__/popup.spec.tsx (1)
28-33: 点击目标错误:应点击遮罩而非容器当前对 container 触发 click 不能保证命中 Overlay,可能导致假阳性。应直接点击 .nut-overlay 元素。
-test('prop close-on-click-overlay test', async () => { - const { container } = render(<Popup visible closeOnOverlayClick={false} />) - fireEvent.click(container) - const overlay = container.querySelector('.nut-overlay') as HTMLElement - expect(overlay.style.display).toEqual('') -}) +test('prop close-on-click-overlay test', async () => { + const { container } = render(<Popup visible closeOnOverlayClick={false} />) + const overlay = container.querySelector('.nut-overlay') as HTMLElement + fireEvent.click(overlay) + // 期望不关闭:overlay 仍可见(未进入退出态) + expect(overlay.className).not.toContain('nut-overlay-slide-exit') +})src/packages/popup/doc.en-US.md (1)
125-125: 排版/用词问题:应为 “close icon’s animation duration”当前为 “lose icon's animation duration”,存在拼写问题。作为对外文档,建议及时修正。
-| --nutui-popup-animation-duration | lose icon's animation duration | `0.3s` | +| --nutui-popup-animation-duration | close icon's animation duration | `0.3s` |
🧹 Nitpick comments (28)
src/packages/input/input.taro.tsx (5)
80-88: 强化 getNativeInput 的健壮性与类型声明,避免 WEB 端取不到原生 input
- 当前在 WEB 下假设 ref 指向外层容器并去 querySelector('input'),若 ref 已经就是原生 input,将返回 null,导致后续 focus/blur 失效。
- 建议:增加返回类型、空指针保护,并在 querySelector 失败时回退到 ref;同时统一使用 getEnv/ENV_TYPE 以与文件其他处保持一致。
应用如下补丁可消除上述隐患:
- const getNativeInput = () => { - if (Taro.getEnv() === 'WEB') { - const taroInputCoreEl = inputRef.current as HTMLElement - const inputEl = taroInputCoreEl.querySelector('input') - return inputEl - } - return inputRef.current - } + const getNativeInput = (): HTMLInputElement | null => { + if (getEnv() === ENV_TYPE.WEB) { + const coreEl = inputRef.current as unknown as HTMLElement | null + const inputEl = coreEl?.querySelector?.('input') as HTMLInputElement | null + // 兜底:当 ref 已是原生 input 时,直接返回它 + return inputEl ?? (inputRef.current as unknown as HTMLInputElement | null) + } + return (inputRef.current as unknown as HTMLInputElement) ?? null + }
95-102: 非 WEB 端的 ref.focus/blur 往往无效,建议增加“受控 focus”兜底以覆盖小程序端在小程序环境 ref 往往不是原生 input,直接调用 focus/blur 多数无效。建议在找不到原生 input 时,回退到通过受控属性 focus 触发。
针对本段可做如下最小改动:
focus: () => { - const nativeInput = getNativeInput() - nativeInput?.focus() + const nativeInput = getNativeInput() + if (nativeInput) { + nativeInput.focus() + } else { + // 兜底:通过受控属性触发小程序端聚焦 + setForceFocus(true) + } }, blur: () => { - const nativeInput = getNativeInput() - nativeInput?.blur() + const nativeInput = getNativeInput() + if (nativeInput) { + nativeInput.blur() + } + setForceFocus(false) },并在组件其他位置补充(非 diff,仅支持上述改动):
// 在 useState 声明附近增加一行 const [forceFocus, setForceFocus] = useState(false) // 在 TaroInput 上将 focus 改为 focus={autoFocus || focus || forceFocus}请在 Weapp/Alipay/TT 三端验证 ref.focus()/blur() 的行为是否按预期生效。
109-118: useCallback 依赖不完整,readOnly/plain 变化不会刷新样式
inputClass使用了disabled/readOnly/plain,但依赖数组仅包含disabled,当只读或朴素样式变更时,类名不会更新。建议将依赖更新为:
}, [disabled, readOnly, plain])
103-106: nativeElement 与 focus/blur 取值不一致,建议返回真实原生 input当前对外暴露的
nativeElement返回的是inputRef.current,而focus/blur使用的是getNativeInput()。为一致性与可预期行为,建议同步返回真实原生 input(若存在),否则回退到 ref。可改为:
get nativeElement() { return getNativeInput() ?? inputRef.current }
141-158: 统一环境判断方式,减少分支不一致导致的行为偏差文件中混用
Taro.getEnv()与getEnv()/ENV_TYPE。建议统一使用后一种写法(本文件已引入getEnv与ENV_TYPE),便于静态检查与 tree-shaking。Also applies to: 161-169
CHANGELOG.md (1)
1-12: v3.0.18 缺少与本 PR 直接相关的条目(Popup 新增属性、Input 只读可点击/聚焦修复)为便于回溯,请在 v3.0.18 段落补充本次 PR 的功能与修复项。
建议追加如下两条(位置与风格与现有条目保持一致):
# v3.0.18 `2025-08-15` - 🏡 chore: 升级icon库 (#3330) - 🏡 chore: 发布taro下的样式按需插件 - 📖 docs: 更新介绍部分内容 (#3324) +- :sparkles: feat(popup): 新增 resizable/minHeight/top 与触摸事件回调 onTouchStart/onTouchMove/onTouchEnd +- :bug: fix(input): 修复 readonly 输入框无法点击与聚焦的问题 - :sparkles: feat: Ellipsis校验越界不走缓存配置 (#3329) - :sparkles: feat(price): 支持自定义颜色&数据原样输出 (#3328) - :sparkles: feat(notify): 支持promise调用notice (#3319) - :bug: fix(noticebar): 适配鸿蒙样式修复 (#3332) - :bug: Fix icons svg (#3331)如需我直接提交补充条目,请告知我对应的 issue/PR 编号与描述用语。
src/packages/popup/doc.md (2)
98-99: 修正 portal 类型标注的反引号与管道转义当前单元存在反引号错配与 union 写法不一致,建议统一为行内代码包裹、并转义管道。
可直接应用:
-| portal | 指定节点挂载 | `HTMLElement` \| `(() => HTMLElement)` \| null` | `null` | +| portal | 指定节点挂载 | `HTMLElement \| (() => HTMLElement) \| null` | `null` |
82-82: 补充 lockScroll 的字面量取值书写,避免歧义文档中写为
boolean\|strict,建议将strict标注为字符串字面量,且与 boolean 之间保留空格并转义管道。建议改为:
-| lockScroll | 背景是否锁定,strict 用于支持 iOS12 | `boolean\|strict` | `true` | +| lockScroll | 背景是否锁定,'strict' 用于支持 iOS12 | `boolean \| 'strict'` | `true` |src/packages/popup/doc.taro.md (2)
104-104: 统一用字:繁体“頂部佔位”建议改为简体“顶部占位”本页其余内容为简体中文,建议保持一致。
-| top | 頂部佔位 | `ReactNode` | `-` | +| top | 顶部占位 | `ReactNode` | `-` |
108-108: 修正 portal 类型单元的反引号与 union 写法当前为
HTMLElement` \| `(() => HTMLElement)` \| null,存在嵌套反引号错误。-| portal | 指定节点挂载 | ``HTMLElement` \| `(() => HTMLElement)` \| null`` | `null` | +| portal | 指定节点挂载 | `HTMLElement \| (() => HTMLElement) \| null` | `null` |src/sites/sites-react/doc/docs/taro/migrate-from-v2.md (1)
97-103: Popup 小节用语与信息密度可再完善(事件“方法”→“回调”,补充签名与空格规范)
- “title上侧”建议在中英文之间留空格:title 上侧。
- “onTouchStart、onTouchMove、onTouchEnd 方法”更准确应表述为“触摸事件回调”,并建议在迁移文档里直接给出简要签名,降低理解成本。
- “用于底部弹出时,可上下滑动”可更精确为“仅 bottom 位置生效,支持拖拽调整高度”。
可按下列 diff 精简并增强可读性(仅替换该段落的四条 bullet):
- - 新增属性 resizable,用于底部弹出时,可上下滑动 - - 新增属性 minHeight,用于设置最小高度,可搭配 resizable 使用 - - 新增属性 top,用于在title上侧展示用户自定义内容 - - 新增 onTouchStart、onTouchMove、onTouchEnd 方法 + - 新增属性 `resizable`(仅 `position="bottom"` 生效),支持拖拽上下调整弹窗高度 + - 新增属性 `minHeight`,用于设置最小高度(可与 `resizable` 搭配) + - 新增属性 `top`,用于在 title 上侧展示自定义内容 + - 新增触摸事件回调:onTouchStart(height, e)、onTouchMove(height, e, direction)、onTouchEnd(height, e)请确认对外文档是否在 API 表格中标注了
resizable的默认值(通常为 false)与minHeight的单位/示例值;如未标注,建议补充。src/sites/sites-react/doc/docs/react/migrate-from-v2.en-US.md (1)
97-103: Rephrase “Added ...” repetition; prefer “prop” and include callback signatures
- Avoid repetitive “Added ...” openings (LanguageTool 提示);统一使用 “prop”。
- Clarify bottom-only scope for resizable and add concise callback signatures.
Proposed diff for this bullet list:
- - Added the resizable property for scrolling up and down when the bottom popup is active. - - Added the minHeight property for setting the minimum height, which can be used with resizable. - - Added a new attribute top to display user-defined content above the title. - - Added the onTouchStart, onTouchMove, and onTouchEnd methods. + - New prop `resizable` (bottom position only): enables vertical drag to resize. + - New prop `minHeight`: sets the minimum height; use together with `resizable`. + - New prop `top`: render custom content above the title. + - New touch callbacks: onTouchStart(height, e), onTouchMove(height, e, direction), onTouchEnd(height, e).建议在 API 表格中同时补充默认值(e.g.
resizable: false)与minHeight的单位和示例值,确保与实现一致。src/sites/sites-react/doc/docs/taro/migrate-from-v2.en-US.md (1)
97-103: Taro 迁移文档与 React 英文版措辞保持一致,减少歧义同上,建议消除 “Added ...” 重复,并明确回调签名与 bottom-only 约束。建议应用相同修改:
- - Added the resizable property for scrolling up and down when the bottom popup is active. - - Added the minHeight property for setting the minimum height, which can be used with resizable. - - Added a new attribute top to display user-defined content above the title. - - Added the onTouchStart, onTouchMove, and onTouchEnd methods. + - New prop `resizable` (bottom position only): enables vertical drag to resize. + - New prop `minHeight`: sets the minimum height; use together with `resizable`. + - New prop `top`: render custom content above the title. + - New touch callbacks: onTouchStart(height, e), onTouchMove(height, e, direction), onTouchEnd(height, e).请同步检查 taro 版 API 表格的默认值与参数签名是否与实现一致(尤其
direction仅存在于 onTouchMove)。src/sites/sites-react/doc/docs/react/migrate-from-v2.md (1)
97-103: 中文迁移文档:统一术语与空格,补充回调签名与 taro 中文文档同样问题,建议统一术语为“触摸事件回调”,并补充签名;中英文混排加空格。
- - 新增属性 resizable,用于底部弹出时,可上下滑动 - - 新增属性 minHeight,用于设置最小高度,可搭配 resizable 使用 - - 新增属性 top,用于在title上侧展示用户自定义内容 - - 新增 onTouchStart、onTouchMove、onTouchEnd 方法 + - 新增属性 `resizable`(仅 `position="bottom"` 生效),支持拖拽上下调整弹窗高度 + - 新增属性 `minHeight`,用于设置最小高度(可与 `resizable` 搭配) + - 新增属性 `top`,用于在 title 上侧展示自定义内容 + - 新增触摸事件回调:onTouchStart(height, e)、onTouchMove(height, e, direction)、onTouchEnd(height, e)建议在 API 表格中明确:
direction取值'up' | 'down',且仅 onTouchMove 提供。src/packages/popup/demos/h5/demo1.tsx (2)
6-6: 命名拼写错误:showPopupResiable 建议更正为 showPopupResizable避免拼写错误在示例与文档中传播,统一命名可提升可读性与可搜索性。
- const [showPopupResiable, setShowPopupResiable] = useState(false) + const [showPopupResizable, setShowPopupResizable] = useState(false) @@ - onClick={() => { - setShowPopupResiable(true) - }} + onClick={() => { + setShowPopupResizable(true) + }} @@ - visible={showPopupResiable} + visible={showPopupResizable} @@ - onClose={() => { - setShowPopupResiable(false) - }} + onClose={() => { + setShowPopupResizable(false) + }}Also applies to: 17-21, 33-53
44-51: 演示事件日志 OK,但建议在发布示例站点前去除 console.log示例中大量 console.log 可能干扰使用者调试台,建议换成注释说明或在文档中描述回调签名。
src/packages/popup/demos/taro/demo1.tsx (2)
6-6: 命名拼写错误:showPopupResiable 建议更正为 showPopupResizable与 H5 示例保持一致,避免拼写错误带来的误导。
- const [showPopupResiable, setShowPopupResiable] = useState(false) + const [showPopupResizable, setShowPopupResizable] = useState(false) @@ - onClick={() => { - setShowPopupResiable(true) - }} + onClick={() => { + setShowPopupResizable(true) + }} @@ - visible={showPopupResiable} + visible={showPopupResizable} @@ - onClose={() => { - setShowPopupResiable(false) - }} + onClose={() => { + setShowPopupResizable(false) + }}Also applies to: 17-21, 33-53
44-51: 演示事件日志 OK,但建议在发布示例站点前去除 console.log与 H5 示例相同建议。
src/packages/popup/__tests__/popup.spec.tsx (4)
11-20: 测试名与断言不符:仅验证了“打开”,未验证“关闭”要么重命名测试,要么补一段关闭断言,建议后者以提升回归价值。
test('opens and closes correctly', () => { const { rerender } = render(<Popup visible={false}>Test Content</Popup>) @@ // Rerender with visible true rerender(<Popup visible>Test Content</Popup>) expect(screen.getByText('Test Content')).toBeInTheDocument() + + // Rerender with visible false (close) + rerender(<Popup visible={false}>Test Content</Popup>) + expect(screen.queryByText('Test Content')).not.toBeInTheDocument() })
109-114: 异步断言易抖动:建议使用 waitFor 包裹动画/过渡类名的添加通常是异步的,直接断言可能偶发失败。
- await fireEvent.click(overlay) - expect(overlay).toHaveClass('nut-overlay-slide-exit') + await fireEvent.click(overlay) + await waitFor(() => { + expect(overlay).toHaveClass('nut-overlay-slide-exit') + })
116-128: 同上:过渡类断言建议 waitFor保证测试稳定性,减少 CI 偶发现象。
- fireEvent.click(closeIcon) - expect(onCloseIconClick).toBeCalled() - expect(overlay).toHaveClass('nut-overlay-slide-exit') + fireEvent.click(closeIcon) + expect(onCloseIconClick).toBeCalled() + await waitFor(() => { + expect(overlay).toHaveClass('nut-overlay-slide-exit') + })
175-208: 触摸事件用例已覆盖,但可进一步验证方向与高度参数当前仅校验回调被触发,建议断言回调入参(高度为 number、方向 'up'/'down')以提高回归信心。
示例增强(非必须):
- fireEvent.touchStart(popup, { touches: [{ pageY: 400 }] }) - expect(handleTouchStart).toHaveBeenCalled() + fireEvent.touchStart(popup, { touches: [{ pageY: 400 }] }) + expect(handleTouchStart).toHaveBeenCalledWith(expect.any(Number), expect.any(Object)) @@ - fireEvent.touchMove(popup, { touches: [{ pageY: 50 }] }) - expect(handleTouchMove).toHaveBeenCalled() + fireEvent.touchMove(popup, { touches: [{ pageY: 50 }] }) + expect(handleTouchMove).toHaveBeenCalledWith(expect.any(Number), expect.any(Object), 'up') @@ - fireEvent.touchMove(popup, { touches: [{ pageY: 450 }] }) - expect(handleTouchMove).toHaveBeenCalled() + fireEvent.touchMove(popup, { touches: [{ pageY: 450 }] }) + expect(handleTouchMove).toHaveBeenCalledWith(expect.any(Number), expect.any(Object), 'down')src/packages/popup/popup.taro.tsx (1)
361-368: 死代碼:transitionName 在 Taro 端未被使用
transitionName僅為 Web 端轉場所需,Taro 端已移除 CSSTransition。建議刪除相關 state 與 effect,保持代碼整潔。- const [transitionName, setTransitionName] = useState('') @@ - useEffect(() => { - setTransitionName(transition || `${classPrefix}-slide-${position}`) - }, [position, transition])src/packages/popup/popup.tsx (3)
231-256: 用 requestAnimationFrame 合并样式写入,降低抖动与回流风险连续在 touchmove 中直接写 style.height 容易造成掉帧。建议以 rAF 合并写入并在 touchend 取消。
应用以下改动:
@@ - const touchStartRef = useRef(0) + const touchStartRef = useRef(0) const touchMoveDistanceRef = useRef(0) const heightRef = useRef(0) // 首次可调整时记录的默认高度 const defaultHeightRef = useRef(0) - const isTouching = useRef(false) + const isTouching = useRef(false) + const rafIdRef = useRef<number | null>(null) @@ - nodeRef.current.style.height = `${currentHeight}px` + if (rafIdRef.current) cancelAnimationFrame(rafIdRef.current) + rafIdRef.current = requestAnimationFrame(() => { + if (!nodeRef.current) return + nodeRef.current.style.height = `${currentHeight}px` + }) @@ - onTouchEnd?.(currentHeight, event) + if (rafIdRef.current) { + cancelAnimationFrame(rafIdRef.current) + rafIdRef.current = null + } + onTouchEnd?.(currentHeight, event)可选的清理(组件卸载时):
// 额外代码块(可选) useEffect(() => { return () => { if (rafIdRef.current) cancelAnimationFrame(rafIdRef.current) } }, [])Also applies to: 258-270, 98-104
231-236: 当 lockScroll=true 时建议在 touchmove 主动 preventDefault,彻底阻断穿透虽然全局的 useLockScroll 已经尽量阻断滚动,但在组件内的 resize 手势过程中,仍可能在部分设备上发生滚动穿透或橡皮筋回弹。可在可取消的事件上补充 preventDefault 作为兜底。
const handleTouchMove = (event: TouchEvent<HTMLDivElement>) => { if (position !== 'bottom' || !resizable || !nodeRef.current) return - event.stopPropagation() + event.stopPropagation() + if (lockScroll && event.cancelable) { + event.preventDefault() + }
103-104: isTouching 未被读取,建议移除以减轻心智负担该 ref 仅赋值未使用,建议删除。
- const isTouching = useRef(false) @@ - // 标记开始滑动 - isTouching.current = true @@ - isTouching.current = falseAlso applies to: 224-225, 260-261
src/packages/popup/doc.en-US.md (2)
106-108: 触摸回调的高度单位说明建议补充当前签名为 height: number,但未说明是“像素”。鉴于实现中会有单位换算(%/vh → px),建议在文档明确 height 单位为“px”,并说明 direction 枚举含义(up/down)。
可在本行描述中追加 “height in px”。
99-100: afterShow/afterClose 的参数说明建议贴近实际回调签名这里标注为
event: HTMLElement。在实现中,afterShow 透传给了 CSSTransition 的 onEntered,该回调签名为 (node: HTMLElement, isAppearing: boolean)。虽然多出的参数会被忽略,但为避免误导,建议在文档中注明可能存在第二个参数 isAppearing(或仅描述为 node: HTMLElement)。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
CHANGELOG.md(1 hunks)package.json(1 hunks)src/packages/input/input.taro.tsx(2 hunks)src/packages/popup/__tests__/popup.spec.tsx(4 hunks)src/packages/popup/demos/h5/demo1.tsx(1 hunks)src/packages/popup/demos/h5/demo2.tsx(1 hunks)src/packages/popup/demos/taro/demo1.tsx(1 hunks)src/packages/popup/doc.en-US.md(1 hunks)src/packages/popup/doc.md(1 hunks)src/packages/popup/doc.taro.md(1 hunks)src/packages/popup/doc.zh-TW.md(1 hunks)src/packages/popup/popup.scss(0 hunks)src/packages/popup/popup.taro.tsx(6 hunks)src/packages/popup/popup.tsx(8 hunks)src/packages/swiper/swiper.taro.tsx(0 hunks)src/sites/sites-react/doc/docs/react/migrate-from-v2.en-US.md(1 hunks)src/sites/sites-react/doc/docs/react/migrate-from-v2.md(1 hunks)src/sites/sites-react/doc/docs/taro/migrate-from-v2.en-US.md(1 hunks)src/sites/sites-react/doc/docs/taro/migrate-from-v2.md(1 hunks)src/types/spec/popup/base.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/packages/popup/popup.scss
- src/packages/swiper/swiper.taro.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-25T02:21:32.906Z
Learnt from: xiaoyatong
PR: jdf2e/nutui-react#2990
File: src/packages/pickerview/__test__/pickerview.spec.tsx:0-0
Timestamp: 2025-02-25T02:21:32.906Z
Learning: In React component tests, avoid using setTimeout with fixed delays. Instead, use act() to wrap async state updates and waitFor() for assertions, which makes tests more stable and reliable.
Applied to files:
src/packages/popup/__tests__/popup.spec.tsx
🧬 Code graph analysis (5)
src/packages/popup/demos/taro/demo1.tsx (2)
src/packages/popup/popup.taro.tsx (1)
Popup(50-382)src/packages/popup/popup.tsx (1)
Popup(47-343)
src/packages/popup/demos/h5/demo1.tsx (2)
src/packages/popup/popup.taro.tsx (1)
Popup(50-382)src/packages/popup/popup.tsx (1)
Popup(47-343)
src/packages/popup/popup.tsx (2)
src/packages/popup/popup.taro.tsx (1)
Popup(50-382)src/hooks/use-lock-scroll.ts (1)
useLockScroll(25-108)
src/packages/popup/__tests__/popup.spec.tsx (2)
src/packages/popup/popup.taro.tsx (1)
Popup(50-382)src/packages/popup/popup.tsx (1)
Popup(47-343)
src/packages/popup/popup.taro.tsx (4)
src/packages/popup/popup.tsx (1)
Popup(47-343)src/utils/taro/platform.ts (1)
harmony(3-7)src/utils/taro/px-transform.ts (1)
pxTransform(5-9)src/utils/taro/get-rect.ts (1)
getRectInMultiPlatformWithoutCache(58-76)
🪛 LanguageTool
src/sites/sites-react/doc/docs/taro/migrate-from-v2.en-US.md
[style] ~101-~101: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ht, which can be used with resizable. - Added a new attribute top to display user-def...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~101-~101: There might be a mistake here.
Context: ...ay user-defined content above the title. - Added the onTouchStart, onTouchMove, and...
(QB_NEW_EN)
[style] ~102-~102: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...user-defined content above the title. - Added the onTouchStart, onTouchMove, and onTo...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
src/sites/sites-react/doc/docs/react/migrate-from-v2.en-US.md
[style] ~101-~101: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ht, which can be used with resizable. - Added a new attribute top to display user-def...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~101-~101: There might be a mistake here.
Context: ...ay user-defined content above the title. - Added the onTouchStart, onTouchMove, and...
(QB_NEW_EN)
[style] ~102-~102: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...user-defined content above the title. - Added the onTouchStart, onTouchMove, and onTo...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
CHANGELOG.md
[grammar] ~6-~6: There might be a mistake here.
Context: ...5-08-15` - 🏡 chore: 升级icon库 (#3330) - 🏡 chore: 发布taro下的样式按需插件 - 📖 docs: 更新介绍部分...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ... (#3330) - 🏡 chore: 发布taro下的样式按需插件 - 📖 docs: 更新介绍部分内容 (#3324) - ✨ feat...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...的样式按需插件 - 📖 docs: 更新介绍部分内容 (#3324) - ✨ feat: Ellipsis校验越界不走缓存配置 (#3329)...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...s: feat: Ellipsis校验越界不走缓存配置 (#3329) - ✨ feat(price): 支持自定义颜色&数据原样输出 (#33...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...feat(price): 支持自定义颜色&数据原样输出 (#3328) - ✨ feat(notify): 支持promise调用notice ...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...(notify): 支持promise调用notice (#3319) - 🐛 fix(noticebar): 适配鸿蒙样式修复 (#3332) - :b...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...g: fix(noticebar): 适配鸿蒙样式修复 (#3332) - 🐛 Fix icons svg (#3331) # v3.0.17 `20...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
src/packages/popup/doc.taro.md
113-113: Link fragments should be valid
(MD051, link-fragments)
🔇 Additional comments (17)
src/packages/popup/doc.md (3)
90-95: API 增强说明清晰,注意与演示代码保持“仅 bottom 可调高”一致
resizable的限制已在文档中明确“当前只支持从底部弹出”。请确保所有 demo 未在非 bottom 位置使用该属性(当前 h5/demo2.tsx 中 top 弹层使用了 resizable,详见对应评论)。
99-101: afterShow/afterClose 事件参数类型请再次核对这里记录为
event: HTMLElement。如果 Overlay 在部分运行时(例如小程序端)并非返回 HTMLElement,建议在 Web 文档与 Taro 文档中分别标注或补充说明以避免歧义。我可以据 types 源码自动提取签名并回填到文档表格中,是否需要我生成脚本来同步类型到文档?
106-108: 触摸回调签名与方向枚举描述清晰,LGTM
(height, event, direction: 'up' | 'down')的语义与顺序合理,易于消费。src/packages/popup/doc.taro.md (2)
100-102: 与 Web 文档一致性良好,强调“仅 bottom 可调高”无误Taro 文档与 Web 文档在
resizable/minHeight的说明保持一致,LGTM。
126-135: 站内链接片段需校验静态检查提示 “Link fragments should be valid (MD051)”。请确认
#/zh-CN/component/configprovider在站点构建后能正确跳转;若不合法,按站点路由规范修订锚点。src/types/spec/popup/base.ts (2)
20-20:top?: ReactNode命名与语义 OK字段为可选且语义清晰,能与现有
title/left协同使用,无冲突点。
31-33: 无需调整 onTouch 回调当前
src/types/spec/popup/base.ts中的onTouchMove、onTouchStart和onTouchEnd均定义在BasePopup接口内,其派生接口WebPopupProps与TaroPopupProps仅继承自BasePopup,并未与React.HTMLAttributes发生合并或继承,也没有出现通过Omit<…, 'onTouchStart' | 'onTouchMove' | 'onTouchEnd'>来剔除原生同名属性的实现(脚本搜索未发现相关用例)。因此,原 review 中提及的“已在组件层通过 Omit 规避重名冲突”并不属实,也无需为此对类型做额外处理。· 检索结果显示:
- 未在任何
.tsx文件中找到对Omit<…, 'onTouchStart' | 'onTouchMove' | 'onTouchEnd'>的引用WebPopupProps与TaroPopupProps均仅继承自BasePopup,并无合并React.HTMLAttributes- 仅在
src/types/spec/popup/base.ts中看到三处必填回调声明,设计上若需可再评估是否改为可选建议:当前无需修改此处类型定义,若后续组件确实需要同时透传原生事件,可在组件层额外使用 Omit 或者采用重命名策略再做兼容。
Likely an incorrect or invalid review comment.
src/packages/popup/__tests__/popup.spec.tsx (2)
6-9: 基础渲染冒烟测试 LGTM覆盖了最关键的可见性与内容挂载路径。
144-153: onOpen 触发用例 LGTM使用 rerender + waitFor 更贴近真实生命周期。
src/packages/popup/popup.taro.tsx (2)
228-242: 事件回調設計整體 OK拖拽開始/移動/結束的參數設計(當前高度、事件、方向)合理,Taro 端利用
pxTransform動態計算高度也對齊了容器渲染。Also applies to: 244-278, 280-299
99-103: 请确认 useLockScrollTaro 在所有 Taro 端返回的 ref 均具备预期的 current、uid 和 offsetHeight 属性
useLockScrollTaro实现中无论环境如何,均以useRef(null)创建并返回一个refObject,仅在WEB环境下调用useLockScroll。在非 Web(小程序、RN 等)环境,虽然refObject.current会被赋值为<View>的组件实例,但并不保证该实例一定包含.uid或.offsetHeight,后续对其测量和样式操作可能产生undefined。- 请在以下位置验证并补充兼容性处理:
• src/packages/popup/popup.taro.tsx (第 99–103 行)
• src/packages/popup/popup.taro.tsx (第 304–317 行,同样逻辑)- 若部分平台不支持直接从
ref.current获取这些属性,可考虑使用 Taro 的createSelectorQuery或组件实例提供的 API 来获取元素尺寸/标识,避免运行时异常。src/packages/popup/popup.tsx (5)
48-53: 类型层面规避原生触摸事件冲突,设计合理通过 Omit 从 HTMLAttributes 中排除了 title/onTouchStart/onTouchMove/onTouchEnd,避免与组件自定义同名 props 的类型冲突,👍。
33-35: minHeight 的默认值与文档不一致,请统一代码默认值为空字符串,但文档(doc.en-US.md)声明默认值为 26%。两端不一致会造成用户困惑,且影响可预测性。
可选方案:
- 方案 A(对齐文档):将默认值改为 '26%'。
- 方案 B(对齐代码):将文档默认值改为 '-' 或明确“仅在 resizable=true 下建议设置,默认不限制”。
建议(方案 A):
round: false, resizable: false, - minHeight: '', + minHeight: '26%', onOpen: () => {},如担心对非 bottom 位的样式影响,也可仅在 position === 'bottom' 且 resizable 时才把 minHeight 写入 style(可后续再做微调)。
219-229: 默认高度记录逻辑 OK,但建议注明“默认高度 = 第一次触摸开始时的高度”该策略在 reopen 时通过 open() 重置有一致性。只是当用户在展开后才首次触摸时才会记录 defaultHeight,符合直觉。无须修改,仅建议在注释里说明。
129-136: reopen 时复位高度的条件判断请确认是否覆盖所有路径当前判断依赖 heightRef.current 是否为真。一旦用户曾经进行过一次拖拽(哪怕是恢复到原高度),heightRef.current 将保持非零,后续 reopen 都会强制复位到 defaultHeightRef.current。请确认这与预期一致,特别是期望“记住上次高度”还是“每次重开回到初始高度”。
如果想“记住上次高度”,可在 close 时把 defaultHeightRef.current 更新为当前高度;如果“每次重开回初始”,则现在逻辑已满足。
294-297: 新增 top 插槽渲染位置合理在标题之前插入顶部内容,满足“底部抽屉顶部区域”的自定义需求,👍。
src/packages/popup/doc.en-US.md (1)
90-92: 文档默认值与实现不一致:minHeight 默认写成了 26%组件实现的默认值目前为 ''(空),而文档为 26%。请与实现对齐,以免用户得出错误预期。
可选方案同代码评审中的两种(修改实现为 '26%',或修改文档为 '-' 并注明适用条件)。若采用 '26%',建议在文档补充说明“仅当 position='bottom' 且 resizable 为 true 时推荐设置/生效”。
| { | ||
| "name": "@nutui/nutui-react-taro", | ||
| "version": "3.0.18-cpp.beta.0", | ||
| "version": "3.0.18-cpp", |
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.
💡 Verification agent
🧩 Analysis chain
版本号提升到 3.0.18-cpp,请同步校验发版信息一致性
- 当前 package.json 使用了带渠道后缀的版本号
3.0.18-cpp,而 CHANGELOG 顶部条目为v3.0.18。请确认是否需要在 CHANGELOG 或发布标签中体现-cpp渠道,以避免包版本与发布记录不一致。
可以用以下命令快速检查并统一:
🏁 Script executed:
#!/bin/bash
# 1) 检查 CHANGELOG 是否存在对应条目
rg -n "^# v3\.0\.18(\-cpp)?$" CHANGELOG.md
# 2) 如果要保留 -cpp 渠道,请在 CHANGELOG 顶部标题同步为 v3.0.18-cpp;否则请将 package.json 恢复为 3.0.18Length of output: 56
请同步 package.json 与 CHANGELOG.md 的版本号后缀
当前存在不一致:
- package.json 第3行:
"version": "3.0.18-cpp" - CHANGELOG.md 第1行:
# v3.0.18
请根据发布渠道需求,二选一并保持一致:
- 如果要保留
-cpp渠道后缀:- 在 CHANGELOG.md 顶部,将
# v3.0.18修改为# v3.0.18-cpp
- 在 CHANGELOG.md 顶部,将
- 如果无需渠道后缀:
- 在 package.json 中,将版本回退为
"3.0.18"
- 在 package.json 中,将版本回退为
🤖 Prompt for AI Agents
In package.json around lines 3 to 3, the version string ("3.0.18-cpp") and
CHANGELOG.md top heading ("# v3.0.18") differ; choose one of the two options and
make them consistent: either update CHANGELOG.md first line to "# v3.0.18-cpp"
to keep the -cpp release channel suffix, or change package.json version to
"3.0.18" to remove the suffix; apply the chosen change and verify both files now
contain the exact same version text.
| | afterShow | 继承于`Overlay`, 遮罩打开动画结束时触发 | `event: HTMLElement` | `-` | | ||
| | afterClose | 继承于`Overlay`, 遮罩关闭动画结束时触发 | `event: HTMLElement` | `-` | |
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.
💡 Verification agent
🧩 Analysis chain
afterShow/afterClose 的事件类型在 Taro 多端需再确认
如果在小程序端没有 HTMLElement 概念,建议注明“仅 H5 返回 HTMLElement”,或改为更通用的 unknown/不带参,并在示例中说明。
可用脚本辅助核对 Overlay 的类型定义来源:
🏁 Script executed:
#!/bin/bash
# 搜索 Overlay 的 afterShow/afterClose 类型定义
rg -nP "(afterShow|afterClose)\s*[:(]" -C2 src | sed -n '1,120p'Length of output: 1111
修正 afterShow/afterClose 参数类型
根据 src/types/spec/overlay/base.ts 中的定义,afterShow 和 afterClose 均为无参回调:
afterShow: () => void
afterClose: () => void 文档中使用了 event: HTMLElement 与代码实现不符,需将类型改为无参并移除 HTMLElement 参数。
请在 src/packages/popup/doc.taro.md 中做如下更新:
• 文件:src/packages/popup/doc.taro.md
位置:约第 109–110 行
原有表格行:
-| afterShow | 继承于`Overlay`, 遮罩打开动画结束时触发 | `event: HTMLElement` | `-` |
-| afterClose | 继承于`Overlay`, 遮罩关闭动画结束时触发 | `event: HTMLElement` | `-` |建议修改为:
| afterShow | 继承于`Overlay`,遮罩打开动画结束时触发 | `() => void` | `-` |
| afterClose | 继承于`Overlay`,遮罩关闭动画结束时触发 | `() => void` | `-` |若未来仅 H5 端支持获取 HTMLElement,可在示例或备注中说明“仅 H5 支持传入或获取对应 DOM 元素”。
🤖 Prompt for AI Agents
In src/packages/popup/doc.taro.md around lines 109–110, the docs list afterShow
and afterClose as taking an `event: HTMLElement` but the actual types in
src/types/spec/overlay/base.ts define them as no-argument callbacks; update the
two table rows so the type column shows `() => void` (remove `event:
HTMLElement`) and adjust the description to indicate they are parameterless
callbacks; optionally add a short note or example remark that only H5 may
provide a DOM element if applicable.
| | resizable | 上下滑動調整高度,目前只支援從底部彈出 | `boolean` | `false` | | ||
| | minHeight | 設定最小高度 | `string` | `26%` | | ||
| | left | 标题左侧部分 | `ReactNode` | `-` | |
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.
minHeight 預設值與實現不一致
文檔標示預設值為 26%,但程式碼(web 與 taro)預設為空字串/未設定。請將預設值改為 - 或在 defaultProps 中統一補上,以避免使用者誤解。
-| minHeight | 設定最小高度 | `string` | `26%` |
+| minHeight | 設定最小高度 | `string \| number` | `-` |📝 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.
| | resizable | 上下滑動調整高度,目前只支援從底部彈出 | `boolean` | `false` | | |
| | minHeight | 設定最小高度 | `string` | `26%` | | |
| | left | 标题左侧部分 | `ReactNode` | `-` | | |
| | resizable | 上下滑動調整高度,目前只支援從底部彈出 | `boolean` | `false` | | |
| | minHeight | 設定最小高度 | `string \| number` | `-` | | |
| | left | 标题左侧部分 | `ReactNode` | `-` | |
🤖 Prompt for AI Agents
In src/packages/popup/doc.zh-TW.md around lines 90 to 92, the docs state
minHeight default is `26%` while the actual web and taro implementations default
to an empty string/unset; update the code to match the docs or update the docs
to reflect reality. Either set a consistent default in the components
(add/modify defaultProps or default parameter to `'26%'` in both web and taro
implementations) or change the markdown table to show `-` as the default; ensure
both implementations and docs are consistent.
| | afterShow | 继承于`Overlay`, 遮罩打開動畫結束時觸發 | `event: HTMLElement` | `-` | | ||
| | afterClose | 继承于`Overlay`, 遮罩關閉動畫結束時觸發 | `event: HTMLElement` | `-` | | ||
| | onClick | 點擊彈框時觸發 | `event: MouseEvent` | `-` | | ||
| | onCloseIconClick | 點擊關閉圖標時觸發 | `event: MouseEvent` | `-` | | ||
| | onOpen | 打開彈框時觸發 | `-` | `-` | | ||
| | onClose | 關閉彈框時觸發 | `-` | `-` | | ||
| | afterShow | 继承于`Overlay`, 遮罩打開動畫結束時觸發 | `event: HTMLElement` | `-` | | ||
| | afterClose | 继承于`Overlay`, 遮罩關閉動畫結束時觸發 | `event: HTMLElement` | `-` | | ||
| | onOverlayClick | 點擊遮罩觸發 | `event: MouseEvent` | `-` | | ||
| | onTouchStart | 開始觸碰時觸發 | `(height: number, event: TouchEvent<HTMLDivElement>) => void` | `-` | | ||
| | onTouchMove | 滑動時觸發 | `(height: number, event: TouchEvent<HTMLDivElement>, 'up' \| 'down') => void` | `-` | | ||
| | onTouchEnd | 結束觸碰時觸發 | `(height: number, event: TouchEvent<HTMLDivElement>) => void` | `-` | |
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.
🛠️ Refactor suggestion
afterShow/afterClose 參數說明不準確 + 建議補齊觸控回調的跨端描述
- 目前 Taro 版本未向下傳遞/觸發 afterShow/afterClose(見 popup.taro.tsx),即便觸發,其參數也非 HTMLElement。建議文檔改為無參或補充“回調不保證帶 DOM 節點”。
- 觸控回調的事件型別在 H5/Taro 端不同,建議型別描述更泛化,或在表格備註跨端差異。
-| afterShow | 继承于`Overlay`, 遮罩打開動畫結束時觸發 | `event: HTMLElement` | `-` |
-| afterClose | 继承于`Overlay`, 遮罩關閉動畫結束時觸發 | `event: HTMLElement` | `-` |
+| afterShow | 继承於 `Overlay`,開啟動畫結束時觸發 | `() => void` | `-` |
+| afterClose | 继承於 `Overlay`,關閉動畫結束時觸發 | `() => void` | `-` |
@@
-| onTouchStart | 開始觸碰時觸發 | `(height: number, event: TouchEvent<HTMLDivElement>) => void` | `-` |
-| onTouchMove | 滑動時觸發 | `(height: number, event: TouchEvent<HTMLDivElement>, 'up' \| 'down') => void` | `-` |
-| onTouchEnd | 結束觸碰時觸發 | `(height: number, event: TouchEvent<HTMLDivElement>) => void` | `-` |
+| onTouchStart | 開始觸碰時觸發 | `(height: number, event: Event) => void` | `-` |
+| onTouchMove | 滑動時觸發 | `(height: number, event: Event, 'up' \| 'down') => void` | `-` |
+| onTouchEnd | 結束觸碰時觸發 | `(height: number, event: Event) => void` | `-` |補充(不在本段差異行,提供參考):left/title 欄位目前為簡體「标题」,建議統一為繁體「標題」。若需要,我可以提 PR 一併修正。
📝 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.
| | afterShow | 继承于`Overlay`, 遮罩打開動畫結束時觸發 | `event: HTMLElement` | `-` | | |
| | afterClose | 继承于`Overlay`, 遮罩關閉動畫結束時觸發 | `event: HTMLElement` | `-` | | |
| | onClick | 點擊彈框時觸發 | `event: MouseEvent` | `-` | | |
| | onCloseIconClick | 點擊關閉圖標時觸發 | `event: MouseEvent` | `-` | | |
| | onOpen | 打開彈框時觸發 | `-` | `-` | | |
| | onClose | 關閉彈框時觸發 | `-` | `-` | | |
| | afterShow | 继承于`Overlay`, 遮罩打開動畫結束時觸發 | `event: HTMLElement` | `-` | | |
| | afterClose | 继承于`Overlay`, 遮罩關閉動畫結束時觸發 | `event: HTMLElement` | `-` | | |
| | onOverlayClick | 點擊遮罩觸發 | `event: MouseEvent` | `-` | | |
| | onTouchStart | 開始觸碰時觸發 | `(height: number, event: TouchEvent<HTMLDivElement>) => void` | `-` | | |
| | onTouchMove | 滑動時觸發 | `(height: number, event: TouchEvent<HTMLDivElement>, 'up' \| 'down') => void` | `-` | | |
| | onTouchEnd | 結束觸碰時觸發 | `(height: number, event: TouchEvent<HTMLDivElement>) => void` | `-` | | |
| | afterShow | 继承於 `Overlay`,開啟動畫結束時觸發 | `() => void` | `-` | | |
| | afterClose | 继承於 `Overlay`,關閉動畫結束時觸發 | `() => void` | `-` | | |
| | onClick | 點擊彈框時觸發 | `event: MouseEvent` | `-` | | |
| | onCloseIconClick | 點擊關閉圖標時觸發 | `event: MouseEvent` | `-` | | |
| | onOpen | 打開彈框時觸發 | `-` | `-` | | |
| | onClose | 關閉彈框時觸發 | `-` | `-` | | |
| | onOverlayClick | 點擊遮罩觸發 | `event: MouseEvent` | `-` | | |
| | onTouchStart | 開始觸碰時觸發 | `(height: number, event: Event) => void` | `-` | | |
| | onTouchMove | 滑動時觸發 | `(height: number, event: Event, 'up' \| 'down') => void` | `-` | | |
| | onTouchEnd | 結束觸碰時觸發 | `(height: number, event: Event) => void` | `-` | |
🤖 Prompt for AI Agents
src/packages/popup/doc.zh-TW.md lines 98-107: afterShow/afterClose currently
claim they pass an HTMLElement but Taro implementation doesn't forward these and
may not provide a DOM node; update the table to state these callbacks have no
guaranteed parameters (use “-” or “回調不保證帶 DOM 節點 / 無參”) or explicitly mark as
optional/implementation-dependent; for touch callbacks
(onTouchStart/onTouchMove/onTouchEnd) replace H5/Taro-specific type signatures
with a generic cross‑platform description (e.g. “(height: number, event:
TouchEvent | unknown) => void” or add a table remark indicating H5 provides DOM
TouchEvent while Taro provides platform touch event shapes), and add a brief
table note about cross‑platform differences; optionally change the simplified
Chinese “标题” in the left/title fields to traditional “標題” as suggested.
| const popStyles = { zIndex: contentZIndex, minHeight, ...style } | ||
| const popClassName = classNames( |
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.
🛠️ Refactor suggestion
高度調整與最小高度單位換算存在問題:百分比被當作像素數字處理
目前 minHeight 若傳入 '10%',在拖動計算中會被 parseInt 解析為 10,等同 10px,與樣式層的 minHeight: '10%' 不一致,導致拖動邏輯與實際渲染不一致。建議統一將 minHeight 轉換為對應像素值(H5 用 window.innerHeight,Taro 用 Taro.getSystemInfoSync().windowHeight)或直接依賴 CSS 的 min-height 而不在 JS 中再做 clamp。
最小改動方案(提取工具函數並替換兩處計算 min 的位置):
+// 建議:頂部加入工具方法(或抽到 utils)
+const toPx = (val: number | string, viewportHeight: number, fallback = 0) => {
+ if (typeof val === 'number') return val
+ const s = String(val).trim()
+ if (s.endsWith('px')) return parseFloat(s)
+ if (s.endsWith('%')) return (parseFloat(s) / 100) * viewportHeight
+ if (s.endsWith('vh')) return (parseFloat(s) / 100) * viewportHeight
+ const n = parseFloat(s)
+ return Number.isFinite(n) ? n : fallback
+}@@ const handleTouchMove: CommonEventFunction = (event) => {
- const min =
- typeof minHeight === 'number'
- ? minHeight
- : parseInt(String(minHeight || 0), 10) || 0
+ const { windowHeight } = Taro.getSystemInfoSync()
+ const min = toPx(minHeight as any, windowHeight, 0)
@@ const handleTouchEnd: CommonEventFunction = (event) => {
- const min =
- typeof minHeight === 'number'
- ? minHeight
- : parseInt(String(minHeight || 0), 10) || 0
+ const { windowHeight } = Taro.getSystemInfoSync()
+ const min = toPx(minHeight as any, windowHeight, 0)如不希望引入系統 API,可刪除 JS 層 clamp,僅依賴樣式層的 min-height 保障:
- const currentHeight = Math.max(
- min,
- heightRef.current - touchMoveDistanceRef.current
- )
+ const currentHeight = heightRef.current - touchMoveDistanceRef.currentAlso applies to: 124-131
🤖 Prompt for AI Agents
In src/packages/popup/popup.taro.tsx around lines 115-116 and 124-131, the JS
drag/clamp logic parses minHeight like '10%' with parseInt (yielding 10px) which
mismatches the DOM minHeight:'10%'; fix by normalizing minHeight before clamp:
implement a small utility that, given minHeight (number or string), returns a
pixel value—if percent string (endsWith '%') compute pixels using runtime window
height (H5: window.innerHeight, Taro: Taro.getSystemInfoSync().windowHeight),
otherwise parse numeric px values—then use that pixel value in both clamp
locations; alternatively, if you prefer minimal change, remove the JS clamp
entirely and rely on CSS min-height (delete the JS min clamp code in both
places) so behavior matches styling.
| const renderPop = () => { | ||
| return ( | ||
| <View | ||
| ref={refObject} | ||
| style={{ ...popStyles, display: innerVisible ? 'block' : 'none' }} | ||
| ref={nodeRef} | ||
| style={{ | ||
| ...popStyles, | ||
| display: innerVisible ? 'block' : 'none', | ||
| ...resizeStyles(), | ||
| }} | ||
| className={popClassName} | ||
| onClick={onClick} | ||
| catchMove={lockScroll} | ||
| onTouchStart={handleTouchStart} | ||
| onTouchMove={handleTouchMove} | ||
| onTouchEnd={handleTouchEnd} | ||
| onTouchCancel={handleTouchEnd} | ||
| > | ||
| {top} | ||
| {renderTitle()} | ||
| {showChildren ? children : null} | ||
| </View> |
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.
🛠️ Refactor suggestion
在 Taro 端未觸發 afterShow/afterClose,與文檔聲明不符
Web 端通過 CSSTransition 的 onEntered/onExited 觸發 afterShow/afterClose,但 Taro 端改為直接渲染 <View> 後未調用這兩個回調,屬於功能缺失。
最小補償方案:基於 innerVisible 與 duration 模擬動畫結束時機並回調(可傳遞當前節點,或無參以與文檔一致)。
@@
useEffect(() => {
visible ? open() : close()
}, [visible])
useEffect(() => {
setTransitionName(transition || `${classPrefix}-slide-${position}`)
}, [position, transition])
+
+ // 補:Taro 端補發 afterShow/afterClose
+ useEffect(() => {
+ const timer = setTimeout(() => {
+ if (innerVisible) {
+ afterShow?.(nodeRef.current as any)
+ } else {
+ afterClose?.(nodeRef.current as any)
+ }
+ }, Number(duration) || 0)
+ return () => clearTimeout(timer)
+ }, [innerVisible, duration])如需嚴格對齊 Overlay 的“遮罩動畫結束”語義,也可將回調掛在 Overlay 上由其冒泡/轉發。
📝 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 renderPop = () => { | |
| return ( | |
| <View | |
| ref={refObject} | |
| style={{ ...popStyles, display: innerVisible ? 'block' : 'none' }} | |
| ref={nodeRef} | |
| style={{ | |
| ...popStyles, | |
| display: innerVisible ? 'block' : 'none', | |
| ...resizeStyles(), | |
| }} | |
| className={popClassName} | |
| onClick={onClick} | |
| catchMove={lockScroll} | |
| onTouchStart={handleTouchStart} | |
| onTouchMove={handleTouchMove} | |
| onTouchEnd={handleTouchEnd} | |
| onTouchCancel={handleTouchEnd} | |
| > | |
| {top} | |
| {renderTitle()} | |
| {showChildren ? children : null} | |
| </View> | |
| // …existing imports and hooks… | |
| useEffect(() => { | |
| visible ? open() : close() | |
| }, [visible]) | |
| useEffect(() => { | |
| setTransitionName(transition || `${classPrefix}-slide-${position}`) | |
| }, [position, transition]) | |
| // 補:Taro 端補發 afterShow/afterClose | |
| useEffect(() => { | |
| const timer = setTimeout(() => { | |
| if (innerVisible) { | |
| afterShow?.(nodeRef.current as any) | |
| } else { | |
| afterClose?.(nodeRef.current as any) | |
| } | |
| }, Number(duration) || 0) | |
| return () => clearTimeout(timer) | |
| }, [innerVisible, duration]) | |
| const renderPop = () => { | |
| return ( | |
| <View | |
| ref={nodeRef} | |
| style={{ | |
| ...popStyles, | |
| display: innerVisible ? 'block' : 'none', | |
| ...resizeStyles(), | |
| }} | |
| className={popClassName} | |
| onClick={onClick} | |
| catchMove={lockScroll} | |
| onTouchStart={handleTouchStart} | |
| onTouchMove={handleTouchMove} | |
| onTouchEnd={handleTouchEnd} | |
| onTouchCancel={handleTouchEnd} | |
| > | |
| {top} | |
| {renderTitle()} | |
| {showChildren ? children : null} | |
| </View> | |
| ) | |
| } | |
| // …rest of component… |
🤖 Prompt for AI Agents
In src/packages/popup/popup.taro.tsx around lines 301 to 321, the Taro branch
renders a <View> directly and never fires afterShow/afterClose as the Web branch
does via CSSTransition; fix by detecting changes to innerVisible and using
duration to simulate animation end: when innerVisible turns true start a timer
for duration (or 0 if no duration) then call afterShow (pass node if desired),
when innerVisible turns false start a timer for duration then call afterClose;
ensure timers are cleared on unmount and when innerVisible toggles rapidly to
avoid duplicate calls; alternatively you can attach the callbacks to the Overlay
component and forward/bubble the end event if you prefer overlay-based
semantics.
| const min = | ||
| typeof minHeight === 'number' | ||
| ? minHeight | ||
| : parseInt(String(minHeight || 0), 10) || 0 | ||
| const currentHeight = Math.max( | ||
| min, | ||
| heightRef.current - touchMoveDistanceRef.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.
minHeight 的数值解析存在逻辑错误:'%' 被当作像素,导致约束失效与回调高度不准
当前用 parseInt 解析字符串 minHeight,会把 '26%' 解析为 26(像素),与实际 CSS 的 26%(相对视口高度/父容器)不一致。结果是:
- 拖动过程中用过小的阈值进行 clamp,onTouchMove/onTouchEnd 回调报出的 height 偏小;
- 实际渲染又受 CSS min-height 约束,视觉高度与回调值不一致。
建议:显式将字符串 minHeight 解析为像素,支持 '%', 'vh', 'px',其余单位按需扩展。
应用以下变更(示例实现):
- const min =
- typeof minHeight === 'number'
- ? minHeight
- : parseInt(String(minHeight || 0), 10) || 0
+ const min = getMinHeightPx(minHeight)
const currentHeight = Math.max(
min,
heightRef.current - touchMoveDistanceRef.current
)- const min =
- typeof minHeight === 'number'
- ? minHeight
- : parseInt(String(minHeight || 0), 10) || 0
+ const min = getMinHeightPx(minHeight)
const currentHeight = Math.max(
min,
heightRef.current - touchMoveDistanceRef.current
)在组件外(或文件顶部)补充工具函数(额外代码块,非 diff):
// 建议放在文件内顶层作用域
function getMinHeightPx(v?: string | number): number {
if (typeof v === 'number') return v
const s = String(v ?? '').trim()
if (!s) return 0
const n = parseFloat(s)
if (Number.isNaN(n)) return 0
// SSR 安全:仅在浏览器环境读取 window
const vh = typeof window !== 'undefined' ? window.innerHeight : 0
if (s.endsWith('vh')) return (vh * n) / 100
if (s.endsWith('%')) return (vh * n) / 100 // 该组件为 fixed 层,% 基本等同于视口高度
if (s.endsWith('px') || /^[0-9.]+$/.test(s)) return n
return 0
}说明:这里将 '%' 视为相对视口高度处理(与 'vh' 等效),符合大多数弹窗(fixed 定位)场景。
Also applies to: 261-269
🤖 Prompt for AI Agents
In src/packages/popup/popup.tsx around lines 239-247 (also apply same change to
261-269): the code uses parseInt on string minHeight which treats "26%" as 26px
causing incorrect clamping and mismatched reported vs rendered height; replace
that logic by calling a small SSR-safe helper that converts minHeight
(number|string) to pixels supporting "%", "vh", "px" and plain numbers (treat
"%" as viewport-relative like "vh"), then use the returned pixel value as min
when computing currentHeight and in the later block; ensure the helper safely
reads window.innerHeight only when window is defined and returns 0 for invalid
inputs.
| resizable: boolean | ||
| minHeight: string | ||
| onOpen: () => void |
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.
💡 Verification agent
🧩 Analysis chain
新增 props 标为必填会造成不兼容类型变更;minHeight 类型过窄
- 当前
resizable: boolean、minHeight: string为必填,这将强制所有实现BasePopup的地方补齐字段,属于不兼容变更。 minHeight建议支持number | CSSProperties['height'](或string | number),以便接受数值像素与多种 CSS 长度单位。
建议将其改为可选并放宽类型:
- resizable: boolean
- minHeight: string
+ resizable?: boolean
+ minHeight?: React.CSSProperties['height'] | number另外需在 import 处补充 CSSProperties(示意,需在文件顶部修改):
// 需在顶部将
import { ReactNode } from 'react'
// 调整为
import type { ReactNode, CSSProperties } from 'react'请确认对外默认值:resizable 是否为 false;minHeight 的默认值(如存在)。若为必填,请在迁移文档显式标注为“破坏性变更”。
下面执行脚本,确认 BasePopup 的类型定义及其默认值来源:
🏁 Script executed:
#!/bin/bash
set -e
echo "======= 查看类型定义: src/types/spec/popup/base.ts ======="
sed -n '1,200p' src/types/spec/popup/base.ts || true
echo ""
echo "======= 全局搜索 BasePopup 引用 ======="
rg -n "BasePopup" -C3 src || true
echo ""
echo "======= 搜索 resizable 默认值或赋值位置 ======="
rg -n "resizable" -C3 src || true
echo ""
echo "======= 搜索 defaultProps 或类似默认值定义 ======="
rg -n "defaultProps" -C3 src || trueLength of output: 200566
让 resizable 和 minHeight 可选并放宽类型
验证结果:src/packages/popup/popup.tsx 与 popup.taro.tsx 的 defaultProps 中,resizable 默认为 false,minHeight 默认为 ''。但在类型定义 src/types/spec/popup/base.ts(约第 25–27 行)中,这两个属性被声明为必填,且 minHeight 仅限 string,会导致调用方在 TypeScript 层面必须显式传入,产生不兼容变更。
建议对类型进行以下修正,并在迁移文档中明确说明破坏性变更及默认值:
- 修改类型定义(
src/types/spec/popup/base.ts,大约 25–27 行)export interface BasePopup extends BaseProps, BaseOverlay { - resizable: boolean - minHeight: string + resizable?: boolean + minHeight?: React.CSSProperties['height'] | number } - 更新顶部导入
- import { ReactNode } from 'react' + import type { ReactNode, CSSProperties } from 'react'
- 确认默认值(已在实现中定义,无需改动,仅供文档说明)
// src/packages/popup/popup.tsx & popup.taro.tsx defaultProps resizable: false, minHeight: '',
- 在迁移指南中标注
在src/sites/sites-react/doc/docs/react/migrate-from-v2.md(及对应的 Taro 文档):- 明确注明
resizable从必填变更为可选,默认false; - 明确注明
minHeight从必填变更为可选,默认'',并将类型放宽为string | number; - 添加“破坏性变更”提示。
- 明确注明
🤖 Prompt for AI Agents
In src/types/spec/popup/base.ts around lines 25 to 27, change resizable and
minHeight from required to optional and widen minHeight's type to accept string
| number; update any top-of-file imports if needed for optional types, e.g.
import or adjust types referenced; ensure defaultProps remain in
src/packages/popup/popup.tsx and popup.taro.tsx (resizable: false, minHeight:
'') and document the breaking change in
src/sites/sites-react/doc/docs/react/migrate-from-v2.md (and Taro doc): state
resizable is now optional with default false, minHeight is now optional with
default '' and type string | number, and add a “breaking change” notice.
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit