-
Notifications
You must be signed in to change notification settings - Fork 163
fix: nest scroll should block #288
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改引入了多个新文件和功能,主要包括一个新的 React 组件 Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #288 +/- ##
==========================================
+ Coverage 98.45% 98.48% +0.02%
==========================================
Files 18 18
Lines 711 725 +14
Branches 170 179 +9
==========================================
+ Hits 700 714 +14
Misses 11 11 ☔ View full report in Codecov by Sentry. |
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
Outside diff range and nitpick comments (1)
examples/nest.tsx (1)
16-39
: 组件结构和 prop 使用正确
MyItem
组件的定义和内部的List
组件嵌套使用看起来没有问题。这个嵌套的List
组件似乎是用来演示嵌套功能的。可以考虑移除注释掉的
debug
prop,保持代码的整洁。
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- docs/demo/nest.md (1 hunks)
- examples/nest.tsx (1 hunks)
- src/Context.tsx (1 hunks)
- src/List.tsx (4 hunks)
- src/hooks/useFrameWheel.ts (3 hunks)
- src/hooks/useMobileTouchMove.ts (2 hunks)
- tests/scroll.test.js (3 hunks)
Additional context used
GitHub Check: codecov/patch
src/List.tsx
[warning] 438-438: src/List.tsx#L438
Added line #L438 was not covered by testssrc/hooks/useFrameWheel.ts
[warning] 43-43: src/hooks/useFrameWheel.ts#L43
Added line #L43 was not covered by tests
Additional comments not posted (13)
docs/demo/nest.md (1)
1-8
: 这是一个文档页面,不包含任何代码更改。这个文件是一个 markdown 文件,用于为嵌套演示提供文档。它本身不包含任何实际的代码实现。
建议检查引用的示例文件
examples/nest.tsx
,以验证嵌套演示的实际实现。src/Context.tsx (2)
1-1
: 导入语句正确无误!导入 React 模块是在该文件中使用 React 的必要步骤。
3-3
: 使用 Context 共享滚轮锁定状态是一个很好的方法!创建
WheelLockContext
用于在组件之间共享滚轮锁定状态是一个很好的方法。Context 的值被正确地定义为一个接受boolean
参数并返回void
的函数。使用空函数作为初始值是一种常见的模式,它允许组件提供自己的实现。这样,子组件可以根据
lock
参数来控制滚轮锁定状态。即使在组件树中没有可用的 Provider,空的初始值也能确保 Context 可以被安全地使用。examples/nest.tsx (3)
1-4
: 代码没问题!导入的依赖是必要的,没有发现任何问题。
5-14
: 代码看起来不错!
Item
接口和data
数组的定义和生成逻辑都很清晰,没有发现任何问题。
41-67
: 代码结构和逻辑正确
Demo
组件的定义和List
组件的使用看起来都没有问题。使用React.StrictMode
包裹组件是一个很好的实践,有助于发现潜在的问题。onScroll
事件处理函数的定义也是正确的。总的来说,这部分代码没有发现任何重大问题。
src/hooks/useMobileTouchMove.ts (3)
42-45
: 条件调用e.preventDefault()
改进了触摸事件处理将
e.preventDefault()
的调用放在scrollHandled
为true
的条件下,确保只有在滚动被处理的情况下才阻止默认的触摸事件行为。这提高了事件处理的准确性,防止不必要地阻止默认行为。
50-62
: 优化平滑滚动,仅在需要时启动间隔将平滑滚动的间隔逻辑置于
if (scrollHandled)
条件内,确保只有在滚动被处理的情况下才启动间隔。这可以避免不必要的间隔执行,提升性能。
10-15
: 更改callback
函数签名可能导致兼容性问题在
callback
的参数中添加可选参数e?: TouchEvent
,改变了函数的预期接口。任何现有的callback
实现可能未接受这个新参数,可能导致运行时错误。建议检查所有对
useMobileTouchMove
的调用,确保传入的callback
函数已更新以接受新的参数。可以运行以下脚本进行验证:
Verification successful
useMobileTouchMove
的更改已正确实施,无兼容性问题经过验证,
useMobileTouchMove
的更改已在整个代码库中正确实施:
- 在 src/List.tsx 中的使用已更新,包含新的 'e' 参数。
useMobileTouchMove
的实现已更新,包含新的可选 'e' 参数。这个更改是向后兼容的,不会影响现有的不使用 'e' 参数的代码。
建议:如果 'e' 参数在
useMobileTouchMove
的实现中未被使用,考虑添加注释说明其目的,以提高代码可读性。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:查找所有调用 `useMobileTouchMove` 的地方,检查传入的 `callback` 参数是否接受新参数 `e` # 测试:搜索所有对 `useMobileTouchMove` 的调用,期望:`callback` 参数的实现匹配新的函数签名 ast-grep --lang typescript --pattern $'useMobileTouchMove($_, $_, (isHorizontal, offset, smoothOffset, e?) => $_)'Length of output: 115
Script:
#!/bin/bash # 搜索所有 useMobileTouchMove 的使用 echo "Searching for useMobileTouchMove usage:" rg "useMobileTouchMove\(" -n echo "\nChecking implementation of useMobileTouchMove:" rg "function useMobileTouchMove" -A 20 src/hooks/useMobileTouchMove.ts echo "\nChecking if the new parameter 'e' is used in the implementation:" rg "e\?: TouchEvent" -A 5 src/hooks/useMobileTouchMove.tsLength of output: 1343
src/hooks/useFrameWheel.ts (1)
21-21
: 确保所有对 onWheelDelta 的调用都已更新参数
horizontal
从可选变为必选,需要确保代码库中所有对onWheelDelta
的调用都已更新以传递该参数。运行以下脚本以验证所有对
onWheelDelta
的调用是否已正确更新:Verification successful
所有 onWheelDelta 的调用均已正确更新
经过全面检查,我们可以确认代码库中所有对
onWheelDelta
的调用都已正确更新,包括必需的horizontal
参数。具体发现如下:
- 在
src/List.tsx
中,onWheelDelta
的类型定义正确使用了两个参数。- 在
src/hooks/useFrameWheel.ts
中,onWheelDelta
的函数签名已更新为(offset: number, horizontal: boolean) => void
。- 所有对
onWheelDelta
的调用都传递了正确数量的参数。因此,可以确定该更改已在整个代码库中得到正确实施。
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:查找所有对 onWheelDelta 的调用,检查是否传递了 'horizontal' 参数。 # 测试:搜索 onWheelDelta 的调用,期望:所有调用都传递了两个参数。 rg --pcre2 "onWheelDelta\(\s*[^,\)]+(?:,\s*[^,\)]+)+\s*\)" --type js --type tsLength of output: 228
Script:
#!/bin/bash # 描述:广泛搜索所有包含 onWheelDelta 的行,以确保没有遗漏任何调用。 # 搜索所有包含 onWheelDelta 的行 rg "onWheelDelta" --type js --type tsLength of output: 420
src/List.tsx (3)
394-394
: 修复缺少的返回值在
syncScrollTop
的回调函数中添加了return newTop;
,确保正确更新滚动位置。
411-412
: 引入onDeduplicatedRawWheel
变量将
onRawWheel
赋值给新的变量onDeduplicatedRawWheel
,方便在多个地方引用,提升代码可读性。
566-568
: 添加开发环境调试属性在非生产环境下,向
containerProps
添加了data-dev-offset-top
属性,方便调试滚动位置。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
tests/touch.test.js (1)
160-161
: 建议:使用合理的时间值前进定时器在第160行,您将定时器前进了
1000000
毫秒(约16分钟):jest.advanceTimersByTime(1000000);这可能不是必要的,使用过大的时间值可能会增加测试时间。建议使用合适的时间值,例如:
jest.advanceTimersByTime(1000); // 前进1秒
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/List.tsx (3 hunks)
- tests/touch.test.js (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/List.tsx
Additional comments not posted (1)
tests/touch.test.js (1)
164-176
: 验证:确保断言的准确性在对
data-dev-offset-top
属性进行断言时,请确认选择器和索引是否准确。请检查以下代码:
expect(container.querySelectorAll('[data-dev-offset-top]')[0]).toHaveAttribute( 'data-dev-offset-top', '0', );确保
querySelectorAll
返回的元素列表与预期的元素匹配,以避免断言可能通过但实际未测试到目标元素的情况。
fix ant-design/ant-design#50907
Summary by CodeRabbit
新特性
nest.md
,用于演示和组织代码示例。examples/nest.tsx
文件,展示如何高效渲染大型数据集的List
组件。WheelLockContext
,用于集中管理锁定机制。错误修复
测试
List
组件的测试,特别是嵌套滚动和触摸事件的测试用例。