Skip to content

Commit 8ee30ba

Browse files
Reland: [Android] Fix Text and ScrollView payloads that Props 2.0 sends wrong or drops (#59388)
Summary: Reland of #59080, which was reverted together with #59079. Code outside this repository that compiles against these headers also read `snapToAlignment` as a non-optional value. With Props 2.0 on Android (`enableExclusivePropsUpdateAndroid` and the related flags), the Text and ScrollView payloads come from `getDiffProps`. Some props come out wrong: - A removed Text `letterSpacing`: sent as `NaN`, which Java ignores, so the old spacing stays. - `scrollsChildToFocus`: never sent. - `snapToAlignment`: an explicit `"start"` is not sent on mount, and removing another alignment sends `"start"`. - `decelerationRate={0.998}`: not sent, because the C++ default is the iOS rate, 0.998. Android ScrollViews start at 0.985. - A removed `nestedScrollEnabled` or `sendMomentumEvents`: stays on. This PR sends `null` for a removed `letterSpacing`, adds `scrollsChildToFocus` on Android, makes `snapToAlignment` optional, defaults `decelerationRate` to 0.985 on Android, and resets the two booleans to `false`. iOS keeps its defaults. Nothing changes on Android with the flags off. One difference from the flags-off path is intended. A removed `decelerationRate` now restores 0.985. With the flags off it reaches Java as `null`, which the setter reads as 0, so a fling stops at once. ## Changelog: [ANDROID] [FIXED] - Fix Text letterSpacing and ScrollView scrollsChildToFocus, snapToAlignment, decelerationRate, nestedScrollEnabled and sendMomentumEvents with Props 2.0 Differential Revision: D126922845
1 parent 074e081 commit 8ee30ba

11 files changed

Lines changed: 145 additions & 25 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,8 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
412412

413413
auto *scrollView = (RCTEnhancedScrollView *)_scrollView;
414414
if (oldScrollViewProps.snapToAlignment != newScrollViewProps.snapToAlignment) {
415-
scrollView.snapToAlignment = RCTNSStringFromString(toString(newScrollViewProps.snapToAlignment));
415+
scrollView.snapToAlignment =
416+
RCTNSStringFromString(toString(newScrollViewProps.snapToAlignment.value_or(ScrollViewSnapToAlignment::Start)));
416417
}
417418

418419
scrollView.snapToStart = newScrollViewProps.snapToStart;

‎packages/react-native/ReactCommon/react/renderer/components/scrollview/BaseScrollViewProps.cpp‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ BaseScrollViewProps::BaseScrollViewProps(
8080
rawProps,
8181
"decelerationRate",
8282
sourceProps.decelerationRate,
83-
(Float)0.998)),
83+
kDefaultDecelerationRate)),
8484
endDraggingSensitivityMultiplier(convertRawProp(
8585
context,
8686
rawProps,
@@ -427,10 +427,7 @@ SharedDebugStringConvertibleList BaseScrollViewProps::getDebugProps() const {
427427
"snapToInterval",
428428
snapToInterval,
429429
defaultScrollViewProps.snapToInterval),
430-
debugStringConvertibleItem(
431-
"snapToAlignment",
432-
snapToAlignment,
433-
defaultScrollViewProps.snapToAlignment),
430+
debugStringConvertibleItem("snapToAlignment", snapToAlignment),
434431
debugStringConvertibleItem(
435432
"disableIntervalMomentum",
436433
disableIntervalMomentum,

‎packages/react-native/ReactCommon/react/renderer/components/scrollview/BaseScrollViewProps.h‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,13 @@ class BaseScrollViewProps : public ViewProps {
4141
bool automaticallyAdjustContentInsets{};
4242
bool automaticallyAdjustsScrollIndicatorInsets{true};
4343
bool automaticallyAdjustKeyboardInsets{false};
44-
Float decelerationRate{0.998f};
44+
#ifdef ANDROID
45+
// ReactScrollView's initial rate.
46+
static constexpr Float kDefaultDecelerationRate = 0.985f;
47+
#else
48+
static constexpr Float kDefaultDecelerationRate = 0.998f;
49+
#endif
50+
Float decelerationRate{kDefaultDecelerationRate};
4551
Float endDraggingSensitivityMultiplier{1};
4652
bool directionalLockEnabled{};
4753
ScrollViewIndicatorStyle indicatorStyle{};
@@ -63,7 +69,7 @@ class BaseScrollViewProps : public ViewProps {
6369
Point contentOffset{};
6470
EdgeInsets scrollIndicatorInsets{};
6571
Float snapToInterval{};
66-
ScrollViewSnapToAlignment snapToAlignment{};
72+
std::optional<ScrollViewSnapToAlignment> snapToAlignment{};
6773
bool disableIntervalMomentum{false};
6874
std::vector<Float> snapToOffsets{};
6975
bool snapToStart{true};

‎packages/react-native/ReactCommon/react/renderer/components/scrollview/platform/android/react/renderer/components/scrollview/HostPlatformScrollViewProps.cpp‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,13 @@ HostPlatformScrollViewProps::HostPlatformScrollViewProps(
4949
rawProps,
5050
"endFillColor",
5151
sourceProps.endFillColor,
52-
clearColor())) {}
52+
clearColor())),
53+
scrollsChildToFocus(convertRawProp(
54+
context,
55+
rawProps,
56+
"scrollsChildToFocus",
57+
sourceProps.scrollsChildToFocus,
58+
true)) {}
5359

5460
void HostPlatformScrollViewProps::setProp(
5561
const PropsParserContext& context,
@@ -69,6 +75,7 @@ void HostPlatformScrollViewProps::setProp(
6975
RAW_SET_PROP_SWITCH_CASE_BASIC(fadingEdgeLength);
7076
RAW_SET_PROP_SWITCH_CASE_BASIC(overScrollMode);
7177
RAW_SET_PROP_SWITCH_CASE_BASIC(endFillColor);
78+
RAW_SET_PROP_SWITCH_CASE_BASIC(scrollsChildToFocus);
7279
}
7380
}
7481

@@ -96,7 +103,11 @@ SharedDebugStringConvertibleList HostPlatformScrollViewProps::getDebugProps()
96103
debugStringConvertibleItem(
97104
"endFillColor",
98105
endFillColor,
99-
defaultScrollViewProps.endFillColor)};
106+
defaultScrollViewProps.endFillColor),
107+
debugStringConvertibleItem(
108+
"scrollsChildToFocus",
109+
scrollsChildToFocus,
110+
defaultScrollViewProps.scrollsChildToFocus)};
100111
}
101112
#endif
102113

