fix: infinite scroll wrap animation direction#6653
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds wrap-scrolling to PagedView: detects drags past first/last pages, starts/cancels/finalizes wrap drags, temporarily extends scroll bounds and translates the wrap target page, suppresses edge-glow during wrap, centralizes snap-duration computation, and exposes visual-scroll APIs ( Changes
sequenceDiagram
participant Touch as Touch Input
participant Paged as PagedView
participant Scroller as Scroller
participant Indicator as PageIndicator
participant Wallpaper as WallpaperInterpolator
rect rgba(200,200,255,0.5)
Touch->>Paged: drag beyond first/last page
Paged->>Paged: detect boundary, startWrapDrag()
end
rect rgba(200,255,200,0.5)
Paged->>Scroller: snapToPageWrapped(targetVirtualScroll, duration)
Scroller-->>Paged: animation completes / aborted
Paged->>Paged: finalizeWrapScroll() / cancelWrapScroll()
end
rect rgba(255,230,200,0.5)
Paged->>Indicator: getScrollForPageIndicator() (visual scroll)
Paged->>Wallpaper: getScrollForWallpaper() (visual scroll)
Indicator-->>Paged: update indicator
Wallpaper-->>Paged: update parallax
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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 |
wellorbetter
force-pushed
the
fix/infinite-scroll-wrap-animation
branch
11 times, most recently
from
9815b6c to
9e10987
Compare
wellorbetter
left a comment
wellorbetter
left a comment
There was a problem hiding this comment.
This fix makes the infinite scroll wrap animation scroll one page in the correct swipe direction, instead of visually passing through all intermediate pages. The approach uses translationX to move the target page's View next to the current page, lets the scroller animate one page distance, then resets the translation and jumps to the real scroll position once the animation ends.
Edge cases covered: user dragging past the edge then dragging back, animation interrupted by touch, touch cancel events, and RTL layouts.
wellorbetter
force-pushed
the
fix/infinite-scroll-wrap-animation
branch
from
7f2a584 to
3b00e37
Compare
wellorbetter
force-pushed
the
fix/infinite-scroll-wrap-animation
branch
from
3b00e37 to
694a680
Compare
![coderabbitai[bot]](https://avatars.mcszy.xyz/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/com/android/launcher3/PagedView.java (1)
1458-1547:⚠️ Potential issue | 🟠 MajorCancel prepared wrap state before non-wrap or cancelled settlements.
After
startWrapDrag(...), the release can still resolve to a normal page, for example when the user reverses direction. These branches callsnapToPageWithVelocity(...)whilemWrapToPageremains set, so the later scroller abort/finalization can jump to the stale wrap target.ACTION_CANCELhas the same problem by committing the wrap target instead of cancelling the prepared drag.Add a separate “cancel wrap” path that restores translation/bounds without jumping to
mWrapToPage, and use it before normal snaps or cancellation.Suggested fix shape
+ private void cancelWrapScroll() { + if (!isWrapScrolling()) { + return; + } + int targetPage = mWrapToPage; + mWrapToPage = INVALID_PAGE; + + View targetView = getPageAt(targetPage); + if (targetView != null) { + targetView.setTranslationX(0); + } + + mMinScroll = mSavedMinScroll; + mMaxScroll = mSavedMaxScroll; + + int currentScroll = Utilities.boundToRange( + mOrientationHandler.getPrimaryScroll(this), mMinScroll, mMaxScroll); + mOrientationHandler.setPrimary(this, VIEW_SCROLL_TO, currentScroll); + }Then call
cancelWrapScroll()before any non-wrapsnapToPageWithVelocity(...)path and fromACTION_CANCEL; reservefinalizeWrapScroll()for completed/committed wrap animations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/com/android/launcher3/PagedView.java` around lines 1458 - 1547, The code is leaving a prepared wrap target (mWrapToPage) set and sometimes commits it when a normal snap or cancel occurs; update the branches that call snapToPageWithVelocity(...) (the non-wrap snap paths inside the fling/settle logic) to first call cancelWrapScroll() to restore translation/bounds and clear the prepared wrap state instead of finalizing it, and replace uses of finalizeWrapScroll() only for committed/finished wrap animations; also invoke cancelWrapScroll() in the ACTION_CANCEL path before calling snapToDestination so the prepared wrap target is not applied on cancel. Ensure you reference startWrapDrag, mWrapToPage, snapToPageWithVelocity, snapToPageWrapped, cancelWrapScroll(), finalizeWrapScroll(), and ACTION_CANCEL when making the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/com/android/launcher3/PagedView.java`:
- Around line 1363-1371: The edge-wrap logic incorrectly treats single-child
indices as page boundaries; update the checks in the block using mCurrentPage,
getChildCount(), pulledTo, mMinScroll/mMaxScroll and startWrapDrag(...) to
operate on visible page-group boundaries and group distances: compute the
current visible group index and the last/first visible group index (instead of
comparing mCurrentPage == getChildCount()-1 or == 0), compute the scroll
distance between group anchors using mPageScrolls[groupIndex] (use the delta
between group anchors, not adjacent child indices, to avoid the zero-delta
two-panel case), then use that group-based distance to determine
pastEnd/pastStart and call startWrapDrag(targetGroupIndex) with the correct
group target; retain the existing checks for isWrapScrolling() and enableFeed
but switch all comparisons and startWrapDrag parameters to group indices/scroll
anchors rather than raw child indices.
- Around line 1886-1890: When wrap-scrolling is active (mWrapToPage != -1) the
visible wrap target has a temporary translation that must be included in
scroll-progress calculations; update the code so that any code that calls
getScrollProgress(...) or uses getScrollForPage(...) while wrapping uses an
effective scroll that adds the wrap target's translation. Add a small helper
(e.g., getEffectiveScrollForPage(page)) that for the mWrapToPage returns
getScrollForPage(page) plus the wrap target's current translationX (read from
the view or compute from the same values used to set it), and then use that
helper from Workspace.updatePageAlphaValues(), updatePageScrollValues(), and any
screen-center/scroll-progress calculations so fading/alpha uses the translated
position.
---
Outside diff comments:
In `@src/com/android/launcher3/PagedView.java`:
- Around line 1458-1547: The code is leaving a prepared wrap target
(mWrapToPage) set and sometimes commits it when a normal snap or cancel occurs;
update the branches that call snapToPageWithVelocity(...) (the non-wrap snap
paths inside the fling/settle logic) to first call cancelWrapScroll() to restore
translation/bounds and clear the prepared wrap state instead of finalizing it,
and replace uses of finalizeWrapScroll() only for committed/finished wrap
animations; also invoke cancelWrapScroll() in the ACTION_CANCEL path before
calling snapToDestination so the prepared wrap target is not applied on cancel.
Ensure you reference startWrapDrag, mWrapToPage, snapToPageWithVelocity,
snapToPageWrapped, cancelWrapScroll(), finalizeWrapScroll(), and ACTION_CANCEL
when making the changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4de44bd0-d938-4105-a2c1-2b859b7cfcf1
📒 Files selected for processing (2)
src/com/android/launcher3/PagedView.javasrc/com/android/launcher3/Workspace.java
wellorbetter
force-pushed
the
fix/infinite-scroll-wrap-animation
branch
from
694a680 to
fffbfaf
Compare
|
This fix is working great for me. need to merge this. |
SuperDragonXD
changed the title
SuperDragonXD
left a comment
SuperDragonXD
left a comment
There was a problem hiding this comment.
Hello! I've tested this feature, and it doesn't seem to account for wallpaper scrolling. Would it be possible to implement this first?
recording-scrolling.mp4
|
It is in src\com\android\launcher3\util\WallpaperOffsetInterpolator.java I use live wallpaper. |
I didn't consider the issue of the wallpaper. The one I'm using is the static type. I'll have a look. |
Thanks. I will have a look later.... |
wellorbetter
force-pushed
the
fix/infinite-scroll-wrap-animation
branch
from
fffbfaf to
42a7d2f
Compare
|
I can't build, but I am able to fix. You need to make the function public in PagedView.java to |
wellorbetter
left a comment
wellorbetter
left a comment
There was a problem hiding this comment.
Wallpaper scrolling during wrap is now fixed — syncWithScroll uses the same mapped scroll value as the page indicator, so the wallpaper follows the wrap animation correctly.
wellorbetter
force-pushed
the
fix/infinite-scroll-wrap-animation
branch
from
42a7d2f to
eae4c23
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/com/android/launcher3/PagedView.java (1)
1381-1389:⚠️ Potential issue | 🟠 MajorWrap logic still breaks on grouped pages and can silently no-op.
Line [1381] / Line [1491] still gate wrap on raw child edges (
mCurrentPage == getChildCount() - 1), and Line [1883] derives one-page distance from adjacent child indices. In multi-panel/grouped layouts this can miss the true edge group and make wrap distance0, then Line [1788]-[1790] exits without any fallback snap.Suggested fix direction
+ private int getLastVisiblePageGroupStart() { + return getLeftmostVisiblePageForIndex(getChildCount() - 1); + } - if (!isWrapScrolling() && mCurrentPage == getChildCount() - 1) { + int lastGroupStart = getLastVisiblePageGroupStart(); + if (!isWrapScrolling() && mCurrentPage == lastGroupStart) { ... - } else if (!isWrapScrolling() && mCurrentPage == 0 && !enableFeed) { + } else if (!isWrapScrolling() && mCurrentPage == 0 && !enableFeed) { ... } - // Returns 0 on two-panel (foldable) workspaces, disabling wrap scroll. private int getOnePageDistance() { - if (!isPageScrollsInitialized() || mPageScrolls.length < 2) { + int panelCount = getPanelCount(); + if (!isPageScrollsInitialized() || mPageScrolls.length <= panelCount) { return 0; } - return Math.abs(mPageScrolls[1] - mPageScrolls[0]); + return Math.abs(getScrollForPage(panelCount) - getScrollForPage(0)); } private void snapToPageWrapped(int targetPage, int velocity) { ... - if (!isWrapScrolling()) { - return; - } + if (!isWrapScrolling()) { + snapToPageWithVelocity(targetPage, velocity); + return; + } ... }Also applies to: 1491-1514, 1788-1790, 1878-1884
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/com/android/launcher3/PagedView.java` around lines 1381 - 1389, The wrap logic currently gates wrap detection on raw child indices (mCurrentPage == getChildCount() - 1 / == 0) which breaks for grouped/multi-panel pages and can yield a zero wrap distance that causes a silent no-op; change the checks to compute the true edge group page instead of using getChildCount() directly (introduce a helper like findWrapTargetPage(boolean last) that returns the actual last/first page index for wrapping based on grouping), use that helper in the conditions around isWrapScrolling(), pulledTo, mMinScroll/mMaxScroll and pass its result into startWrapDrag(...) instead of getChildCount()-1 or 0, and add a fallback when the computed wrap distance is 0 (e.g. call snapToPage(mCurrentPage) or equivalent) to avoid silently exiting in the block around the early return at the code paths referencing lines ~1788-1790.
🧹 Nitpick comments (1)
src/com/android/launcher3/PagedView.java (1)
1379-1380: Cache feed preference reads outside the ACTION_MOVE handler.Lines 1379 and 1474 both call
PreferenceExtensionsKt.firstBlocking(prefs2.getEnableFeed())during drag handling inonTouchEvent(). Blocking reads in theACTION_MOVEpath, which fires continuously during drags, will cause frame drops. Cache this preference value at gesture start (ACTION_DOWN) or observe it outside touch dispatch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/com/android/launcher3/PagedView.java` around lines 1379 - 1380, The touch handler performs blocking preference reads during ACTION_MOVE by calling PreferenceExtensionsKt.firstBlocking(prefs2.getEnableFeed()) which can drop frames; modify onTouchEvent() to read and cache the enableFeed value at gesture start (handle ACTION_DOWN) into a local field or local variable (e.g., cachedEnableFeed) and then use that cached value in the ACTION_MOVE path (and any other move/drag logic around pulledTo/oldScroll/delta) instead of calling PreferenceExtensionsKt.firstBlocking again; ensure the cached value is invalidated or refreshed on gesture end/cancel so subsequent gestures re-read prefs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/com/android/launcher3/PagedView.java`:
- Around line 1381-1389: The wrap logic currently gates wrap detection on raw
child indices (mCurrentPage == getChildCount() - 1 / == 0) which breaks for
grouped/multi-panel pages and can yield a zero wrap distance that causes a
silent no-op; change the checks to compute the true edge group page instead of
using getChildCount() directly (introduce a helper like
findWrapTargetPage(boolean last) that returns the actual last/first page index
for wrapping based on grouping), use that helper in the conditions around
isWrapScrolling(), pulledTo, mMinScroll/mMaxScroll and pass its result into
startWrapDrag(...) instead of getChildCount()-1 or 0, and add a fallback when
the computed wrap distance is 0 (e.g. call snapToPage(mCurrentPage) or
equivalent) to avoid silently exiting in the block around the early return at
the code paths referencing lines ~1788-1790.
---
Nitpick comments:
In `@src/com/android/launcher3/PagedView.java`:
- Around line 1379-1380: The touch handler performs blocking preference reads
during ACTION_MOVE by calling
PreferenceExtensionsKt.firstBlocking(prefs2.getEnableFeed()) which can drop
frames; modify onTouchEvent() to read and cache the enableFeed value at gesture
start (handle ACTION_DOWN) into a local field or local variable (e.g.,
cachedEnableFeed) and then use that cached value in the ACTION_MOVE path (and
any other move/drag logic around pulledTo/oldScroll/delta) instead of calling
PreferenceExtensionsKt.firstBlocking again; ensure the cached value is
invalidated or refreshed on gesture end/cancel so subsequent gestures re-read
prefs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85de5b7d-fbc5-454f-84f6-66b3eb00141e
📒 Files selected for processing (3)
src/com/android/launcher3/PagedView.javasrc/com/android/launcher3/Workspace.javasrc/com/android/launcher3/util/WallpaperOffsetInterpolator.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/com/android/launcher3/util/WallpaperOffsetInterpolator.java
SuperDragonXD
left a comment
SuperDragonXD
left a comment
There was a problem hiding this comment.
Have checked this, and the behavior seems to be the same as before:
Screen_Recording_20260428_101200.mp4
We should try to match the old wallpaper behavior before this change, where the wallpaper slides from right to left, as seen here in Nova:
Screen_Recording_20260428_101357.mp4
|
@wellorbetter Are you going to take more look in it and try to work on it and fix? |
Yes, I'm heading into a break soon — I'll pick this up again when I have time. Thanks for your patience! |
wellorbetter
force-pushed
the
fix/infinite-scroll-wrap-animation
branch
from
eae4c23 to
dfaad2a
Compare
wellorbetter
force-pushed
the
fix/infinite-scroll-wrap-animation
branch
from
dfaad2a to
ebb0fad
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/com/android/launcher3/PagedView.java`:
- Around line 1847-1858: cancelWrapScroll() currently resets
mMinScroll/mMaxScroll immediately which causes a visible jump when the caller
starts a normal snap; change cancelWrapScroll() so it does NOT restore
mMinScroll/mMaxScroll right away. Instead either (A) keep the extended bounds
(leave mMinScroll/mMaxScroll as mSavedMinScroll/mSavedMaxScroll) and restore
them only after the caller has created/configured the fallback snap, or (B) if
you must restore immediately, remap/clamp the current scroll position into the
saved range (compute a mappedScroll within mSavedMinScroll..mSavedMaxScroll and
call scrollTo(mappedScroll)) before assigning mMinScroll = mSavedMinScroll and
mMaxScroll = mSavedMaxScroll. Update cancelWrapScroll() (and callers if needed)
to use approach A or B so the first scrollTo in the replacement snap is not
clamped unexpectedly; reference mWrapToPage, getPageAt(...),
mSavedMinScroll/mSavedMaxScroll, and mMinScroll/mMaxScroll when making the
change.
- Around line 1383-1385: The touch handler is blocking the UI by calling
PreferenceExtensionsKt.firstBlocking(prefs2.getEnableFeed()) on ACTION_MOVE;
cache the preference at gesture start instead of reading it per-frame: read and
store the enableFeed value once when the touch sequence begins (e.g., on
ACTION_DOWN or in the method that initializes a drag) and use that cached
boolean in the ACTION_MOVE and ACTION_UP handling (replace direct calls to
PreferenceExtensionsKt.firstBlocking(...) in the ACTION_MOVE/ACTION_UP paths).
Alternatively, subscribe to prefs2.getEnableFeed() asynchronously outside the
touch event path and read the latest cached value during the gesture.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29400f40-9103-4b26-943e-fc1be8b06582
📒 Files selected for processing (2)
src/com/android/launcher3/PagedView.javasrc/com/android/launcher3/util/WallpaperOffsetInterpolator.java
The page wraps one-page distance via a translationX trick, but the wallpaper should still sweep the full scroll range in reverse, as it did before the wrap animation and as Nova does. Add getScrollForWallpaper() that maps the out-of-bounds wrap scroll into a synthetic full-range scroll, and use it from syncWithScroll. Also exclude the wrap target's translationX from getLayoutTransitionOffsetForPage — it's a wrap trick, not a layout transition, and was zeroing out the wallpaper's adjustedScroll.
wellorbetter
force-pushed
the
fix/infinite-scroll-wrap-animation
branch
from
ebb0fad to
1c70c57
Compare
- Read enableFeed once in ACTION_DOWN instead of per-frame firstBlocking in ACTION_MOVE/ACTION_UP, avoiding a blocking DataStore read on every drag frame. - cancelWrapScroll now clamps the current scroll back into the saved range before restoring mMinScroll/mMaxScroll, so the fallback snap starts from a valid position.
wellorbetter
force-pushed
the
fix/infinite-scroll-wrap-animation
branch
from
1c70c57 to
94f75fc
Compare
|
@SuperDragonXD Have you tested this? I hope the latest fixes resolve the wallpaper scrolling. |
|
@wellorbetter I want to see this fix merged. |
I'm very sorry. Recently, I have been busy with job interviews.. I will try my best to merge this change within the next two weeks. |
3 participants
Description
Fixes the infinite scroll wrap animation so it scrolls one page in the correct swipe direction instead of animating through all intermediate pages.
Fixes #6268
Reasoning
When wrapping between first↔last page, snapToPageWithVelocity computed a delta spanning the entire page range, causing the
scroller to visually pass through every intermediate page. The core problem: pages are laid out linearly, so scrolling from page 2 to page 0 must traverse pages 2→1→0.
The fix uses setTranslationX to temporarily move the target page's View next to the current page (e.g., page 0 is moved to
the right of page 2). This lets the scroller travel just one page distance in the correct direction. Scroll bounds (mMinScroll/mMaxScroll) are temporarily extended so scrollTo doesn't clamp the position. When the animation completes (or is
interrupted), the translation is reset, bounds are restored, and scroll jumps to the target page's real position.
The page indicator is also handled: during wrap, the out-of-bounds scroll value is mapped back into the normal range so the
dots animate correctly.
Testing
Type of change
✅ Bug fix (A non-breaking change that fixes an issue)
❌ New feature (A non-breaking change that adds functionality)
❌ Breaking change (A fix or feature that would cause existing functionality to not work as expected)
❌ Refactor (A code change that neither fixes a bug nor adds a feature)
❌ Performance (A code change that improves performance)
❌ Style (Code style changes)
❌ Docs (Changes to documentation)
❌ Chore (Changes to the build process or other tooling)
Summary by CodeRabbit
New Features
Improvements