From 8d37fde5ee105f4a78a927a3913e4fea2eee3014 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Tue, 4 Feb 2020 16:05:41 -0800 Subject: [PATCH] [change] modernize Picker Rewrite Picker to use function components and hooks. Rewrite the tests to replace enzyme with testing-library. --- .../src/exports/Picker/PickerItem.js | 11 +-- .../__snapshots__/index-test.js.snap | 15 ++-- .../exports/Picker/__tests__/index-test.js | 39 ++++++---- .../src/exports/Picker/index.js | 78 ++++++++++--------- 4 files changed, 75 insertions(+), 68 deletions(-) diff --git a/packages/react-native-web/src/exports/Picker/PickerItem.js b/packages/react-native-web/src/exports/Picker/PickerItem.js index ed0541d8..649b2cdf 100644 --- a/packages/react-native-web/src/exports/Picker/PickerItem.js +++ b/packages/react-native-web/src/exports/Picker/PickerItem.js @@ -10,7 +10,6 @@ import type { ColorValue } from '../../types'; -import React from 'react'; import createElement from '../createElement'; type Props = { @@ -20,10 +19,8 @@ type Props = { value?: number | string }; -export default class PickerItem extends React.Component { - render() { - const { color, label, testID, value } = this.props; - const style = { color }; - return createElement('option', { style, testID, value }, label); - } +export default function PickerItem(props: Props) { + const { color, label, testID, value } = props; + const style = { color }; + return createElement('option', { style, testID, value }, label); } diff --git a/packages/react-native-web/src/exports/Picker/__tests__/__snapshots__/index-test.js.snap b/packages/react-native-web/src/exports/Picker/__tests__/__snapshots__/index-test.js.snap index 5c506648..03454e94 100644 --- a/packages/react-native-web/src/exports/Picker/__tests__/__snapshots__/index-test.js.snap +++ b/packages/react-native-web/src/exports/Picker/__tests__/__snapshots__/index-test.js.snap @@ -9,14 +9,9 @@ exports[`components/Picker prop "children" items 1`] = ` `; exports[`components/Picker prop "children" renders items 1`] = ` -Array [ - , - , -] + `; diff --git a/packages/react-native-web/src/exports/Picker/__tests__/index-test.js b/packages/react-native-web/src/exports/Picker/__tests__/index-test.js index 4d6cae14..44dd6a1b 100644 --- a/packages/react-native-web/src/exports/Picker/__tests__/index-test.js +++ b/packages/react-native-web/src/exports/Picker/__tests__/index-test.js @@ -1,9 +1,13 @@ /* eslint-env jasmine, jest */ import React from 'react'; -import { shallow } from 'enzyme'; +import { render } from '@testing-library/react'; import Picker from '..'; +function findSelect(container) { + return container.querySelector('select'); +} + describe('components/Picker', () => { describe('prop "children"', () => { test('renders items', () => { @@ -13,14 +17,14 @@ describe('components/Picker', () => { ); - const component = shallow(picker); - expect(component.children()).toMatchSnapshot(); + const { container } = render(picker); + expect(container.firstChild.firstChild).toMatchSnapshot(); }); test('items', () => { const pickerItem = ; - const component = shallow(pickerItem); - expect(component).toMatchSnapshot(); + const { container } = render(pickerItem); + expect(container.firstChild).toMatchSnapshot(); }); }); @@ -32,8 +36,8 @@ describe('components/Picker', () => { ); - const component = shallow(picker); - expect(component.find('select').prop('disabled')).toBe(true); + const { container } = render(picker); + expect(findSelect(container).disabled).toBe(true); }); }); @@ -46,11 +50,14 @@ describe('components/Picker', () => { ); - const component = shallow(picker); - component.find('select').simulate('change', { - target: { selectedIndex: '1', value: 'value-2' } - }); - expect(onValueChange).toHaveBeenCalledWith('value-2', '1'); + const { container } = render(picker); + const select = findSelect(container); + // mock change event + select.selectedIndex = '1'; + select.value = 'value-2'; + select.dispatchEvent(new window.Event('change', { bubbles: true })); + + expect(onValueChange).toHaveBeenCalledWith('value-2', 1); }); }); @@ -62,8 +69,8 @@ describe('components/Picker', () => { ); - const component = shallow(picker); - expect(component.find('select').prop('value')).toBe('value-2'); + const { container } = render(picker); + expect(findSelect(container).value).toBe('value-2'); }); test('selects the correct item (number)', () => { @@ -73,8 +80,8 @@ describe('components/Picker', () => { ); - const component = shallow(picker); - expect(component.find('select').prop('value')).toBe(22); + const { container } = render(picker); + expect(findSelect(container).value).toBe('22'); }); }); }); diff --git a/packages/react-native-web/src/exports/Picker/index.js b/packages/react-native-web/src/exports/Picker/index.js index fce93192..07148740 100644 --- a/packages/react-native-web/src/exports/Picker/index.js +++ b/packages/react-native-web/src/exports/Picker/index.js @@ -10,11 +10,12 @@ import type { ViewProps } from '../View'; -import applyNativeMethods from '../../modules/applyNativeMethods'; -import { Component } from 'react'; import createElement from '../createElement'; +import setAndForwardRef from '../../modules/setAndForwardRef'; +import usePlatformMethods from '../../hooks/usePlatformMethods'; import PickerItem from './PickerItem'; import StyleSheet, { type StyleObj } from '../StyleSheet'; +import { forwardRef, useRef } from 'react'; type PickerProps = { ...ViewProps, @@ -29,44 +30,51 @@ type PickerProps = { prompt?: string }; -class Picker extends Component { - static Item = PickerItem; +const Picker = forwardRef((props, ref) => { + const { + children, + enabled, + onValueChange, + selectedValue, + style, + testID, + /* eslint-disable */ + itemStyle, + mode, + prompt, + /* eslint-enable */ + ...other + } = props; - render() { - const { - children, - enabled, - selectedValue, - style, - testID, - /* eslint-disable */ - itemStyle, - mode, - prompt, - onValueChange, - /* eslint-enable */ - ...otherProps - } = this.props; + const hostRef = useRef(null); + const setRef = setAndForwardRef({ + getForwardedRef: () => ref, + setLocalRef: c => { + hostRef.current = c; + } + }); + usePlatformMethods(hostRef, ref, null, style); - return createElement('select', { - children, - disabled: enabled === false ? true : undefined, - onChange: this._handleChange, - style: [styles.initial, style], - testID, - value: selectedValue, - ...otherProps - }); - } - - _handleChange = (e: Object) => { - const { onValueChange } = this.props; + function handleChange(e: Object) { const { selectedIndex, value } = e.target; if (onValueChange) { onValueChange(value, selectedIndex); } - }; -} + } + + return createElement('select', { + children, + disabled: enabled === false ? true : undefined, + onChange: handleChange, + ref: setRef, + style: [styles.initial, style], + testID, + value: selectedValue, + ...other + }); +}); + +Picker.Item = PickerItem; const styles = StyleSheet.create({ initial: { @@ -76,4 +84,4 @@ const styles = StyleSheet.create({ } }); -export default applyNativeMethods(Picker); +export default Picker;