Conversation
|
Someone is attempting to deploy a commit to the afc163's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. Walkthrough本次修改更新 Changes平滑滚动修复
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~15 minutes Change: Bug fix · Severity of issue fixed: Medium 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 小兔挥爪触发滚动， Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #388 +/- ##
=======================================
Coverage 97.62% 97.62%
=======================================
Files 19 19
Lines 843 843
Branches 206 210 +4
=======================================
Hits 823 823
Misses 20 20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
36cdf75 to
2fb57c0
Compare
A touch swipe that scrolls a virtual list upwards never comes to rest. After the
finger lifts, the list keeps moving at a constant 1px per frame until something
else interrupts it, instead of easing out the way the opposite direction does.
The smooth-scroll interval decays the offset by SMOOTH_PTG and stops once there
is nothing left to move:
const offset = Math.floor(isHorizontal ? offsetX : offsetY);
if (!callback(...) || Math.abs(offset) <= 0.1) clearInterval(...);
Math.floor never returns 0 for a negative value - Math.floor(-0.9) is -1. Once
the offset decays into (-1, 0), `offset` stays -1 forever, so the stop check
never passes and the interval keeps firing. The other direction is unaffected
because Math.floor(0.9) is 0, which is why only one direction ever hangs.
Use Math.trunc, which rounds towards zero, so the existing
`Math.abs(offset) <= 0.1` check finally does what it was written to do - in both
directions. The original threshold and structure are kept.
Needs all of: a virtual list (height + itemHeight), touch input, and not already
being at the top - at the top `useOriginScroll` hands the gesture to the browser
and no interval is created at all.
Fixes react-component#275
2fb57c0 to
88497c0
Compare
1 participant
🤔 This is a ...
🔗 Probably related Issues
Fixes #275, #305
💡 Background and Solution
A touch swipe that scrolls a virtual list upwards never comes to rest. After the finger lifts, the list keeps moving at a constant 1px per frame until something else interrupts it, instead of easing out the way the opposite direction does.
The smooth-scroll interval decays the offset by
SMOOTH_PTGand stops once there is nothing left to move:Math.floornever returns 0 for a negative value —Math.floor(-0.9)is-1. Once the offset decays into(-1, 0),offsetstays-1forever, so the stop check never passes and the interval keeps firing. The other direction is unaffected becauseMath.floor(0.9)is0— which is exactly why only one direction ever hangs.The fix is one line:
Math.truncrounds towards zero, so the existingMath.abs(offset) <= 0.1check finally does what it was written to do, for both directions.The original threshold and the original structure are kept.
Reproduces only when all of these hold:
height+itemHeight)useMobileTouchMoveis only wired up when virtual; the wheel path has no interval of its own)useOriginScrollhands the gesture to the browser and no interval is ever created✅ Verification
Red / green with the change reverted and re-applied, full suite both times:
Only the negative direction goes red, which matches the
Math.flooranalysis.Also confirmed in a real browser (Chrome device emulation) on an rc-tree virtual list:
-1useMobileTouchMove.tsis at 100% line / statement / function coverage (the one uncovered branch isif (touchedRef.current), unreachable because the listeners are only attached after a successfultouchstart).Full suite: 9 suites / 288 tests passed.
tsc --noEmitclean, eslint 0 errors, prettier clean.📝 Change Log
Summary by CodeRabbit
Bug 修复
测试