-
Notifications
You must be signed in to change notification settings - Fork 280
feat(Popup): 新增弹层可上下滑动 #3340
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
base: feat_v3.x
Are you sure you want to change the base?
feat(Popup): 新增弹层可上下滑动 #3340
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 3 minutes and 50 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 (11)
Walkthrough此更改为 Popup 组件新增可垂直拖拽调整高度能力:引入 resizable 与 minHeight 属性并添加 onTouchStart/onTouchMove/onTouchEnd 触摸回调,更新 Web 与 Taro 实现、示例、文档、类型与样式以支持可变高度与触摸交互。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as 用户
participant P as Popup(resizable)
participant M as 测量工具
participant DOM as DOM/样式
U->>P: TouchStart
P->>M: 获取初始高度/位置
M-->>P: 初始高度/矩形
P->>U: onTouchStart(height,event)
U->>P: TouchMove(位移)
P->>P: 计算新高度与方向(up/down)
P->>DOM: 更新内联样式(height,minHeight)
P->>U: onTouchMove(height,event,direction)
U->>P: TouchEnd
P->>P: 固化高度/重置触摸状态
P->>U: onTouchEnd(height,event)
Note over P,DOM: 打开时若 resizable 且存在上次高度,恢复该高度
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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
❌ Your patch check has failed because the patch coverage (66.10%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #3340 +/- ##
=============================================
- Coverage 88.13% 88.06% -0.07%
=============================================
Files 291 291
Lines 19140 19198 +58
Branches 2971 2983 +12
=============================================
+ Hits 16869 16907 +38
- Misses 2266 2285 +19
- Partials 5 6 +1 ☔ 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: 7
🧹 Nitpick comments (12)
src/types/spec/popup/base.ts (2)
24-25
: 新增 resizable/minHeight 属性整体合理,但建议微调 minHeight 类型以增强可读性与约束目前将 minHeight 标注为 string,虽然能覆盖 '26%' 等用法,但更贴近语义的写法是使用 CSSProperties['height']。这能更直观表达“合法高度值”的约束(百分比、px、vh 等)。
可以在不影响跨端实现的前提下,做如下微调:
- minHeight: string + minHeight: CSSProperties['height']此外需要同步在文件顶部引入 CSSProperties(该变更不在当前选区,单独附上建议代码):
// 将 import { ReactNode } from 'react' // 调整为 import { ReactNode, CSSProperties } from 'react'
30-32
: 建议补充 onTouchCancel 回调,保持与实现事件集一致Taro 侧已有 handleTouchCancel 处理(组件内部绑定了 onTouchCancel/Cancel),为保证 API 完整度与 Web/Taro 一致,建议在 Base 类型中补充 onTouchCancel。
建议变更:
onTouchMove: (height: number, e: any, direction: 'up' | 'down') => void onTouchStart: (height: number, e: any) => void onTouchEnd: (height: number, e: any) => void + onTouchCancel: (height: number, e: any) => void
备注:参数 e 目前使用 any 以适配跨端(React.TouchEvent/ITouchEvent)是可接受的折中方案。
src/packages/popup/doc.zh-TW.md (1)
105-107
: onTouchMove 建议补充第三个参数名 direction,统一三端文档风格当前仅展示了联合字面量,缺少参数名,不利于阅读。
建议改动如下:
-| 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: TouchEvent<HTMLDivElement>) => void` | `-` | +| onTouchMove | 滑動時觸發 | `(height: number, event: TouchEvent<HTMLDivElement>, direction: 'up' \| 'down') => void` | `-` | +| onTouchEnd | 結束觸碰時觸發 | `(height: number, event: TouchEvent<HTMLDivElement>) => void` | `-` |顺带一提:表格中「left」「title」两行目前混用了简体用词(“标题”“中间”),可统一为繁体(如「標題」「中間」)以提升文档一致性。
src/packages/popup/demos/taro/demo1.tsx (2)
5-7
: 变量命名拼写错误(Resiable → Resizable),请统一修正以避免混淆当前 state 与 setter 均拼写为 Resiable,建议统一为 Resizable。该问题虽不影响功能,但会降低可读性并增加后续维护成本。
可直接应用如下改动(包含声明与所有引用处):
- const [showPopupResiable, setShowPopupResiable] = useState(false) + const [showPopupResizable, setShowPopupResizable] = useState(false) ... - title="基础弹框:可上下滑动" + title="基础弹框:可上下滑动" onClick={() => { - setShowPopupResiable(true) + setShowPopupResizable(true) }} ... - visible={showPopupResiable} + visible={showPopupResizable} ... onClose={() => { - setShowPopupResiable(false) + setShowPopupResizable(false) }}为确保无遗漏,建议全局检索“Resiable”并替换为“Resizable”。
Also applies to: 16-21, 32-53
44-51
: 移动回调内频繁 console.log 可能影响滑动性能onTouchMove 在滑动过程中高频触发,直接 console.log 可能造成卡顿,建议在示例中去掉或做节流/采样。
可选示例(节流打印),供参考:
const rafId = useRef<number | null>(null) const onMove = (height: number, e: any, direction: 'up' | 'down') => { if (rafId.current) return rafId.current = requestAnimationFrame(() => { rafId.current = null console.log('onTouchMove', height, direction) }) }src/packages/popup/demos/h5/demo1.tsx (2)
6-6
: 拼写错误:将 showPopupResiable 重命名为 showPopupResizable当前状态变量与 setter 均拼写有误,建议统一为 showPopupResizable / setShowPopupResizable,提升可读性与一致性。
可直接应用如下修改:
- 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, 37-43
44-52
: Demo 中的调试日志可考虑去除或用条件包裹示例中大量 console.log 对读者不友好,建议删掉或用开发环境开关包裹,以避免噪音。
示例做法:
- onTouchMove={(height, e, direction) => { - console.log('onTouchMove', height, e, direction) - }} + onTouchMove={() => {}} - onTouchStart={(height, e) => { - console.log('onTouchStart', height, e) - }} + onTouchStart={() => {}} - onTouchEnd={(height, e) => { - console.log('onTouchEnd', height, e) - }} + onTouchEnd={() => {}}src/packages/popup/doc.md (1)
105-107
: 为 onTouchMove 第三个参数补充参数名 direction当前函数类型缺少第三个参数名,建议补充以更清晰表达含义。
-| onTouchMove | 滑动时触发 | `(height: number, event: TouchEvent<HTMLDivElement>, 'up' \| 'down') => void` | `-` | +| onTouchMove | 滑动时触发 | `(height: number, event: TouchEvent<HTMLDivElement>, direction: 'up' \| 'down') => void` | `-` |src/packages/popup/popup.taro.tsx (1)
120-128
: open 时恢复高度的写入触发 TS 报错(.style 不存在于 never)在 nodeRef 仍可能推断不准确时,建议在使用处做断言,确保编译通过。
- if (resizable && nodeRef.current && heightRef.current) { - nodeRef.current.style.height = `${heightRef.current}px` - } + if (resizable && nodeRef.current && heightRef.current) { + ;(nodeRef.current as any).style.height = `${heightRef.current}px` + }说明:上述与上一条对 nodeRef 的断言二选一即可;若已将 nodeRef 明确为 MutableRefObject,此处可不再断言。
src/packages/popup/popup.tsx (3)
211-221
: 去除调试日志 console.log库代码中不建议保留日志,避免污染用户控制台。
- console.log('touchstart', touchStartRef.current, heightRef.current)
242-248
: 去除调试日志 console.log与上同。
- console.log('touchend', event)
227-239
: 可选:在移动过程中对高度做下限约束,减少抖动虽然已通过 CSS minHeight 兜底,但直接在计算层面 clamp 能减少多余布局计算与避免视觉跳变。
示例(仅供参考,不强制):
- const currentHeight = heightRef.current - touchMoveDistanceRef.current + const rawHeight = heightRef.current - touchMoveDistanceRef.current + const currentHeight = Math.max(rawHeight, (typeof minHeight === 'number' ? minHeight : 0))说明:若要严谨处理百分比 minHeight,可读取元素父容器高度转为像素后再 clamp。
📜 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 (10)
src/packages/popup/demos/h5/demo1.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
(5 hunks)src/packages/popup/popup.tsx
(7 hunks)src/types/spec/popup/base.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- src/packages/popup/popup.scss
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/packages/popup/demos/h5/demo1.tsx (2)
src/packages/popup/popup.tsx (1)
Popup
(47-320)src/packages/popup/popup.taro.tsx (1)
Popup
(49-345)
src/packages/popup/demos/taro/demo1.tsx (1)
src/packages/popup/popup.taro.tsx (1)
Popup
(49-345)
src/packages/popup/popup.tsx (2)
src/packages/popup/popup.taro.tsx (1)
Popup
(49-345)src/hooks/use-lock-scroll.ts (1)
useLockScroll
(25-108)
src/packages/popup/popup.taro.tsx (1)
src/packages/popup/popup.tsx (1)
Popup
(47-320)
🪛 markdownlint-cli2 (0.17.2)
src/packages/popup/doc.taro.md
113-113: Link fragments should be valid
(MD051, link-fragments)
🪛 GitHub Check: build
src/packages/popup/popup.taro.tsx
[failure] 124-124:
Property 'style' does not exist on type 'never'.
[failure] 280-280:
Type '(event: ITouchEvent) => void' is not assignable to type 'CommonEventFunction'.
[failure] 279-279:
Type '(event: ITouchEvent) => void' is not assignable to type 'CommonEventFunction'.
[failure] 278-278:
Type '(event: ITouchEvent) => void' is not assignable to type 'CommonEventFunction'.
[failure] 277-277:
Type '(event: ITouchEvent) => Promise' is not assignable to type 'CommonEventFunction'.
[failure] 242-242:
'nodeRef.current' is possibly 'null'.
[failure] 220-220:
Property 'offsetHeight' does not exist on type 'never'.
🪛 GitHub Actions: CI
src/packages/popup/popup.taro.tsx
[error] 124-124: TypeScript compile error in step 'tsc --project ./tsconfig.h5.json --noEmit': Property 'style' does not exist on type 'never'.
⏰ 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: test
🔇 Additional comments (6)
src/packages/popup/doc.taro.md (1)
100-101
: 新增 resizable/minHeight 文档说明清晰描述与默认值与实现一致,LGTM。
src/packages/popup/doc.zh-TW.md (1)
90-91
: 新增 resizable/minHeight 文档项清晰,默认值与实现一致描述简洁明确,LGTM。
src/packages/popup/demos/taro/demo1.tsx (1)
32-41
: 新增可拖拽底部弹层示例符合 PR 目标resizable + minHeight + 固定初始高度的组合演示清晰,交互直观,LGTM。
src/packages/popup/doc.md (2)
90-92
: 新增 resizable/minHeight 文档项对齐实现,LGTM属性名、类型与默认值与实现一致。
98-99
: afterShow/afterClose 文档对齐实现,LGTM与 CSSTransition 的 onEntered/onExited 回调保持一致(节点参数)。
src/packages/popup/popup.taro.tsx (1)
271-281
: 请校验 Popup 组件中 View 的 onTouch 事件处理函数签名与 CommonEventFunction 是否对齐*
当前绑定情况:
- onTouchStart = handleTouchStart(async (event: ITouchEvent) ⇒ Promise)
- onTouchMove = handleTouchMove ((event: ITouchEvent) ⇒ void)
- onTouchEnd/onTouchCancel = handleTouchEnd ((event: ITouchEvent) ⇒ void)
注意:async 函数会返回 Promise,若 CommonEventFunction 要求返回 void,可能会导致类型不匹配。
可在本地执行静态类型检查(无需完整构建)确认:
npx tsc --project ./tsconfig.h5.json --noEmit
src/packages/popup/doc.en-US.md
Outdated
| resizable | you can resize the height of popup | `boolean` | `false` | | ||
| minHeight | The minHeight of popup | `string` | `26%` | | ||
| left | The left of title | `ReactNode` | `-` | | ||
| title | The center of title | `ReactNode` | `-` | | ||
| description | The subtitle/description | `ReactNode` | `-` | | ||
| destroyOnClose | Whether to close after the component is destroyed | `boolean` | `false` | | ||
| round | Whether to show rounded corners | `boolean` | `false` | | ||
| portal | Mount the specified node | `HTMLElement` \| `(() => HTMLElement)` \| null` | `null` | | ||
| afterShow | afterShow from `Overlay`, Fired when the mask opening animation ends | `event: HTMLElement` | `-` | | ||
| afterClose | afterClose from `Overlay`, Fired when the mask closing animation ends | `event: HTMLElement` | `-` | | ||
| onClick | Triggered when the popup is clicked | `event: MouseEvent` | `-` | | ||
| onCloseIconClick | Fired when the close icon is clicked | `event: MouseEvent` | `-` | | ||
| onOpen | Triggered when the popup is opened | `-` | `-` | | ||
| onClose | Fired when the popup is closed | `-` | `-` | | ||
| afterShow | afterShow from `Overlay`, Fired when the mask opening animation ends | `event: HTMLElement` | `-` | | ||
| afterClose | afterClose from `Overlay`, Fired when the mask closing animation ends | `event: HTMLElement` | `-` | | ||
| onOverlayClick | Click on the mask to trigger | `event: MouseEvent` | `-` | | ||
| onTouchStart | triggered when starting to touch | `height: number, (event: TouchEvent<HTMLDivElement>) => void` | `-` | | ||
| onTouchMove | triggered when starting to move | `(height: number, event: TouchEvent<HTMLDivElement>, 'up' \| 'down') => void` | `-` | | ||
| onTouchEnd | triggered when finishing to touch | `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.
完善文案与类型签名,提升准确性与一致性
- 文案优化:更符合英文表达(vertical resizing / Minimum height)。
- 类型签名修复:onTouchStart/onTouchEnd 的当前写法有括号位置错误;onTouchMove 建议补充参数名 direction 以便阅读。
建议改动如下:
-| resizable | you can resize the height of popup | `boolean` | `false` |
-| minHeight | The minHeight of popup | `string` | `26%` |
+| resizable | Enable vertical resizing of the popup | `boolean` | `false` |
+| minHeight | Minimum height of the popup | `string` | `26%` |
...
-| onTouchStart | triggered when starting to touch | `height: number, (event: TouchEvent<HTMLDivElement>) => void` | `-` |
-| onTouchMove | triggered when starting to move | `(height: number, event: TouchEvent<HTMLDivElement>, 'up' \| 'down') => void` | `-` |
-| onTouchEnd | triggered when finishing to touch | `height: number, (event: TouchEvent<HTMLDivElement>) => void` | `-` |
+| onTouchStart | triggered when starting to touch | `(height: number, event: TouchEvent<HTMLDivElement>) => void` | `-` |
+| onTouchMove | triggered while moving | `(height: number, event: TouchEvent<HTMLDivElement>, direction: 'up' \| 'down') => void` | `-` |
+| onTouchEnd | triggered when finishing to touch | `(height: number, event: TouchEvent<HTMLDivElement>) => void` | `-` |
另外(可选):若按建议在类型中新增 onTouchCancel,可在此处同步补充文档一行,确保 API 面对开发者完整且易发现。
注意:同表格中 portal 一行(HTMLElement
| (() => HTMLElement)
| null)右侧反引号存在不对称现象,后续可一并修正。
📝 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 | you can resize the height of popup | `boolean` | `false` | | |
| minHeight | The minHeight of popup | `string` | `26%` | | |
| left | The left of title | `ReactNode` | `-` | | |
| title | The center of title | `ReactNode` | `-` | | |
| description | The subtitle/description | `ReactNode` | `-` | | |
| destroyOnClose | Whether to close after the component is destroyed | `boolean` | `false` | | |
| round | Whether to show rounded corners | `boolean` | `false` | | |
| portal | Mount the specified node | `HTMLElement` \| `(() => HTMLElement)` \| null` | `null` | | |
| afterShow | afterShow from `Overlay`, Fired when the mask opening animation ends | `event: HTMLElement` | `-` | | |
| afterClose | afterClose from `Overlay`, Fired when the mask closing animation ends | `event: HTMLElement` | `-` | | |
| onClick | Triggered when the popup is clicked | `event: MouseEvent` | `-` | | |
| onCloseIconClick | Fired when the close icon is clicked | `event: MouseEvent` | `-` | | |
| onOpen | Triggered when the popup is opened | `-` | `-` | | |
| onClose | Fired when the popup is closed | `-` | `-` | | |
| afterShow | afterShow from `Overlay`, Fired when the mask opening animation ends | `event: HTMLElement` | `-` | | |
| afterClose | afterClose from `Overlay`, Fired when the mask closing animation ends | `event: HTMLElement` | `-` | | |
| onOverlayClick | Click on the mask to trigger | `event: MouseEvent` | `-` | | |
| onTouchStart | triggered when starting to touch | `height: number, (event: TouchEvent<HTMLDivElement>) => void` | `-` | | |
| onTouchMove | triggered when starting to move | `(height: number, event: TouchEvent<HTMLDivElement>, 'up' \| 'down') => void` | `-` | | |
| onTouchEnd | triggered when finishing to touch | `height: number, (event: TouchEvent<HTMLDivElement>) => void` | `-` | | |
| resizable | Enable vertical resizing of the popup | `boolean` | `false` | | |
| minHeight | Minimum height of the popup | `string` | `26%` | | |
| left | The left of title | `ReactNode` | `-` | | |
| title | The center of title | `ReactNode` | `-` | | |
| description | The subtitle/description | `ReactNode` | `-` | | |
| destroyOnClose | Whether to close after the component is destroyed | `boolean` | `false` | | |
| round | Whether to show rounded corners | `boolean` | `false` | | |
| portal | Mount the specified node | `HTMLElement` \| `(() => HTMLElement)` \| `null` | `null` | | |
| afterShow | afterShow from `Overlay`, Fired when the mask opening animation ends | `event: HTMLElement` | `-` | | |
| afterClose | afterClose from `Overlay`, Fired when the mask closing animation ends | `event: HTMLElement` | `-` | | |
| onClick | Triggered when the popup is clicked | `event: MouseEvent` | `-` | | |
| onCloseIconClick | Fired when the close icon is clicked | `event: MouseEvent` | `-` | | |
| onOpen | Triggered when the popup is opened | `-` | `-` | | |
| onClose | Fired when the popup is closed | `-` | `-` | | |
| onOverlayClick | Click on the mask to trigger | `event: MouseEvent` | `-` | | |
| onTouchStart | triggered when starting to touch | `(height: number, event: TouchEvent<HTMLDivElement>) => void` | `-` | | |
| onTouchMove | triggered while moving | `(height: number, event: TouchEvent<HTMLDivElement>, direction: 'up' \| 'down') => void` | `-` | | |
| onTouchEnd | triggered when finishing to touch | `(height: number, event: TouchEvent<HTMLDivElement>) => void` | `-` | |
🤖 Prompt for AI Agents
In src/packages/popup/doc.en-US.md around lines 90–107, update copy and type
signatures for clarity: change "you can resize the height of popup" to "vertical
resizing" and "The minHeight of popup" to "Minimum height"; fix onTouchStart and
onTouchEnd signatures to use full function types like (height: number, event:
TouchEvent<HTMLDivElement>) => void; change onTouchMove to include a named
direction parameter like (height: number, event: TouchEvent<HTMLDivElement>,
direction: 'up' | 'down') => void; optionally add an onTouchCancel row if API
supports it; and fix the mismatched backtick in the portal type (`HTMLElement` |
`(() => HTMLElement)` | `null`) so the code fence is symmetric.
| 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.
将事件类型改为 Taro 端的 ITouchEvent,并补充参数名 direction
当前文档在 Taro 端仍使用了 React 的 TouchEvent 类型,不符合端侧实际;同时 onTouchMove 建议显式展示第三参的参数名 direction。
建议改动如下:
-| 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: ITouchEvent) => void` | `-` |
+| onTouchMove | 滑动时触发 | `(height: number, event: ITouchEvent, direction: 'up' \| 'down') => void` | `-` |
+| onTouchEnd | 结束触碰时触发 | `(height: number, event: ITouchEvent) => void` | `-` |
若类型栏想避免引入具体平台类型,也可简化为 event: TouchEvent
,并在注释中注明为 Taro ITouchEvent。
📝 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.
| 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: ITouchEvent) => void` | `-` | | |
| onTouchMove | 滑动时触发 | `(height: number, event: ITouchEvent, direction: 'up' \| 'down') => void` | `-` | | |
| onTouchEnd | 结束触碰时触发 | `(height: number, event: ITouchEvent) => void` | `-` | |
🤖 Prompt for AI Agents
In src/packages/popup/doc.taro.md around lines 115 to 117, the event parameter
types are React TouchEvent but should be Taro's ITouchEvent and onTouchMove
should explicitly name the third parameter as direction; update the three
signatures to use `event: ITouchEvent` (or `event: TouchEvent` with a comment
noting it refers to Taro's ITouchEvent if you prefer avoiding platform types)
and change onTouchMove's type to `(height: number, event: ITouchEvent,
direction: 'up' | 'down') => void`, ensuring the docs show the parameter name
`direction`.
import { View, ITouchEvent } from '@tarojs/components' | ||
import { getRectInMultiPlatformWithoutCache } from '@/utils/taro/get-rect' | ||
import { defaultOverlayProps } from '@/packages/overlay/overlay.taro' |
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.
修正 Taro 事件类型导入,避免 onTouch 类型不匹配*
View 的 onTouchStart/Move/End 在 Taro 中要求 CommonEventFunction,需显式引入类型,并避免将 handler 定义为返回 Promise 的 async 函数。
建议调整导入为:
-import { View, ITouchEvent } from '@tarojs/components'
+import { View } from '@tarojs/components'
+import type { ITouchEvent, CommonEventFunction } from '@tarojs/components'
📝 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.
import { View, ITouchEvent } from '@tarojs/components' | |
import { getRectInMultiPlatformWithoutCache } from '@/utils/taro/get-rect' | |
import { defaultOverlayProps } from '@/packages/overlay/overlay.taro' | |
import { View } from '@tarojs/components' | |
import type { ITouchEvent, CommonEventFunction } from '@tarojs/components' | |
import { getRectInMultiPlatformWithoutCache } from '@/utils/taro/get-rect' | |
import { defaultOverlayProps } from '@/packages/overlay/overlay.taro' |
🤖 Prompt for AI Agents
In src/packages/popup/popup.taro.tsx around lines 13 to 15, the file currently
imports ITouchEvent which causes type mismatches for View's
onTouchStart/onTouchMove/onTouchEnd handlers; replace ITouchEvent with the
proper Taro types (import CommonEvent and CommonEventFunction from
'@tarojs/components'), annotate touch handler parameters with CommonEvent (or
use CommonEventFunction as the handler type) and ensure handlers are not
declared async or returning Promise (if async logic is needed wrap it inside a
synchronous handler that calls the async function without returning the Promise
to Taro).
let innerIndex = zIndex || _zIndex | ||
const [index, setIndex] = useState(innerIndex) | ||
const [innerVisible, setInnerVisible] = useState(visible) | ||
const [showChildren, setShowChildren] = useState(true) | ||
const [transitionName, setTransitionName] = useState('') | ||
const refObject = useLockScrollTaro(innerVisible && lockScroll) | ||
const classPrefix = 'nut-popup' | ||
const nodeRef = useLockScrollTaro(innerVisible && lockScroll) |
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.
nodeRef 类型推断为 never 导致 .style/.offsetHeight 报错
useLockScrollTaro 返回的 ref 未正确带出元素类型,导致 nodeRef.current 推断为 never。需给出明确的可写 ref 类型或断言为 any。
- const nodeRef = useLockScrollTaro(innerVisible && lockScroll)
+ const nodeRef = useLockScrollTaro(
+ innerVisible && lockScroll
+ ) as React.MutableRefObject<any>
📝 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 nodeRef = useLockScrollTaro(innerVisible && lockScroll) | |
const nodeRef = useLockScrollTaro( | |
innerVisible && lockScroll | |
) as React.MutableRefObject<any> |
🤖 Prompt for AI Agents
In src/packages/popup/popup.taro.tsx around line 97, nodeRef is inferred as
never (so nodeRef.current has no .style/.offsetHeight), because
useLockScrollTaro’s ref return type is not typed; fix by giving the hook a
concrete writable ref type (e.g. MutableRefObject<HTMLElement | null> or
appropriate element type) or by asserting the returned ref when calling
useLockScrollTaro (cast to MutableRefObject<HTMLElement | null> or any) so
nodeRef.current is correctly typed and .style/.offsetHeight access no longer
errors.
src/packages/popup/popup.taro.tsx
Outdated
const handleTouchMove = (event: ITouchEvent) => { | ||
if (!resizable || !nodeRef.current || !rootRect.current) return | ||
event.stopPropagation() | ||
|
||
// move过程中,当前的pageY 与 start值比较 | ||
touchMoveDistanceRef.current = | ||
event.touches[0].pageY - touchStartRef.current | ||
|
||
const handleMove = () => { | ||
const currentHeight = heightRef.current - touchMoveDistanceRef.current | ||
nodeRef.current.style.height = pxTransform(currentHeight) | ||
if (touchMoveDistanceRef.current > 0 && isTouching.current) { | ||
// 向下滑动 | ||
onTouchMove?.(currentHeight, event, 'down') | ||
// console.log('向下', nodeRef.current.style.height) | ||
} else { | ||
// 向上滑动 | ||
onTouchMove?.(currentHeight, event, 'up') | ||
console.log( | ||
'向上', | ||
heightRef.current, | ||
touchMoveDistanceRef.current, | ||
currentHeight | ||
) | ||
} | ||
} | ||
requestAnimationFrame(handleMove) | ||
} |
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.
修复闭包内 nodeRef 可能为 null 的类型告警;去除调试日志
在 requestAnimationFrame 的闭包内,TS 无法沿用外层的非空收窄;需缓存当前元素引用并在闭包内使用。同时建议移除 console.log。
- const handleTouchMove = (event: ITouchEvent) => {
- if (!resizable || !nodeRef.current || !rootRect.current) return
- event.stopPropagation()
-
- // move过程中,当前的pageY 与 start值比较
- touchMoveDistanceRef.current =
- event.touches[0].pageY - touchStartRef.current
-
- const handleMove = () => {
- const currentHeight = heightRef.current - touchMoveDistanceRef.current
- nodeRef.current.style.height = pxTransform(currentHeight)
- if (touchMoveDistanceRef.current > 0 && isTouching.current) {
- // 向下滑动
- onTouchMove?.(currentHeight, event, 'down')
- } else {
- // 向上滑动
- onTouchMove?.(currentHeight, event, 'up')
- console.log(
- '向上',
- heightRef.current,
- touchMoveDistanceRef.current,
- currentHeight
- )
- }
- }
- requestAnimationFrame(handleMove)
- }
+ const handleTouchMove: CommonEventFunction = (event) => {
+ if (!resizable || !nodeRef.current || !rootRect.current) return
+ const e = event as ITouchEvent
+ e.stopPropagation()
+ // 计算位移
+ touchMoveDistanceRef.current = e.touches[0].pageY - touchStartRef.current
+ const el = nodeRef.current as any
+ requestAnimationFrame(() => {
+ if (!el) return
+ const currentHeight = heightRef.current - touchMoveDistanceRef.current
+ el.style.height = pxTransform(currentHeight)
+ if (touchMoveDistanceRef.current > 0 && isTouching.current) {
+ onTouchMove?.(currentHeight, e, 'down')
+ } else {
+ onTouchMove?.(currentHeight, e, 'up')
+ }
+ })
+ }
📝 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 handleTouchMove = (event: ITouchEvent) => { | |
if (!resizable || !nodeRef.current || !rootRect.current) return | |
event.stopPropagation() | |
// move过程中,当前的pageY 与 start值比较 | |
touchMoveDistanceRef.current = | |
event.touches[0].pageY - touchStartRef.current | |
const handleMove = () => { | |
const currentHeight = heightRef.current - touchMoveDistanceRef.current | |
nodeRef.current.style.height = pxTransform(currentHeight) | |
if (touchMoveDistanceRef.current > 0 && isTouching.current) { | |
// 向下滑动 | |
onTouchMove?.(currentHeight, event, 'down') | |
// console.log('向下', nodeRef.current.style.height) | |
} else { | |
// 向上滑动 | |
onTouchMove?.(currentHeight, event, 'up') | |
console.log( | |
'向上', | |
heightRef.current, | |
touchMoveDistanceRef.current, | |
currentHeight | |
) | |
} | |
} | |
requestAnimationFrame(handleMove) | |
} | |
const handleTouchMove: CommonEventFunction = (event) => { | |
if (!resizable || !nodeRef.current || !rootRect.current) return | |
const e = event as ITouchEvent | |
e.stopPropagation() | |
// 计算位移 | |
touchMoveDistanceRef.current = e.touches[0].pageY - touchStartRef.current | |
const el = nodeRef.current as any | |
requestAnimationFrame(() => { | |
if (!el) return | |
const currentHeight = heightRef.current - touchMoveDistanceRef.current | |
el.style.height = pxTransform(currentHeight) | |
if (touchMoveDistanceRef.current > 0 && isTouching.current) { | |
onTouchMove?.(currentHeight, e, 'down') | |
} else { | |
onTouchMove?.(currentHeight, e, 'up') | |
} | |
}) | |
} |
🧰 Tools
🪛 GitHub Check: build
[failure] 242-242:
'nodeRef.current' is possibly 'null'.
🤖 Prompt for AI Agents
In src/packages/popup/popup.taro.tsx around lines 232 to 259, the
requestAnimationFrame closure uses nodeRef.current and TS can't rely on the
outer non-null narrowing; fix by caching nodeRef.current (and any other needed
values like heightRef.current) into local consts before creating the handleMove
closure, perform a null-check and return if the element is absent, then use
those cached consts inside the closure instead of accessing nodeRef.current
directly; also remove the stray console.log used for debugging and keep
onTouchMove calls as-is.
src/packages/popup/popup.taro.tsx
Outdated
const handleTouchEnd = (event: ITouchEvent) => { | ||
if (!resizable || !nodeRef.current || !rootRect.current) return | ||
console.log('touchend', event) | ||
isTouching.current = false | ||
const currentHeight = heightRef.current - touchMoveDistanceRef.current | ||
onTouchEnd?.(currentHeight, event) | ||
} |
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.
修正 onTouchEnd 类型;移除调试日志
与前两处一致,使用 CommonEventFunction 并去除 console.log。
- const handleTouchEnd = (event: ITouchEvent) => {
- if (!resizable || !nodeRef.current || !rootRect.current) return
- console.log('touchend', event)
- isTouching.current = false
- const currentHeight = heightRef.current - touchMoveDistanceRef.current
- onTouchEnd?.(currentHeight, event)
- }
+ const handleTouchEnd: CommonEventFunction = (event) => {
+ if (!resizable || !nodeRef.current || !rootRect.current) return
+ const e = event as ITouchEvent
+ isTouching.current = false
+ const currentHeight = heightRef.current - touchMoveDistanceRef.current
+ onTouchEnd?.(currentHeight, e)
+ }
📝 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 handleTouchEnd = (event: ITouchEvent) => { | |
if (!resizable || !nodeRef.current || !rootRect.current) return | |
console.log('touchend', event) | |
isTouching.current = false | |
const currentHeight = heightRef.current - touchMoveDistanceRef.current | |
onTouchEnd?.(currentHeight, event) | |
} | |
const handleTouchEnd: CommonEventFunction = (event) => { | |
if (!resizable || !nodeRef.current || !rootRect.current) return | |
const e = event as ITouchEvent | |
isTouching.current = false | |
const currentHeight = heightRef.current - touchMoveDistanceRef.current | |
onTouchEnd?.(currentHeight, e) | |
} |
🤖 Prompt for AI Agents
In src/packages/popup/popup.taro.tsx around lines 261 to 267, remove the
console.log and change the touch handler signature and onTouchEnd usage to match
the other two handlers by using the CommonEventFunction type: replace the
ITouchEvent parameter with the CommonEvent (or the component's
CommonEventFunction signature) and ensure onTouchEnd is typed/called as a
CommonEventFunction so types align; keep the same logic (set isTouching.current
= false, compute currentHeight, call onTouchEnd with currentHeight and the
event) but without the debug log.
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
♻️ Duplicate comments (5)
src/packages/popup/popup.taro.tsx (5)
13-15
: 修正 Taro 触摸事件类型导入,消除 onTouch 类型不匹配(CI 报错)*CI 显示 onTouchStart/Move/End 的 handler 不是 CommonEventFunction(见 Lines 277-280)。View 的触摸事件在 Taro 中要求 CommonEventFunction。请只从组件包导入 View,本地以类型方式引入 ITouchEvent 和 CommonEventFunction。
-import { View, ITouchEvent } from '@tarojs/components' +import { View } from '@tarojs/components' +import type { ITouchEvent, CommonEventFunction } from '@tarojs/components'
97-99
: nodeRef 推断为 never,导致 .style/.offsetHeight 报错当前 nodeRef 类型未从 useLockScrollTaro 正确推断,触发 “Property 'style' does not exist on type 'never'” 等错误(Lines 124、220)。临时方案:在调用处断言为可写 ref。
- const nodeRef = useLockScrollTaro(innerVisible && lockScroll) + const nodeRef = useLockScrollTaro( + innerVisible && lockScroll + ) as React.MutableRefObject<any>
210-230
: onTouchStart 不应返回 Promise;改为 CommonEventFunction 并异步链式更新高度当前为 async 函数,返回 Promise,与 Taro 的 CommonEventFunction 不兼容(CI 报错 Line 277)。改为同步 handler,内部使用 Promise.then 获取 rect,再回调 onTouchStart。
- const handleTouchStart = async (event: ITouchEvent) => { - if (!resizable || !nodeRef.current) return - // 开始touch,记录下touch的pageY,用以判断是向上滑动还是向下滑动 - touchStartRef.current = event.touches[0].pageY - // 标记开始滑动 - isTouching.current = true - // 标记当前popup的高度 - const rect = await getRectInMultiPlatformWithoutCache(nodeRef.current) - rootRect.current = rect - heightRef.current = - nodeRef.current?.offsetHeight || rootRect.current?.height || 0 - // console.log( - // 'touchstart', - // touchStartRef.current, - // heightRef.current, // - // rootRect.current, - // nodeRef.current?.offsetHeight, // - // nodeRef.current.style.height // - // ) - onTouchStart?.(heightRef.current, event) - } + const handleTouchStart: CommonEventFunction = (event) => { + if (!resizable || !nodeRef.current) return + const e = event as ITouchEvent + // 记录起点 + touchStartRef.current = e.touches[0].pageY + isTouching.current = true + const el = nodeRef.current as any + // 先取 offsetHeight,避免等待异步 + heightRef.current = el?.offsetHeight || 0 + getRectInMultiPlatformWithoutCache(el).then((rect) => { + rootRect.current = rect + // 若未取到高度,再回落到 rect + if (!heightRef.current) { + heightRef.current = rect?.height || 0 + } + onTouchStart?.(heightRef.current, e) + }) + }
232-259
: 修复 onTouchMove 类型与潜在空引用;移除调试日志
- 使用 CommonEventFunction 以满足 Taro 类型要求(CI 报错 Lines 278-279)。
- 在 requestAnimationFrame 闭包内缓存并使用 el,避免 TS 的可能为 null 报警(CI 报错 Line 242)。
- 统一使用 pxTransform;移除 console.log。
- const handleTouchMove = (event: ITouchEvent) => { - if (!resizable || !nodeRef.current || !rootRect.current) return - event.stopPropagation() - - // move过程中,当前的pageY 与 start值比较 - touchMoveDistanceRef.current = - event.touches[0].pageY - touchStartRef.current - - const handleMove = () => { - const currentHeight = heightRef.current - touchMoveDistanceRef.current - nodeRef.current.style.height = pxTransform(currentHeight) - if (touchMoveDistanceRef.current > 0 && isTouching.current) { - // 向下滑动 - onTouchMove?.(currentHeight, event, 'down') - // console.log('向下', nodeRef.current.style.height) - } else { - // 向上滑动 - onTouchMove?.(currentHeight, event, 'up') - console.log( - '向上', - heightRef.current, - touchMoveDistanceRef.current, - currentHeight - ) - } - } - requestAnimationFrame(handleMove) - } + const handleTouchMove: CommonEventFunction = (event) => { + if (!resizable || !nodeRef.current || !rootRect.current) return + const e = event as ITouchEvent + e.stopPropagation() + // 计算位移 + touchMoveDistanceRef.current = e.touches[0].pageY - touchStartRef.current + const el = nodeRef.current as any + requestAnimationFrame(() => { + if (!el) return + const currentHeight = heightRef.current - touchMoveDistanceRef.current + el.style.height = pxTransform(currentHeight) + if (touchMoveDistanceRef.current > 0 && isTouching.current) { + onTouchMove?.(currentHeight, e, 'down') + } else { + onTouchMove?.(currentHeight, e, 'up') + } + }) + }
261-267
: 修复 onTouchEnd 类型与调试日志与前两处保持一致,使用 CommonEventFunction,并移除 console.log。
- const handleTouchEnd = (event: ITouchEvent) => { - if (!resizable || !nodeRef.current || !rootRect.current) return - console.log('touchend', event) - isTouching.current = false - const currentHeight = heightRef.current - touchMoveDistanceRef.current - onTouchEnd?.(currentHeight, event) - } + const handleTouchEnd: CommonEventFunction = (event) => { + if (!resizable || !nodeRef.current || !rootRect.current) return + const e = event as ITouchEvent + isTouching.current = false + const currentHeight = heightRef.current - touchMoveDistanceRef.current + onTouchEnd?.(currentHeight, e) + }
🧹 Nitpick comments (2)
src/packages/popup/popup.taro.tsx (2)
123-125
: 恢复高度时推荐使用 pxTransform 以适配多端(Harmony/TD)功能上无问题,但为了与后续动态设置高度保持一致,并适配多端单位转换,建议使用 pxTransform。
- nodeRef.current.style.height = `${heightRef.current}px` + nodeRef.current.style.height = pxTransform(heightRef.current)
236-246
: 建议对 currentHeight 进行最小高度限制,避免小于 minHeight 或出现负值虽然样式上有 minHeight,但事件回调与内联 height 仍可能小于该值。建议在计算时进行 clamp。
- const el = nodeRef.current as any + const el = nodeRef.current as any requestAnimationFrame(() => { if (!el) return - const currentHeight = heightRef.current - touchMoveDistanceRef.current + const rawHeight = heightRef.current - touchMoveDistanceRef.current + const minH = parseMinHeight(minHeight) + const currentHeight = Math.max(rawHeight, minH, 0) el.style.height = pxTransform(currentHeight)可在文件顶部(import 之后)添加辅助方法:
function parseMinHeight(minHeight?: string | number): number { if (typeof minHeight === 'number') return minHeight if (!minHeight) return 0 const m = String(minHeight).trim() if (m.endsWith('px')) return parseFloat(m) if (m.endsWith('rpx')) { // 根据 pxTransform 的实现,rpx 会在 transform 内部处理;这里可做近似换算或直接返回 0 让 pxTransform 处理 return parseFloat(m) // 可按需替换为更精准的换算 } const n = Number(m) return Number.isFinite(n) ? n : 0 }
📜 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 (2)
src/packages/popup/popup.taro.tsx
(5 hunks)src/packages/popup/popup.tsx
(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/popup/popup.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/packages/popup/popup.taro.tsx (4)
src/packages/popup/popup.tsx (1)
Popup
(47-320)src/utils/taro/platform.ts (1)
harmony
(3-7)src/utils/taro/get-rect.ts (1)
getRectInMultiPlatformWithoutCache
(58-76)src/utils/taro/px-transform.ts (1)
pxTransform
(5-9)
🪛 GitHub Check: build
src/packages/popup/popup.taro.tsx
[failure] 124-124:
Property 'style' does not exist on type 'never'.
[failure] 280-280:
Type '(event: ITouchEvent) => void' is not assignable to type 'CommonEventFunction'.
[failure] 279-279:
Type '(event: ITouchEvent) => void' is not assignable to type 'CommonEventFunction'.
[failure] 278-278:
Type '(event: ITouchEvent) => void' is not assignable to type 'CommonEventFunction'.
[failure] 277-277:
Type '(event: ITouchEvent) => Promise' is not assignable to type 'CommonEventFunction'.
[failure] 242-242:
'nodeRef.current' is possibly 'null'.
[failure] 220-220:
Property 'offsetHeight' does not exist on type 'never'.
🪛 GitHub Actions: CI
src/packages/popup/popup.taro.tsx
[error] 124-124: TypeScript error TS2339: Property 'style' does not exist on type 'never'.
⏰ 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: test
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit