From d03d89ac714aeda43c1a31218f41587c33fd4f6a Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Tue, 21 Jun 2016 14:53:08 -0700 Subject: [PATCH] [fix] CoreComponent -> createNativeComponent 'CoreComponent' creates new component instances and clutters the React component tree during debugging. This patch converts 'CoreComponent' to a simple function that creates a native web element. This patch also includes a fix for use of the 'flexShrink' style on 'View'. Fix #140 --- src/components/CoreComponent/index.js | 68 ------------------- src/components/Image/index.js | 14 ++-- src/components/Text/__tests__/index-test.js | 24 ------- src/components/Text/index.js | 34 +++++----- .../TextInput/__tests__/index-test.js | 12 ---- src/components/TextInput/index.js | 6 +- src/components/View/__tests__/index-test.js | 43 ++++-------- src/components/View/index.js | 58 ++++++++-------- .../__tests__/index-test.js | 27 ++++---- src/modules/createNativeComponent/index.js | 57 ++++++++++++++++ 10 files changed, 138 insertions(+), 205 deletions(-) delete mode 100644 src/components/CoreComponent/index.js rename src/{components/CoreComponent => modules/createNativeComponent}/__tests__/index-test.js (61%) create mode 100644 src/modules/createNativeComponent/index.js diff --git a/src/components/CoreComponent/index.js b/src/components/CoreComponent/index.js deleted file mode 100644 index 64d7048a..00000000 --- a/src/components/CoreComponent/index.js +++ /dev/null @@ -1,68 +0,0 @@ -import NativeMethodsDecorator from '../../modules/NativeMethodsDecorator' -import React, { Component, PropTypes } from 'react' -import StyleSheet from '../../apis/StyleSheet' - -const roleComponents = { - article: 'article', - banner: 'header', - button: 'button', - complementary: 'aside', - contentinfo: 'footer', - form: 'form', - heading: 'h1', - link: 'a', - list: 'ul', - listitem: 'li', - main: 'main', - navigation: 'nav', - region: 'section' -} - -@NativeMethodsDecorator -class CoreComponent extends Component { - static propTypes = { - accessibilityLabel: PropTypes.string, - accessibilityLiveRegion: PropTypes.oneOf([ 'assertive', 'off', 'polite' ]), - accessibilityRole: PropTypes.string, - accessible: PropTypes.bool, - component: PropTypes.oneOfType([ PropTypes.func, PropTypes.string ]), - style: PropTypes.oneOfType([ PropTypes.array, PropTypes.object ]), - testID: PropTypes.string, - type: PropTypes.string - }; - - static defaultProps = { - accessible: true, - component: 'div' - }; - - render() { - const { - accessibilityLabel, - accessibilityLiveRegion, - accessibilityRole, - accessible, - component, - testID, - type, - ...other - } = this.props - - const Component = roleComponents[accessibilityRole] || component - - return ( - - ) - } -} - -module.exports = CoreComponent diff --git a/src/components/Image/index.js b/src/components/Image/index.js index f804e3ee..66a11533 100644 --- a/src/components/Image/index.js +++ b/src/components/Image/index.js @@ -1,9 +1,9 @@ /* global window */ -import NativeMethodsDecorator from '../../modules/NativeMethodsDecorator' -import resolveAssetSource from './resolveAssetSource' -import CoreComponent from '../CoreComponent' +import createNativeComponent from '../../modules/createNativeComponent' import ImageResizeMode from './ImageResizeMode' import ImageStylePropTypes from './ImageStylePropTypes' +import NativeMethodsDecorator from '../../modules/NativeMethodsDecorator' +import resolveAssetSource from './resolveAssetSource' import React, { Component, PropTypes } from 'react' import StyleSheet from '../../apis/StyleSheet' import StyleSheetPropType from '../../apis/StyleSheet/StyleSheetPropType' @@ -25,8 +25,8 @@ const ImageSourcePropType = PropTypes.oneOfType([ @NativeMethodsDecorator class Image extends Component { static propTypes = { - accessibilityLabel: CoreComponent.propTypes.accessibilityLabel, - accessible: CoreComponent.propTypes.accessible, + accessibilityLabel: createNativeComponent.propTypes.accessibilityLabel, + accessible: createNativeComponent.propTypes.accessible, children: PropTypes.any, defaultSource: ImageSourcePropType, onError: PropTypes.func, @@ -36,7 +36,7 @@ class Image extends Component { resizeMode: PropTypes.oneOf(['contain', 'cover', 'none', 'stretch']), source: ImageSourcePropType, style: StyleSheetPropType(ImageStylePropTypes), - testID: CoreComponent.propTypes.testID + testID: createNativeComponent.propTypes.testID }; static defaultProps = { @@ -170,7 +170,7 @@ class Image extends Component { ]} testID={testID} > - + {createNativeComponent({ component: 'img', src: displayImage, style: styles.img })} {children ? ( ) : null} diff --git a/src/components/Text/__tests__/index-test.js b/src/components/Text/__tests__/index-test.js index b455af05..b8d29ae8 100644 --- a/src/components/Text/__tests__/index-test.js +++ b/src/components/Text/__tests__/index-test.js @@ -8,24 +8,6 @@ import ReactTestUtils from 'react-addons-test-utils' import Text from '../' suite('components/Text', () => { - test('prop "accessibilityLabel"', () => { - const accessibilityLabel = 'accessibilityLabel' - const result = utils.shallowRender() - assert.equal(result.props.accessibilityLabel, accessibilityLabel) - }) - - test('prop "accessibilityRole"', () => { - const accessibilityRole = 'accessibilityRole' - const result = utils.shallowRender() - assert.equal(result.props.accessibilityRole, accessibilityRole) - }) - - test('prop "accessible"', () => { - const accessible = false - const result = utils.shallowRender() - assert.equal(result.props.accessible, accessible) - }) - test('prop "children"', () => { const children = 'children' const result = utils.shallowRender({children}) @@ -42,10 +24,4 @@ suite('components/Text', () => { done() } }) - - test('prop "testID"', () => { - const testID = 'testID' - const result = utils.shallowRender() - assert.equal(result.props.testID, testID) - }) }) diff --git a/src/components/Text/index.js b/src/components/Text/index.js index 389b1390..54738ce0 100644 --- a/src/components/Text/index.js +++ b/src/components/Text/index.js @@ -1,6 +1,6 @@ +import createNativeComponent from '../../modules/createNativeComponent' import NativeMethodsDecorator from '../../modules/NativeMethodsDecorator' -import CoreComponent from '../CoreComponent' -import React, { Component, PropTypes } from 'react' +import { Component, PropTypes } from 'react' import StyleSheet from '../../apis/StyleSheet' import StyleSheetPropType from '../../apis/StyleSheet/StyleSheetPropType' import TextStylePropTypes from './TextStylePropTypes' @@ -8,14 +8,14 @@ import TextStylePropTypes from './TextStylePropTypes' @NativeMethodsDecorator class Text extends Component { static propTypes = { - accessibilityLabel: CoreComponent.propTypes.accessibilityLabel, - accessibilityRole: CoreComponent.propTypes.accessibilityRole, - accessible: CoreComponent.propTypes.accessible, + accessibilityLabel: createNativeComponent.propTypes.accessibilityLabel, + accessibilityRole: createNativeComponent.propTypes.accessibilityRole, + accessible: createNativeComponent.propTypes.accessible, children: PropTypes.any, numberOfLines: PropTypes.number, onPress: PropTypes.func, style: StyleSheetPropType(TextStylePropTypes), - testID: CoreComponent.propTypes.testID + testID: createNativeComponent.propTypes.testID }; static defaultProps = { @@ -36,18 +36,16 @@ class Text extends Component { ...other } = this.props - return ( - - ) + return createNativeComponent({ + ...other, + component: 'span', + onClick: this._onPress, + style: [ + styles.initial, + style, + numberOfLines === 1 && styles.singleLineStyle + ] + }) } } diff --git a/src/components/TextInput/__tests__/index-test.js b/src/components/TextInput/__tests__/index-test.js index 03184f5f..919bae7b 100644 --- a/src/components/TextInput/__tests__/index-test.js +++ b/src/components/TextInput/__tests__/index-test.js @@ -13,12 +13,6 @@ const findShallowInput = (vdom) => vdom.props.children.props.children[0] const findShallowPlaceholder = (vdom) => vdom.props.children.props.children[1] suite('components/TextInput', () => { - test('prop "accessibilityLabel"', () => { - const accessibilityLabel = 'accessibilityLabel' - const result = utils.shallowRender() - assert.equal(result.props.accessibilityLabel, accessibilityLabel) - }) - test('prop "autoComplete"', () => { // off let input = findInput(utils.renderToDOM()) @@ -221,12 +215,6 @@ suite('components/TextInput', () => { assert.equal(input.selectionStart, 0) }) - test('prop "testID"', () => { - const testID = 'testID' - const result = utils.shallowRender() - assert.equal(result.props.testID, testID) - }) - test('prop "value"', () => { const value = 'value' const input = findShallowInput(utils.shallowRender()) diff --git a/src/components/TextInput/index.js b/src/components/TextInput/index.js index 97fccc57..3d8d8cb5 100644 --- a/src/components/TextInput/index.js +++ b/src/components/TextInput/index.js @@ -1,5 +1,5 @@ +import createNativeComponent from '../../modules/createNativeComponent' import NativeMethodsDecorator from '../../modules/NativeMethodsDecorator' -import CoreComponent from '../CoreComponent' import React, { Component, PropTypes } from 'react' import ReactDOM from 'react-dom' import StyleSheet from '../../apis/StyleSheet' @@ -32,7 +32,7 @@ class TextInput extends Component { secureTextEntry: PropTypes.bool, selectTextOnFocus: PropTypes.bool, style: Text.propTypes.style, - testID: CoreComponent.propTypes.testID, + testID: Text.propTypes.testID, value: PropTypes.string }; @@ -197,7 +197,7 @@ class TextInput extends Component { testID={testID} > - + {createNativeComponent({ ...props, ref: 'input' })} {placeholder && this.state.showPlaceholder && { - test('prop "accessibilityLabel"', () => { - const accessibilityLabel = 'accessibilityLabel' - const result = utils.shallowRender() - assert.equal(result.props.accessibilityLabel, accessibilityLabel) - }) - - test('prop "accessibilityLiveRegion"', () => { - const accessibilityLiveRegion = 'polite' - const result = utils.shallowRender() - assert.equal(result.props.accessibilityLiveRegion, accessibilityLiveRegion) - }) - - test('prop "accessibilityRole"', () => { - const accessibilityRole = 'accessibilityRole' - const result = utils.shallowRender() - assert.equal(result.props.accessibilityRole, accessibilityRole) - }) - - test('prop "accessible"', () => { - const accessible = false - const result = utils.shallowRender() - assert.equal(result.props.accessible, accessible) - }) - test('prop "children"', () => { const children = 'children' const result = utils.shallowRender({children}) @@ -40,12 +15,20 @@ suite('components/View', () => { test('prop "pointerEvents"', () => { const result = utils.shallowRender() - assert.equal(StyleSheet.flatten(result.props.style).pointerEvents, 'box-only') + assert.equal(result.props.className, '__style_pebo') }) - test('prop "testID"', () => { - const testID = 'testID' - const result = utils.shallowRender() - assert.equal(result.props.testID, testID) + test('prop "style"', () => { + const noFlex = utils.shallowRender() + assert.equal(noFlex.props.style.flexShrink, 0) + + const flex = utils.shallowRender() + assert.equal(flex.props.style.flexShrink, 1) + + const flexShrink = utils.shallowRender() + assert.equal(flexShrink.props.style.flexShrink, 1) + + const flexAndShrink = utils.shallowRender() + assert.equal(flexAndShrink.props.style.flexShrink, 2) }) }) diff --git a/src/components/View/index.js b/src/components/View/index.js index 4f95cba5..a17c8e56 100644 --- a/src/components/View/index.js +++ b/src/components/View/index.js @@ -1,7 +1,7 @@ +import createNativeComponent from '../../modules/createNativeComponent' import NativeMethodsDecorator from '../../modules/NativeMethodsDecorator' import normalizeNativeEvent from '../../apis/PanResponder/normalizeNativeEvent' -import CoreComponent from '../CoreComponent' -import React, { Component, PropTypes } from 'react' +import { Component, PropTypes } from 'react' import StyleSheet from '../../apis/StyleSheet' import StyleSheetPropType from '../../apis/StyleSheet/StyleSheetPropType' import ViewStylePropTypes from './ViewStylePropTypes' @@ -9,10 +9,10 @@ import ViewStylePropTypes from './ViewStylePropTypes' @NativeMethodsDecorator class View extends Component { static propTypes = { - accessibilityLabel: CoreComponent.propTypes.accessibilityLabel, - accessibilityLiveRegion: CoreComponent.propTypes.accessibilityLiveRegion, - accessibilityRole: CoreComponent.propTypes.accessibilityRole, - accessible: CoreComponent.propTypes.accessible, + accessibilityLabel: createNativeComponent.propTypes.accessibilityLabel, + accessibilityLiveRegion: createNativeComponent.propTypes.accessibilityLiveRegion, + accessibilityRole: createNativeComponent.propTypes.accessibilityRole, + accessible: createNativeComponent.propTypes.accessible, children: PropTypes.any, onClick: PropTypes.func, onClickCapture: PropTypes.func, @@ -36,7 +36,7 @@ class View extends Component { onTouchStartCapture: PropTypes.func, pointerEvents: PropTypes.oneOf(['auto', 'box-none', 'box-only', 'none']), style: StyleSheetPropType(ViewStylePropTypes), - testID: CoreComponent.propTypes.testID + testID: createNativeComponent.propTypes.testID }; static defaultProps = { @@ -59,28 +59,28 @@ class View extends Component { const flattenedStyle = StyleSheet.flatten(style) const pointerEventsStyle = pointerEvents && { pointerEvents } - return ( - - ) + const props = { + ...other, + onClick: this._handleClick, + onClickCapture: this._normalizeEventForHandler(this.props.onClickCapture), + onTouchCancel: this._normalizeEventForHandler(this.props.onTouchCancel), + onTouchCancelCapture: this._normalizeEventForHandler(this.props.onTouchCancelCapture), + onTouchEnd: this._normalizeEventForHandler(this.props.onTouchEnd), + onTouchEndCapture: this._normalizeEventForHandler(this.props.onTouchEndCapture), + onTouchMove: this._normalizeEventForHandler(this.props.onTouchMove), + onTouchMoveCapture: this._normalizeEventForHandler(this.props.onTouchMoveCapture), + onTouchStart: this._normalizeEventForHandler(this.props.onTouchStart), + onTouchStartCapture: this._normalizeEventForHandler(this.props.onTouchStartCapture), + style: [ + styles.initial, + style, + // 'View' needs to use 'flexShrink' in its reset when there is no 'flex' style provided + (flattenedStyle.flex == null && flattenedStyle.flexShrink == null) && styles.flexReset, + pointerEventsStyle + ] + } + + return createNativeComponent(props) } /** diff --git a/src/components/CoreComponent/__tests__/index-test.js b/src/modules/createNativeComponent/__tests__/index-test.js similarity index 61% rename from src/components/CoreComponent/__tests__/index-test.js rename to src/modules/createNativeComponent/__tests__/index-test.js index 3f1995c9..c2d1d310 100644 --- a/src/components/CoreComponent/__tests__/index-test.js +++ b/src/modules/createNativeComponent/__tests__/index-test.js @@ -1,62 +1,61 @@ /* eslint-env mocha */ -import * as utils from '../../../modules/specHelpers' +import * as utils from '../../specHelpers' import assert from 'assert' -import React from 'react' -import CoreComponent from '../' +import createNativeComponent from '../' -suite('components/CoreComponent', () => { +suite('modules/createNativeComponent', () => { test('prop "accessibilityLabel"', () => { const accessibilityLabel = 'accessibilityLabel' - const dom = utils.renderToDOM() + const dom = utils.renderToDOM(createNativeComponent({ accessibilityLabel })) assert.equal(dom.getAttribute('aria-label'), accessibilityLabel) }) test('prop "accessibilityLiveRegion"', () => { const accessibilityLiveRegion = 'polite' - const dom = utils.renderToDOM() + const dom = utils.renderToDOM(createNativeComponent({ accessibilityLiveRegion })) assert.equal(dom.getAttribute('aria-live'), accessibilityLiveRegion) }) test('prop "accessibilityRole"', () => { const accessibilityRole = 'banner' - let dom = utils.renderToDOM() + let dom = utils.renderToDOM(createNativeComponent({ accessibilityRole })) assert.equal(dom.getAttribute('role'), accessibilityRole) assert.equal((dom.tagName).toLowerCase(), 'header') const button = 'button' - dom = utils.renderToDOM() + dom = utils.renderToDOM(createNativeComponent({ accessibilityRole: 'button' })) assert.equal(dom.getAttribute('type'), button) assert.equal((dom.tagName).toLowerCase(), button) }) test('prop "accessible"', () => { // accessible (implicit) - let dom = utils.renderToDOM() + let dom = utils.renderToDOM(createNativeComponent({})) assert.equal(dom.getAttribute('aria-hidden'), null) // accessible (explicit) - dom = utils.renderToDOM() + dom = utils.renderToDOM(createNativeComponent({ accessible: true })) assert.equal(dom.getAttribute('aria-hidden'), null) // not accessible - dom = utils.renderToDOM() + dom = utils.renderToDOM(createNativeComponent({ accessible: false })) assert.equal(dom.getAttribute('aria-hidden'), 'true') }) test('prop "component"', () => { const component = 'main' - const dom = utils.renderToDOM() + const dom = utils.renderToDOM(createNativeComponent({ component })) const tagName = (dom.tagName).toLowerCase() assert.equal(tagName, component) }) test('prop "testID"', () => { // no testID - let dom = utils.renderToDOM() + let dom = utils.renderToDOM(createNativeComponent({})) assert.equal(dom.getAttribute('data-testid'), null) // with testID const testID = 'Example.testID' - dom = utils.renderToDOM() + dom = utils.renderToDOM(createNativeComponent({ testID })) assert.equal(dom.getAttribute('data-testid'), testID) }) }) diff --git a/src/modules/createNativeComponent/index.js b/src/modules/createNativeComponent/index.js new file mode 100644 index 00000000..a37756f1 --- /dev/null +++ b/src/modules/createNativeComponent/index.js @@ -0,0 +1,57 @@ +import React, { PropTypes } from 'react' +import StyleSheet from '../../apis/StyleSheet' + +const roleComponents = { + article: 'article', + banner: 'header', + button: 'button', + complementary: 'aside', + contentinfo: 'footer', + form: 'form', + heading: 'h1', + link: 'a', + list: 'ul', + listitem: 'li', + main: 'main', + navigation: 'nav', + region: 'section' +} + +const createNativeComponent = ({ + accessibilityLabel, + accessibilityLiveRegion, + accessibilityRole, + accessible = true, + component = 'div', + testID, + type, + ...other +}) => { + const Component = accessibilityRole && roleComponents[accessibilityRole] ? roleComponents[accessibilityRole] : component + + return ( + + ) +} + +createNativeComponent.propTypes = { + accessibilityLabel: PropTypes.string, + accessibilityLiveRegion: PropTypes.oneOf([ 'assertive', 'off', 'polite' ]), + accessibilityRole: PropTypes.string, + accessible: PropTypes.bool, + component: PropTypes.oneOfType([ PropTypes.func, PropTypes.string ]), + style: PropTypes.oneOfType([ PropTypes.array, PropTypes.object ]), + testID: PropTypes.string, + type: PropTypes.string +} + +module.exports = createNativeComponent