@@ -301,17 +312,9 @@ folly::dynamic HostPlatformScrollViewProps::getDiffProps(
301312
}
302313

303314
if (snapToAlignment != oldProps->snapToAlignment) {
304-
switch (snapToAlignment) {
305-
case ScrollViewSnapToAlignment::Start:
306-
result["snapToAlignment"] = "start";
307-
break;
308-
case ScrollViewSnapToAlignment::Center:
309-
result["snapToAlignment"] = "center";
310-
break;
311-
case ScrollViewSnapToAlignment::End:
312-
result["snapToAlignment"] = "end";
313-
break;
314-
}
315+
result["snapToAlignment"] = snapToAlignment.has_value()
316+
? toString(*snapToAlignment)
317+
: folly::dynamic(nullptr);
315318
}
316319

317320
if (disableIntervalMomentum != oldProps->disableIntervalMomentum) {
@@ -380,6 +383,10 @@ folly::dynamic HostPlatformScrollViewProps::getDiffProps(
380383
result["endFillColor"] = *endFillColor;
381384
}
382385

386+
if (scrollsChildToFocus != oldProps->scrollsChildToFocus) {
387+
result["scrollsChildToFocus"] = scrollsChildToFocus;
388+
}
389+
383390
return result;
384391
}
385392

‎packages/react-native/ReactCommon/react/renderer/components/scrollview/platform/android/react/renderer/components/scrollview/HostPlatformScrollViewProps.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ class HostPlatformScrollViewProps : public BaseScrollViewProps {
3838
folly::dynamic fadingEdgeLength{};
3939
std::string overScrollMode{"auto"};
4040
SharedColor endFillColor{clearColor()};
41+
bool scrollsChildToFocus{true};
4142

4243
#pragma mark - DebugStringConvertible
4344

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#include <gtest/gtest.h>
9+
10+
#include <react/renderer/components/scrollview/ScrollViewProps.h>
11+
#include <react/test_utils/PropsPayloads.h>
12+
13+
#ifdef RN_SERIALIZABLE_STATE
14+
15+
namespace facebook::react {
16+
17+
namespace {
18+
19+
using folly::dynamic;
20+
21+
void expectPayloads(const std::vector<PropsCommit>& commits) {
22+
expectPayloadsMatchRawProps<ScrollViewProps>(commits);
23+
}
24+
25+
} // namespace
26+
27+
// ReactScrollViewManager has a setter for it; with
28+
// enableExclusivePropsUpdateAndroid there is no rawProps fallback.
29+
TEST(ScrollViewPropsDiffTest, emitsScrollsChildToFocus) {
30+
expectPayloads({{dynamic::object("scrollsChildToFocus", false)}});
31+
}
32+
33+
// Android ScrollViews start with snapping alignment disabled, and
34+
// setSnapToAlignment disables it for null.
35+
TEST(
36+
ScrollViewPropsDiffTest,
37+
keepsAnExplicitSnapToAlignmentStartApartFromUnset) {
38+
expectPayloads({
39+
{dynamic::object("snapToAlignment", "start")},
40+
{dynamic::object("snapToAlignment", "center")},
41+
{dynamic::object("snapToAlignment", nullptr)},
42+
});
43+
}
44+
45+
// Android ScrollViews start decelerating at 0.985 (ReactScrollViewHelper).
46+
// A removed rate restores it; Props 1.5 sends null, which the setter reads as
47+
// 0.
48+
TEST(ScrollViewPropsDiffTest, emitsAnExplicitDecelerationRate) {
49+
expectPayloads({
50+
{dynamic::object("decelerationRate", 0.998)},
51+
{dynamic::object("decelerationRate", nullptr),
52+
dynamic::object("decelerationRate", 0.985)},
53+
});
54+
}
55+
56+
// The ScrollView managers turn both off for null.
57+
TEST(ScrollViewPropsDiffTest, removingNestedScrollAndMomentumEventsEmitsFalse) {
58+
expectPayloads({
59+
{dynamic::object("nestedScrollEnabled", true)(
60+
"sendMomentumEvents", true)},
61+
{dynamic::object("nestedScrollEnabled", nullptr)(
62+
"sendMomentumEvents", nullptr),
63+
dynamic::object("nestedScrollEnabled", false)(
64+
"sendMomentumEvents", false)},
65+
});
66+
}
67+
68+
} // namespace facebook::react
69+
70+
#endif

‎packages/react-native/ReactCommon/react/renderer/components/text/BaseTextProps.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#include <react/renderer/debug/DebugStringConvertibleItem.h>
1414
#include <react/utils/FloatComparison.h>
1515

16+
#include <cmath>
17+
1618
namespace facebook::react {
1719

1820
static TextAttributes convertRawProp(
@@ -436,7 +438,9 @@ void BaseTextProps::appendTextAttributesProps(
436438
if (!floatEquality(
437439
textAttributes.letterSpacing,
438440
oldProps->textAttributes.letterSpacing)) {
439-
result["letterSpacing"] = textAttributes.letterSpacing;
441+
result["letterSpacing"] = std::isnan(textAttributes.letterSpacing)
442+
? folly::dynamic(nullptr)
443+
: textAttributes.letterSpacing;
440444
}
441445

442446
if (textAttributes.textTransform != oldProps->textAttributes.textTransform) {
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#include <gtest/gtest.h>
9+
10+
#include <react/renderer/components/text/ParagraphProps.h>
11+
#include <react/test_utils/PropsPayloads.h>
12+
13+
#ifdef RN_SERIALIZABLE_STATE
14+
15+
namespace facebook::react {
16+
17+
using folly::dynamic;
18+
19+
// ReactTextViewManager resets letterSpacing to 0 for null;
20+
// ReactTextView.setLetterSpacing ignores NaN.
21+
TEST(ParagraphPropsDiffTest, removingLetterSpacingEmitsNull) {
22+
expectPayloadsMatchRawProps<ParagraphProps>({
23+
{dynamic::object("letterSpacing", 4.0)},
24+
{dynamic::object("letterSpacing", nullptr)},
25+
});
26+
}
27+
28+
} // namespace facebook::react
29+
30+
#endif

‎scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1439,8 +1439,9 @@ class facebook::react::BaseScrollViewProps : public facebook::react::HostPlatfor
14391439
public facebook::react::Point contentOffset;
14401440
public facebook::react::ScrollViewIndicatorStyle indicatorStyle;
14411441
public facebook::react::ScrollViewKeyboardDismissMode keyboardDismissMode;
1442-
public facebook::react::ScrollViewSnapToAlignment snapToAlignment;
1442+
public static constexpr facebook::react::Float kDefaultDecelerationRate;
14431443
public std::optional<facebook::react::ScrollViewMaintainVisibleContentPosition> maintainVisibleContentPosition;
1444+
public std::optional<facebook::react::ScrollViewSnapToAlignment> snapToAlignment;
14441445
public std::vector<facebook::react::Float> snapToOffsets;
14451446
public void setProp(const facebook::react::PropsParserContext& context, facebook::react::RawPropsPropNameHash hash, const char* propName, const facebook::react::RawValue& value);
14461447
}
@@ -2049,6 +2050,7 @@ class facebook::react::HostPlatformScrollViewProps : public facebook::react::Bas
20492050
public HostPlatformScrollViewProps() = default;
20502051
public HostPlatformScrollViewProps(const facebook::react::PropsParserContext& context, const facebook::react::HostPlatformScrollViewProps& sourceProps, const facebook::react::RawProps& rawProps);
20512052
public bool nestedScrollEnabled;
2053+
public bool scrollsChildToFocus;
20522054
public bool sendMomentumEvents;
20532055
public facebook::react::SharedColor endFillColor;
20542056
public folly::dynamic fadingEdgeLength;

‎scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4024,8 +4024,9 @@ class facebook::react::BaseScrollViewProps : public facebook::react::HostPlatfor
40244024
public facebook::react::Point contentOffset;
40254025
public facebook::react::ScrollViewIndicatorStyle indicatorStyle;
40264026
public facebook::react::ScrollViewKeyboardDismissMode keyboardDismissMode;
4027-
public facebook::react::ScrollViewSnapToAlignment snapToAlignment;
4027+
public static constexpr facebook::react::Float kDefaultDecelerationRate;
40284028
public std::optional<facebook::react::ScrollViewMaintainVisibleContentPosition> maintainVisibleContentPosition;
4029+
public std::optional<facebook::react::ScrollViewSnapToAlignment> snapToAlignment;
40294030
public std::vector<facebook::react::Float> snapToOffsets;
40304031
public void setProp(const facebook::react::PropsParserContext& context, facebook::react::RawPropsPropNameHash hash, const char* propName, const facebook::react::RawValue& value);
40314032
}

0 commit comments

Comments
 (0)