Skip to content

Commit

Permalink
track previous contentOffset to avoid duplicate calls
Browse files Browse the repository at this point in the history
Summary:
- there was a bug when `contentOffset` was set to {x:0, y:0} and there were items lazy loaded in with prop changes- the View would consistently scroll back to the top after loading in new items
- to avoid this, we store the offset value when it's first set, and only run the `scrollTo` logic then

Changelog:
[Internal][Fixed] - Fix contentOffset forcing scroll to top behavior

Reviewed By: fkgozali, mdvacca

Differential Revision: D42089871

fbshipit-source-id: 3968d98341728a45bec28e8783c9977e91dd4d2c
  • Loading branch information
skinsshark authored and facebook-github-bot committed Jan 3, 2023
1 parent 09ad0cc commit 14330b5
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@
import com.facebook.common.logging.FLog;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.R;
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.uimanager.FabricViewStateManager;
import com.facebook.react.uimanager.MeasureSpecAssertions;
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.PointerEvents;
import com.facebook.react.uimanager.ReactClippingViewGroup;
import com.facebook.react.uimanager.ReactClippingViewGroupHelper;
Expand Down Expand Up @@ -100,6 +102,7 @@ public class ReactScrollView extends ScrollView
private int mSnapToAlignment = SNAP_ALIGNMENT_DISABLED;
private @Nullable View mContentView;
private ReactViewBackgroundManager mReactBackgroundManager;
private @Nullable ReadableMap mCurrentContentOffset = null;
private int pendingContentOffsetX = UNSET_CONTENT_OFFSET;
private int pendingContentOffsetY = UNSET_CONTENT_OFFSET;
private final FabricViewStateManager mFabricViewStateManager = new FabricViewStateManager();
Expand Down Expand Up @@ -1002,6 +1005,23 @@ public void onChildViewRemoved(View parent, View child) {
mContentView = null;
}

public void setContentOffset(ReadableMap value) {
// When contentOffset={{x:0,y:0}} with lazy load items, setContentOffset
// is repeatedly called, causing an unexpected scroll to top behavior.
// Avoid updating contentOffset if the value has not changed.
if (mCurrentContentOffset == null || !mCurrentContentOffset.equals(value)) {
mCurrentContentOffset = value;

if (value != null) {
double x = value.hasKey("x") ? value.getDouble("x") : 0;
double y = value.hasKey("y") ? value.getDouble("y") : 0;
scrollTo((int) PixelUtil.toPixelFromDIP(x), (int) PixelUtil.toPixelFromDIP(y));
} else {
scrollTo(0, 0);
}
}
}

/**
* Calls `smoothScrollTo` and updates state.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,13 +320,7 @@ public void setFadingEdgeLength(ReactScrollView view, int value) {

@ReactProp(name = "contentOffset", customType = "Point")
public void setContentOffset(ReactScrollView view, ReadableMap value) {
if (value != null) {
double x = value.hasKey("x") ? value.getDouble("x") : 0;
double y = value.hasKey("y") ? value.getDouble("y") : 0;
view.scrollTo((int) PixelUtil.toPixelFromDIP(x), (int) PixelUtil.toPixelFromDIP(y));
} else {
view.scrollTo(0, 0);
}
view.setContentOffset(value);
}

@Override
Expand Down

0 comments on commit 14330b5

Please sign in to comment.