Skip to content
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 maintainVisibleContentPosition on Android during momentum scroll #43425

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private void updateScrollPositionInternal() {
int deltaX = newFrame.left - mPrevFirstVisibleFrame.left;
if (deltaX != 0) {
int scrollX = mScrollView.getScrollX();
mScrollView.scrollTo(scrollX + deltaX, mScrollView.getScrollY());
mScrollView.scrollToPreservingMomentum(scrollX + deltaX, mScrollView.getScrollY());
mPrevFirstVisibleFrame = newFrame;
if (mConfig.autoScrollToTopThreshold != null
&& scrollX <= mConfig.autoScrollToTopThreshold) {
Expand All @@ -129,7 +129,7 @@ private void updateScrollPositionInternal() {
int deltaY = newFrame.top - mPrevFirstVisibleFrame.top;
if (deltaY != 0) {
int scrollY = mScrollView.getScrollY();
mScrollView.scrollTo(mScrollView.getScrollX(), scrollY + deltaY);
mScrollView.scrollToPreservingMomentum(mScrollView.getScrollX(), scrollY + deltaY);
mPrevFirstVisibleFrame = newFrame;
if (mConfig.autoScrollToTopThreshold != null
&& scrollY <= mConfig.autoScrollToTopThreshold) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1323,6 +1323,14 @@ public void scrollTo(int x, int y) {
setPendingContentOffsets(x, y);
}

/**
* Scrolls to a new position preserving any momentum scrolling animation.
*/
public void scrollToPreservingMomentum(int x, int y) {
janicduplessis marked this conversation as resolved.
Show resolved Hide resolved
scrollTo(x, y);
recreateFlingAnimation(x, Integer.MAX_VALUE);
}

private boolean isContentReady() {
View child = getContentView();
return child != null && child.getWidth() != 0 && child.getHeight() != 0;
Expand Down Expand Up @@ -1376,24 +1384,14 @@ public void onLayoutChange(
}
}

private void adjustPositionForContentChangeRTL(int left, int right, int oldLeft, int oldRight) {
// If we have any pending custon flings (e.g. from aninmated `scrollTo`, or flinging to a snap
// point), finish them, commiting the final `scrollX`.
// TODO: Can we be more graceful (like OverScroller flings)?
if (getFlingAnimator().isRunning()) {
getFlingAnimator().end();
}

int distanceToRightEdge = oldRight - getScrollX();
int newWidth = right - left;
int scrollX = newWidth - distanceToRightEdge;
scrollTo(scrollX, getScrollY());

// If we are in the middle of a fling animation from the user removing their finger
// (OverScroller is in `FLING_MODE`), we must cancel and recreate the existing fling animation
// since it was calculated against outdated scroll offsets.
/**
* If we are in the middle of a fling animation from the user removing their finger
* (OverScroller is in `FLING_MODE`), recreate the existing fling animation since it was
* calculated against outdated scroll offsets.
*/
private void recreateFlingAnimation(int scrollX, int maxX) {
if (mScroller != null && !mScroller.isFinished()) {
// Calculate the veliocity and position of the fling animation at the time of this layout
// Calculate the velocity and position of the fling animation at the time of this layout
// event, which may be later than the last ScrollView tick. These values are not commited to
// the underlying ScrollView, which will recalculate positions on its next tick.
int scrollerXBeforeTick = mScroller.getCurrX();
Expand All @@ -1413,13 +1411,29 @@ private void adjustPositionForContentChangeRTL(int left, int right, int oldLeft,
float flingVelocityX = mScroller.getCurrVelocity() * direction;

mScroller.fling(
scrollX, getScrollY(), (int) flingVelocityX, 0, 0, newWidth - getWidth(), 0, 0);
scrollX, getScrollY(), (int) flingVelocityX, 0, 0, maxX, 0, 0);
} else {
scrollTo(scrollX + (mScroller.getCurrX() - scrollerXBeforeTick), getScrollY());
}
}
}

private void adjustPositionForContentChangeRTL(int left, int right, int oldLeft, int oldRight) {
// If we have any pending custon flings (e.g. from aninmated `scrollTo`, or flinging to a snap
// point), finish them, commiting the final `scrollX`.
// TODO: Can we be more graceful (like OverScroller flings)?
if (getFlingAnimator().isRunning()) {
getFlingAnimator().end();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this piece still relevant? I.e. if adjustment happens during programmatically triggered fling?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be pretty rare cases, but I added the extra logic just in case.

There I used cancel instead of end since I do not want to commit the last scroll position of the animation. I think it is better to cancel the scroll still than to have content jumps because of the animation being wrong.


int distanceToRightEdge = oldRight - getScrollX();
int newWidth = right - left;
int scrollX = newWidth - distanceToRightEdge;
scrollTo(scrollX, getScrollY());

recreateFlingAnimation(scrollX, newWidth - getWidth());
}

@Nullable
public StateWrapper getStateWrapper() {
return mStateWrapper;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,48 @@ public void scrollTo(int x, int y) {
setPendingContentOffsets(x, y);
}

/**
* If we are in the middle of a fling animation from the user removing their finger
* (OverScroller is in `FLING_MODE`), recreate the existing fling animation since it was
* calculated against outdated scroll offsets.
*/
private void recreateFlingAnimation(int scrollY) {
if (mScroller != null && !mScroller.isFinished()) {
// Calculate the velocity and position of the fling animation at the time of this layout
// event, which may be later than the last ScrollView tick. These values are not committed to
// the underlying ScrollView, which will recalculate positions on its next tick.
int scrollerYBeforeTick = mScroller.getCurrY();
boolean hasMoreTicks = mScroller.computeScrollOffset();

// Stop the existing animation at the current state of the scroller. We will then recreate
// it starting at the adjusted y offset.
mScroller.forceFinished(true);

if (hasMoreTicks) {
// OverScroller.getCurrVelocity() returns an absolute value of the velocity a current fling
// animation (only FLING_MODE animations). We derive direction along the Y axis from the
// start and end of the, animation assuming ScrollView never fires horizontal fling
// animations.
// TODO: This does not fully handle overscroll.
float direction = Math.signum(mScroller.getFinalY() - mScroller.getStartY());
float flingVelocityY = mScroller.getCurrVelocity() * direction;

mScroller.fling(
getScrollX(), scrollY, 0, (int) flingVelocityY, 0, 0, 0, Integer.MAX_VALUE);
} else {
scrollTo(getScrollX(), scrollY + (mScroller.getCurrX() - scrollerYBeforeTick));
}
}
}

/**
* Scrolls to a new position preserving any momentum scrolling animation.
*/
public void scrollToPreservingMomentum(int x, int y) {
scrollTo(x, y);
recreateFlingAnimation(y);
}

private boolean isContentReady() {
View child = getContentView();
return child != null && child.getWidth() != 0 && child.getHeight() != 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,5 +597,7 @@ public interface HasScrollEventThrottle {

public interface HasSmoothScroll {
void reactSmoothScrollTo(int x, int y);

void scrollToPreservingMomentum(int x, int y);
}
}
Loading