From e5ba6ab7b482c380e35765b898e522e9d4e1d3b0 Mon Sep 17 00:00:00 2001 From: Xin Chen Date: Wed, 6 Jul 2022 12:18:37 -0700 Subject: [PATCH] Allow horizontal scroll view also retry side-effects when content view is not mounted Summary: This diff fixed a NPE in horizontal scroll view when calling scrollToEnd API in side effect. The problem here is we may trigger side-effects before the required view got mounted for performance reasons. We've been fixing this with retry logic on those side-effects and we've already done this in vertical scroll view. This is to fix on the horizontal scroll as well. Changelog: [Android][Fixed] - Fixed HorizontalScrollView API scrollToEnd causing NPE in side-effects. Reviewed By: lunaleaps, JoshuaGross Differential Revision: D37571847 fbshipit-source-id: 0a4dc38381008350fd09908aa3ebb64e3e65a424 --- .../scroll/ReactHorizontalScrollViewManager.java | 13 +++++++++++-- .../react/views/scroll/ReactScrollViewManager.java | 3 +++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollViewManager.java index d86b3404c56a84..aada6a7c4079b2 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollViewManager.java @@ -8,10 +8,12 @@ package com.facebook.react.views.scroll; import android.graphics.Color; +import android.view.View; import androidx.annotation.Nullable; import androidx.core.view.ViewCompat; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; +import com.facebook.react.bridge.RetryableMountingLayerException; import com.facebook.react.module.annotations.ReactModule; import com.facebook.react.uimanager.PixelUtil; import com.facebook.react.uimanager.PointerEvents; @@ -208,8 +210,15 @@ public void scrollTo( public void scrollToEnd( ReactHorizontalScrollView scrollView, ReactScrollViewCommandHelper.ScrollToEndCommandData data) { - // ScrollView always has one child - the scrollable area - int right = scrollView.getChildAt(0).getWidth() + scrollView.getPaddingRight(); + // ScrollView always has one child - the scrollable area. However, it's possible today that we + // execute this method as view command before the child view is mounted. Here we will retry the + // view commands as a workaround. + @Nullable View child = scrollView.getChildAt(0); + if (child == null) { + throw new RetryableMountingLayerException( + "scrollToEnd called on HorizontalScrollView without child"); + } + int right = child.getWidth() + scrollView.getPaddingRight(); if (data.mAnimated) { scrollView.reactSmoothScrollTo(right, scrollView.getScrollY()); } else { diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java index 2eb7b49781ccf4..933455e8056ac6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java @@ -285,6 +285,9 @@ public void setOverflow(ReactScrollView view, @Nullable String overflow) { @Override public void scrollToEnd( ReactScrollView scrollView, ReactScrollViewCommandHelper.ScrollToEndCommandData data) { + // ScrollView always has one child - the scrollable area. However, it's possible today that we + // execute this method as view command before the child view is mounted. Here we will retry the + // view commands as a workaround. View child = scrollView.getChildAt(0); if (child == null) { throw new RetryableMountingLayerException("scrollToEnd called on ScrollView without child");