Skip to content

Commit 984dbe8

Browse files
sandeee1928meta-codesync[bot]
authored andcommitted
Reset contentInset in scroll view prepareForRecycle (facebook#55733)
Summary: Pull Request resolved: facebook#55733 React Native's Fabric view recycling pools UIScrollView instances by component type, meaning vertical and horizontal scroll views share the same pool. When a scroll view is recycled via prepareForRecycle, contentInset is not reset. This causes stale bottom insets set at runtime (e.g., by the floating tab bar's content inset adjustment) to leak into recycled scroll views. This manifests as horizontal FlatLists allowing vertical/diagonal scrolling because they inherit a non-zero bottom contentInset from a previously vertical scroll view. Fix: Reset contentInset to UIEdgeInsetsZero in prepareForRecycle, consistent with how contentOffset, zoomScale, and contentInsetAdjustmentBehavior are already reset. Changelog: Internal More Context: https://fb.workplace.com/groups/rn.support/posts/30810820418539846/?comment_id=30811286738493214&reply_comment_id=30821628410792380 Reviewed By: cipolleschi Differential Revision: D93342311 fbshipit-source-id: 84c4597596e36a535f9a10840b1ddf280f6402df
1 parent 4d66a26 commit 984dbe8

File tree

3 files changed

+86
-4
lines changed

3 files changed

+86
-4
lines changed

packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,8 @@ - (void)prepareForRecycle
684684
_scrollView.contentOffset = RCTCGPointFromPoint(props.contentOffset);
685685
// Reset zoom scale to default
686686
_scrollView.zoomScale = 1.0;
687+
// Reset contentInset to prevent stale insets leaking into recycled scroll views.
688+
_scrollView.contentInset = UIEdgeInsetsZero;
687689
// We set the default behavior to "never" so that iOS
688690
// doesn't do weird things to UIScrollView insets automatically
689691
// and keeps it as an opt-in behavior.

packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewState.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class ScrollViewState final {
2828

2929
Point contentOffset;
3030
Rect contentBoundingRect;
31-
int scrollAwayPaddingTop;
31+
int scrollAwayPaddingTop{0};
3232

3333
/*
3434
* View Culling has to be disabled when accessibility features are used.

packages/react-native/ReactCommon/react/renderer/components/scrollview/tests/ScrollViewTest.cpp

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,90 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
#include <memory>
8+
#include <react/renderer/components/scrollview/ScrollViewState.h>
99

1010
#include <gtest/gtest.h>
1111

12-
TEST(ScrollViewTest, testSomething) {
13-
// TODO
12+
namespace facebook::react {
13+
14+
TEST(ScrollViewStateTest, defaultConstructor) {
15+
ScrollViewState state;
16+
17+
EXPECT_EQ(state.contentOffset.x, 0);
18+
EXPECT_EQ(state.contentOffset.y, 0);
19+
EXPECT_EQ(state.scrollAwayPaddingTop, 0);
20+
EXPECT_FALSE(state.disableViewCulling);
21+
}
22+
23+
TEST(ScrollViewStateTest, parameterizedConstructor) {
24+
Point contentOffset{.x = 10.0f, .y = 20.0f};
25+
Rect contentBoundingRect{
26+
.origin = {.x = 0.0f, .y = 0.0f},
27+
.size = {.width = 100.0f, .height = 200.0f}};
28+
int scrollAwayPaddingTop = 5;
29+
30+
ScrollViewState state(
31+
contentOffset, contentBoundingRect, scrollAwayPaddingTop);
32+
33+
EXPECT_EQ(state.contentOffset.x, 10.0f);
34+
EXPECT_EQ(state.contentOffset.y, 20.0f);
35+
EXPECT_EQ(state.scrollAwayPaddingTop, 5);
36+
EXPECT_FALSE(state.disableViewCulling);
1437
}
38+
39+
TEST(ScrollViewStateTest, getContentSize) {
40+
Point contentOffset{.x = 0.0f, .y = 0.0f};
41+
Rect contentBoundingRect{
42+
.origin = {.x = 0.0f, .y = 0.0f},
43+
.size = {.width = 150.0f, .height = 300.0f}};
44+
int scrollAwayPaddingTop = 0;
45+
46+
ScrollViewState state(
47+
contentOffset, contentBoundingRect, scrollAwayPaddingTop);
48+
49+
Size contentSize = state.getContentSize();
50+
EXPECT_EQ(contentSize.width, 150.0f);
51+
EXPECT_EQ(contentSize.height, 300.0f);
52+
}
53+
54+
TEST(ScrollViewStateTest, disableViewCulling) {
55+
ScrollViewState state;
56+
57+
// Default should be false
58+
EXPECT_FALSE(state.disableViewCulling);
59+
60+
// Can be set to true
61+
state.disableViewCulling = true;
62+
EXPECT_TRUE(state.disableViewCulling);
63+
}
64+
65+
TEST(ScrollViewStateTest, contentOffsetWithNegativeValues) {
66+
Point contentOffset{.x = -10.0f, .y = -20.0f};
67+
Rect contentBoundingRect{
68+
.origin = {.x = 0.0f, .y = 0.0f},
69+
.size = {.width = 100.0f, .height = 200.0f}};
70+
int scrollAwayPaddingTop = 0;
71+
72+
ScrollViewState state(
73+
contentOffset, contentBoundingRect, scrollAwayPaddingTop);
74+
75+
EXPECT_EQ(state.contentOffset.x, -10.0f);
76+
EXPECT_EQ(state.contentOffset.y, -20.0f);
77+
}
78+
79+
TEST(ScrollViewStateTest, zeroSizeContentBoundingRect) {
80+
Point contentOffset{.x = 0.0f, .y = 0.0f};
81+
Rect contentBoundingRect{
82+
.origin = {.x = 0.0f, .y = 0.0f},
83+
.size = {.width = 0.0f, .height = 0.0f}};
84+
int scrollAwayPaddingTop = 0;
85+
86+
ScrollViewState state(
87+
contentOffset, contentBoundingRect, scrollAwayPaddingTop);
88+
89+
Size contentSize = state.getContentSize();
90+
EXPECT_EQ(contentSize.width, 0.0f);
91+
EXPECT_EQ(contentSize.height, 0.0f);
92+
}
93+
94+
} // namespace facebook::react

0 commit comments

Comments
 (0)