Skip to content

Commit f9b6791

Browse files
Declare ScrollView's fadingEdgeLength once, in the shared props
fadingEdgeLength is a shared ScrollView prop in Flow and TypeScript, but the C++ props declared it twice: on Android's HostPlatformScrollViewProps and on a cxx HostPlatformScrollViewProps written only to carry it. It now lives in BaseScrollViewProps, where both platforms parse it, and the cxx props are an alias of the base again. Android's diff keeps reporting it among the base props. horizontal now resets to false when the prop is removed, as its declared default is. It reset to true, which only iOS reads, so a ScrollView whose horizontal prop was removed faded along its width.
1 parent e818a22 commit f9b6791

10 files changed

Lines changed: 55 additions & 73 deletions

File tree

‎packages/react-native/React/Tests/Mounting/RCTScrollViewComponentViewTests.mm‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,18 @@
1010
#import <react/renderer/components/scrollview/ScrollViewProps.h>
1111
#import <react/renderer/components/scrollview/ScrollViewShadowNode.h>
1212
#import <react/renderer/core/LayoutMetrics.h>
13+
#import <react/renderer/core/PropsParserContext.h>
14+
#import <react/renderer/core/RawProps.h>
15+
#import <react/renderer/core/RawPropsParser.h>
16+
#import <react/utils/ContextContainer.h>
1317

18+
using facebook::react::ContextContainer;
1419
using facebook::react::EmptyLayoutMetrics;
1520
using facebook::react::LayoutMetrics;
1621
using facebook::react::Props;
22+
using facebook::react::PropsParserContext;
23+
using facebook::react::RawProps;
24+
using facebook::react::RawPropsParser;
1725
using facebook::react::ScrollViewProps;
1826
using facebook::react::ScrollViewShadowNode;
1927

@@ -34,6 +42,17 @@
3442
return props;
3543
}
3644

45+
static Props::Shared parseScrollViewProps(const folly::dynamic &rawValue, const ScrollViewProps &sourceProps)
46+
{
47+
ContextContainer contextContainer{};
48+
PropsParserContext parserContext{-1, contextContainer};
49+
auto rawProps = RawProps(rawValue);
50+
auto parser = RawPropsParser();
51+
parser.prepare<ScrollViewProps>();
52+
rawProps.parse(parser);
53+
return std::make_shared<const ScrollViewProps>(parserContext, sourceProps, rawProps);
54+
}
55+
3756
@interface RCTScrollViewComponentView (Tests)
3857
- (void)_keyboardWillChangeFrame:(NSNotification *)notification;
3958
@end
@@ -120,6 +139,24 @@ - (void)testHorizontalFadingEdgeLengthFadesAlongTheWidth
120139
[self assertLocations:mask.locations equal:@[ @0, @0.2, @1, @1 ]];
121140
}
122141

