-
Notifications
You must be signed in to change notification settings - Fork 155
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
fix: scroll offset should not block #294
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/hooks/useScrollDrag.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
概述遍历在 变更
序列图sequenceDiagram
participant User
participant Event
participant Function
participant Window
User->>Event: 触发鼠标/触摸事件
Event->>Function: 调用 getPageXY
Function->>Window: 获取滚动位置
Function-->>Event: 返回调整后的坐标
可能相关的 PR
诗歌
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
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
🧹 Nitpick comments (1)
src/hooks/useScrollDrag.ts (1)
Line range hint
16-93
: 建议补充手动测试用例由于 PR 描述中提到 jsdom 测试的限制,建议:
- 添加手动测试步骤说明
- 考虑在不同滚动场景下的测试用例
- 特别关注大页面滚动时的边界情况
需要我帮助编写测试用例说明文档吗?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/hooks/useScrollDrag.ts
(1 hunks)
🔇 Additional comments (1)
src/hooks/useScrollDrag.ts (1)
13-13
: 修复滚动偏移量计算逻辑正确!
代码变更正确解决了视口坐标与页面坐标之间的不匹配问题。由于 getBoundingClientRect
返回的是相对于视口的坐标,减去滚动偏移量是必要的。
建议验证在不同浏览器中的兼容性:
✅ Verification successful
Based on the search results, I can see that scrollX
and scrollY
are only used in useScrollDrag.ts
and there are no browser compatibility handling code for these properties. However, since these properties are well-supported in modern browsers (IE9+), and the codebase seems to be targeting modern browsers (using React hooks, TypeScript, etc.), I can generate the final response:
滚动偏移量计算方案可靠,无需额外兼容性处理!
window.scrollX
和 window.scrollY
在现代浏览器中有良好的支持度,当前实现方案简洁有效。代码中使用 TypeScript、React Hooks 等现代特性,说明已经定位于现代浏览器,无需增加额外的兼容性处理代码。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有针对 scrollX/scrollY 的兼容性处理代码
# 搜索是否存在浏览器兼容性相关代码
rg -i "scroll|pageXOffset|pageYOffset" --type ts
Length of output: 23561
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #294 +/- ##
=======================================
Coverage 98.32% 98.32%
=======================================
Files 19 19
Lines 778 778
Branches 187 187
=======================================
Hits 765 765
Misses 13 13 ☔ View full report in Codecov by Sentry. |
当 VirtualList 不是定格时,mouseY 和 getBoundingClientRect 的 top & bottom 是不一致的。需要减去滚动 offset。
PS: jsdom 无法测试这个。
Summary by CodeRabbit