From d9a8ac5071d23275c5d4a8e9936f391b967416d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9F=D1=91=D1=82=D1=80=20=D0=9F=D0=BE=D1=82=D0=B0=D0=BF?= =?UTF-8?q?=D0=BE=D0=B2?= Date: Fri, 7 Jun 2019 02:44:21 -0700 Subject: [PATCH] Fix: RefreshControl in FlatList makes borderWidth not working (#24411) Summary: Fixes #22752 On line 1021 you are passing base style to props: `style: [baseStyle, this.props.style],` Explicitly passing base style to ScrollView just overrides this line and doesn't let developers to customise style of any inheritors of ScrollView (not only FlatList) with custom RefreshControl. So this line (1113) seems to be removed. ## Changelog [GENERAL] [Fixed] - fix of Android's bug that doesn't let override ScrollView's Style with custom RefreshControl. Pull Request resolved: https://github.com/facebook/react-native/pull/24411 Differential Revision: D15713061 Pulled By: cpojer fbshipit-source-id: 461259800f867af15e53e0743a5057ea4528ae69 --- Libraries/Components/ScrollView/ScrollView.js | 9 +-- .../__tests__/splitLayoutProps-test.js | 44 +++++++++++++ Libraries/StyleSheet/splitLayoutProps.js | 62 +++++++++++++++++++ 3 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 Libraries/StyleSheet/__tests__/splitLayoutProps-test.js create mode 100644 Libraries/StyleSheet/splitLayoutProps.js diff --git a/Libraries/Components/ScrollView/ScrollView.js b/Libraries/Components/ScrollView/ScrollView.js index f28d779088..260bc0fd94 100644 --- a/Libraries/Components/ScrollView/ScrollView.js +++ b/Libraries/Components/ScrollView/ScrollView.js @@ -25,6 +25,7 @@ const invariant = require('invariant'); const processDecelerationRate = require('./processDecelerationRate'); const requireNativeComponent = require('../../ReactNative/requireNativeComponent'); const resolveAssetSource = require('../../Image/resolveAssetSource'); +const splitLayoutProps = require('../../StyleSheet/splitLayoutProps'); import type { PressEvent, @@ -1125,15 +1126,15 @@ class ScrollView extends React.Component { // On Android wrap the ScrollView with a AndroidSwipeRefreshLayout. // Since the ScrollView is wrapped add the style props to the // AndroidSwipeRefreshLayout and use flex: 1 for the ScrollView. - // Note: we should only apply props.style on the wrapper + // Note: we should split props.style on the inner and outer props // however, the ScrollView still needs the baseStyle to be scrollable - + const {outer, inner} = splitLayoutProps(flattenStyle(props.style)); return React.cloneElement( refreshControl, - {style: props.style}, + {style: [baseStyle, outer]}, {contentContainer} diff --git a/Libraries/StyleSheet/__tests__/splitLayoutProps-test.js b/Libraries/StyleSheet/__tests__/splitLayoutProps-test.js new file mode 100644 index 0000000000..70b2877ae2 --- /dev/null +++ b/Libraries/StyleSheet/__tests__/splitLayoutProps-test.js @@ -0,0 +1,44 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @emails oncall+react_native + */ + +'use strict'; + +const splitLayoutProps = require('../splitLayoutProps'); + +test('splits style objects', () => { + const style = {width: 10, margin: 20, padding: 30}; + const {outer, inner} = splitLayoutProps(style); + expect(outer).toMatchInlineSnapshot(` + Object { + "margin": 20, + "width": 10, + } + `); + expect(inner).toMatchInlineSnapshot(` + Object { + "padding": 30, + } + `); +}); + +test('does not copy values to both returned objects', () => { + const style = {marginVertical: 5, paddingHorizontal: 10}; + const {outer, inner} = splitLayoutProps(style); + expect(outer).toMatchInlineSnapshot(` + Object { + "marginVertical": 5, + } + `); + expect(inner).toMatchInlineSnapshot(` + Object { + "paddingHorizontal": 10, + } + `); +}); diff --git a/Libraries/StyleSheet/splitLayoutProps.js b/Libraries/StyleSheet/splitLayoutProps.js new file mode 100644 index 0000000000..700fbea437 --- /dev/null +++ b/Libraries/StyleSheet/splitLayoutProps.js @@ -0,0 +1,62 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @flow strict-local + */ + +'use strict'; + +import type {DangerouslyImpreciseStyle} from './StyleSheet'; + +const OUTER_PROPS = Object.assign(Object.create(null), { + margin: true, + marginHorizontal: true, + marginVertical: true, + marginBottom: true, + marginTop: true, + marginLeft: true, + marginRight: true, + flex: true, + flexGrow: true, + flexShrink: true, + flexBasis: true, + alignSelf: true, + height: true, + minHeight: true, + maxHeight: true, + width: true, + minWidth: true, + maxWidth: true, + position: true, + left: true, + right: true, + bottom: true, + top: true, +}); + +function splitLayoutProps( + props: ?DangerouslyImpreciseStyle, +): { + outer: DangerouslyImpreciseStyle, + inner: DangerouslyImpreciseStyle, +} { + const inner = {}; + const outer = {}; + if (props) { + Object.keys(props).forEach(k => { + const value: $ElementType = props[k]; + if (OUTER_PROPS[k]) { + outer[k] = value; + } else { + inner[k] = value; + } + }); + } + return {outer, inner}; +} + +module.exports = splitLayoutProps;