142+
- (void)testParsedFadingEdgeFollowsTheAxisWhenHorizontalIsRemoved
143+
{
144+
RCTScrollViewComponentView *view = [[RCTScrollViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 100, 200)];
145+
auto horizontalProps =
146+
parseScrollViewProps(folly::dynamic::object("fadingEdgeLength", 20)("horizontal", true), ScrollViewProps{});
147+
[view updateProps:horizontalProps oldProps:ScrollViewShadowNode::defaultSharedProps()];
148+
XCTAssertTrue(CGPointEqualToPoint([self fadingEdgeMaskOf:view].startPoint, CGPointMake(0, 0.5)));
149+
150+
auto verticalProps = parseScrollViewProps(
151+
folly::dynamic::object("horizontal", nullptr), static_cast<const ScrollViewProps &>(*horizontalProps));
152+
[view updateProps:verticalProps oldProps:horizontalProps];
153+
154+
CAGradientLayer *mask = [self fadingEdgeMaskOf:view];
155+
XCTAssertTrue(CGPointEqualToPoint(mask.startPoint, CGPointMake(0.5, 0)));
156+
XCTAssertTrue(CGPointEqualToPoint(mask.endPoint, CGPointMake(0.5, 1)));
157+
[self assertLocations:mask.locations equal:@[ @0, @0.1, @0.9, @1 ]];
158+
}
159+
123160
- (void)testFadingEdgesNeverOverlap
124161
{
125162
RCTScrollViewComponentView *view = [[RCTScrollViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 100, 200)];

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,13 @@ BaseScrollViewProps::BaseScrollViewProps(
170170
rawProps,
171171
"horizontal",
172172
sourceProps.horizontal,
173-
true)),
173+
false)),
174+
fadingEdgeLength(convertRawProp(
175+
context,
176+
rawProps,
177+
"fadingEdgeLength",
178+
sourceProps.fadingEdgeLength,
179+
nullptr)),
174180
scrollEventThrottle(convertRawProp(
175181
context,
176182
rawProps,
@@ -294,6 +300,7 @@ void BaseScrollViewProps::setProp(
294300
RAW_SET_PROP_SWITCH_CASE_BASIC(showsVerticalScrollIndicator);
295301
RAW_SET_PROP_SWITCH_CASE_BASIC(persistentScrollbar);
296302
RAW_SET_PROP_SWITCH_CASE_BASIC(horizontal);
303+
RAW_SET_PROP_SWITCH_CASE_BASIC(fadingEdgeLength);
297304
RAW_SET_PROP_SWITCH_CASE_BASIC(scrollEventThrottle);
298305
RAW_SET_PROP_SWITCH_CASE_BASIC(zoomScale);
299306
RAW_SET_PROP_SWITCH_CASE_BASIC(contentInset);

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include <React/RendererCore.h>
1313
#include <React/View.h>
14+
#include <folly/dynamic.h>
1415
#include <react/renderer/components/scrollview/primitives.h>
1516

1617
#include <optional>
@@ -57,6 +58,7 @@ class BaseScrollViewProps : public ViewProps {
5758
bool showsVerticalScrollIndicator{true};
5859
bool persistentScrollbar{false};
5960
bool horizontal{false};
61+
folly::dynamic fadingEdgeLength{};
6062
Float scrollEventThrottle{};
6163
Float zoomScale{1.0f};
6264
EdgeInsets contentInset{};

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

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,6 @@ HostPlatformScrollViewProps::HostPlatformScrollViewProps(
3232
"nestedScrollEnabled",
3333
sourceProps.nestedScrollEnabled,
3434
true)),
35-
fadingEdgeLength(convertRawProp(
36-
context,
37-
rawProps,
38-
"fadingEdgeLength",
39-
sourceProps.fadingEdgeLength,
40-
nullptr)),
4135
overScrollMode(convertRawProp(
4236
context,
4337
rawProps,
@@ -66,7 +60,6 @@ void HostPlatformScrollViewProps::setProp(
6660
switch (hash) {
6761
RAW_SET_PROP_SWITCH_CASE_BASIC(sendMomentumEvents);
6862
RAW_SET_PROP_SWITCH_CASE_BASIC(nestedScrollEnabled);
69-
RAW_SET_PROP_SWITCH_CASE_BASIC(fadingEdgeLength);
7063
RAW_SET_PROP_SWITCH_CASE_BASIC(overScrollMode);
7164
RAW_SET_PROP_SWITCH_CASE_BASIC(endFillColor);
7265
}
@@ -276,6 +269,10 @@ folly::dynamic HostPlatformScrollViewProps::getDiffProps(
276269
result["horizontal"] = horizontal;
277270
}
278271

272+
if (fadingEdgeLength != oldProps->fadingEdgeLength) {
273+
result["fadingEdgeLength"] = fadingEdgeLength;
274+
}
275+
279276
if (scrollEventThrottle != oldProps->scrollEventThrottle) {
280277
result["scrollEventThrottle"] = scrollEventThrottle;
281278
}
@@ -368,10 +365,6 @@ folly::dynamic HostPlatformScrollViewProps::getDiffProps(
368365
result["nestedScrollEnabled"] = nestedScrollEnabled;
369366
}
370367

371-
if (fadingEdgeLength != oldProps->fadingEdgeLength) {
372-
result["fadingEdgeLength"] = fadingEdgeLength;
373-
}
374-
375368
if (overScrollMode != oldProps->overScrollMode) {
376369
result["overScrollMode"] = overScrollMode;
377370
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ class HostPlatformScrollViewProps : public BaseScrollViewProps {
3535

3636
bool sendMomentumEvents{};
3737
bool nestedScrollEnabled{};
38-
folly::dynamic fadingEdgeLength{};
3938
std::string overScrollMode{"auto"};
4039
SharedColor endFillColor{clearColor()};
4140

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

Lines changed: 0 additions & 41 deletions
This file was deleted.

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

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,8 @@
99

1010
#include <react/cxxstableapi/FrameworksGuard.h>
1111

12-
#include <folly/dynamic.h>
1312
#include <react/renderer/components/scrollview/BaseScrollViewProps.h>
1413

1514
namespace facebook::react {
16-
17-
class HostPlatformScrollViewProps : public BaseScrollViewProps {
18-
public:
19-
HostPlatformScrollViewProps() = default;
20-
HostPlatformScrollViewProps(
21-
const PropsParserContext &context,
22-
const HostPlatformScrollViewProps &sourceProps,
23-
const RawProps &rawProps);
24-
25-
void
26-
setProp(const PropsParserContext &context, RawPropsPropNameHash hash, const char *propName, const RawValue &value);
27-
28-
#pragma mark - Props
29-
30-
folly::dynamic fadingEdgeLength{};
31-
};
32-
15+
using HostPlatformScrollViewProps = BaseScrollViewProps;
3316
} // namespace facebook::react

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1429,6 +1429,7 @@ class facebook::react::BaseScrollViewProps : public facebook::react::HostPlatfor
14291429
public facebook::react::ScrollViewIndicatorStyle indicatorStyle;
14301430
public facebook::react::ScrollViewKeyboardDismissMode keyboardDismissMode;
14311431
public facebook::react::ScrollViewSnapToAlignment snapToAlignment;
1432+
public folly::dynamic fadingEdgeLength;
14321433
public std::optional<facebook::react::ScrollViewMaintainVisibleContentPosition> maintainVisibleContentPosition;
14331434
public std::vector<facebook::react::Float> snapToOffsets;
14341435
public void setProp(const facebook::react::PropsParserContext& context, facebook::react::RawPropsPropNameHash hash, const char* propName, const facebook::react::RawValue& value);
@@ -2041,7 +2042,6 @@ class facebook::react::HostPlatformScrollViewProps : public facebook::react::Bas
20412042
public bool nestedScrollEnabled;
20422043
public bool sendMomentumEvents;
20432044
public facebook::react::SharedColor endFillColor;
2044-
public folly::dynamic fadingEdgeLength;
20452045
public std::string overScrollMode;
20462046
public virtual facebook::react::ComponentName getDiffPropsImplementationTarget() const override;
20472047
public virtual folly::dynamic getDiffProps(const facebook::react::Props* prevProps) const override;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4026,6 +4026,7 @@ class facebook::react::BaseScrollViewProps : public facebook::react::HostPlatfor
40264026
public facebook::react::ScrollViewIndicatorStyle indicatorStyle;
40274027
public facebook::react::ScrollViewKeyboardDismissMode keyboardDismissMode;
40284028
public facebook::react::ScrollViewSnapToAlignment snapToAlignment;
4029+
public folly::dynamic fadingEdgeLength;
40294030
public std::optional<facebook::react::ScrollViewMaintainVisibleContentPosition> maintainVisibleContentPosition;
40304031
public std::vector<facebook::react::Float> snapToOffsets;
40314032
public void setProp(const facebook::react::PropsParserContext& context, facebook::react::RawPropsPropNameHash hash, const char* propName, const facebook::react::RawValue& value);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,7 @@ class facebook::react::BaseScrollViewProps : public facebook::react::HostPlatfor
772772
public facebook::react::ScrollViewIndicatorStyle indicatorStyle;
773773
public facebook::react::ScrollViewKeyboardDismissMode keyboardDismissMode;
774774
public facebook::react::ScrollViewSnapToAlignment snapToAlignment;
775+
public folly::dynamic fadingEdgeLength;
775776
public std::optional<facebook::react::ScrollViewMaintainVisibleContentPosition> maintainVisibleContentPosition;
776777
public std::vector<Float> snapToOffsets;
777778
public void setProp(const facebook::react::PropsParserContext& context, facebook::react::RawPropsPropNameHash hash, const char* propName, const facebook::react::RawValue& value);

0 commit comments

Comments
 (0)