Skip to content

Commit

Permalink
Fix ReactHorizontalScrollView contentOffset
Browse files Browse the repository at this point in the history
Summary:
Brings the same fix https://www.internalfb.com/diff/D34015853 (be260b9) for ReactScrollView to ReactHorizontalScrollView

When setting ScrollView's contentOffset, if the ScrollView hasn't been laid out yet when ReactHorizontalScrollViewManager.setContentOffset is called, then scroll position is never set properly. This is because the actual scroll offset (0, 0) was being passed into setPendingContentOffsets, instead of the desired scroll offset. Thus, when ReactHorizontalScrollView.onLayout gets called, ReactHorizontalScrollView.scrollTo gets called with (0, 0).

Changelog:
[Android][Fixed] - Fix ReactHorizontalScrollView contentOffset

Reviewed By: bvanderhoof

Differential Revision: D34246489

fbshipit-source-id: d923f7c9f136f7275d64bd658ffd5c2cc049d392
  • Loading branch information
genkikondo authored and facebook-github-bot committed Feb 15, 2022
1 parent 90b98ef commit 9f6f971
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -661,8 +661,7 @@ private int getSnapInterval() {
}

private View getContentView() {
View contentView = getChildAt(0);
return contentView;
return getChildAt(0);
}

public void setEndFillColor(int color) {
Expand Down Expand Up @@ -1191,12 +1190,13 @@ public void scrollTo(int x, int y) {
}

super.scrollTo(x, y);
// The final scroll position might be different from (x, y). For example, we may need to scroll
// to the last item in the list, but that item cannot be move to the start position of the view.
final int actualX = getScrollX();
final int actualY = getScrollY();
ReactScrollViewHelper.updateFabricScrollState(this, actualX, actualY);
setPendingContentOffsets(actualX, actualY);
ReactScrollViewHelper.updateFabricScrollState(this);
setPendingContentOffsets(x, y);
}

private boolean isContentReady() {
View child = getContentView();
return child != null && child.getWidth() != 0 && child.getHeight() != 0;
}

/**
Expand All @@ -1210,8 +1210,8 @@ private void setPendingContentOffsets(int x, int y) {
if (DEBUG_MODE) {
FLog.i(TAG, "setPendingContentOffsets[%d] x %d y %d", getId(), x, y);
}
View child = getContentView();
if (child != null && child.getWidth() != 0 && child.getHeight() != 0) {

if (isContentReady()) {
pendingContentOffsetX = UNSET_CONTENT_OFFSET;
pendingContentOffsetY = UNSET_CONTENT_OFFSET;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -954,10 +954,13 @@ public void reactSmoothScrollTo(int x, int y) {
}

/**
* Calls `updateFabricScrollState` and updates state.
* Calls `super.scrollTo` and updates state.
*
* <p>`scrollTo` changes `contentOffset` and we need to keep `contentOffset` in sync between
* scroll view and state. Calling ScrollView's `scrollTo` doesn't update state.
* <p>`super.scrollTo` changes `contentOffset` and we need to keep `contentOffset` in sync between
* scroll view and state.
*
* <p>Note that while we can override scrollTo, we *cannot* override `smoothScrollTo` because it
* is final. See `reactSmoothScrollTo`.
*/
@Override
public void scrollTo(int x, int y) {
Expand All @@ -967,7 +970,7 @@ public void scrollTo(int x, int y) {
}

private boolean isContentReady() {
View child = getChildAt(0);
View child = getContentView();
return child != null && child.getWidth() != 0 && child.getHeight() != 0;
}

Expand Down

0 comments on commit 9f6f971

Please sign in to comment.