From 39e565341ef08f75915b78967c790ad1765700c5 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Wed, 15 Apr 2020 07:30:11 -0700 Subject: [PATCH 1/2] Fix PickerWindows (and DatePickerExample Page) Fixes #4597 Our implementation of Picker is a bit deformed. In Stock React Native, Picker.js will delegate to PickerAndroid and PickerIOS. In Windows, we the two are copy pasted. In 0.62, invariants are added to prevent duplicate view manager registration. We run into issues with this with Picker, since our copies will both call requireNativeComponent. This change modifies the normal picker to delegate to PickerWindows to work correctly from the stock picker. I.e. The stock picker's child views are meant for data only and will throw if rendered, where the Windows version's are included but used to noop render. Picker is getting lean-cored, so I didn't invest much into fixing PickerWindow's isssues, converting to Flow, NativeComponent strucutre, etc. Validated Picker, PickerWindows, DatePicker RNTester pages work correctly. --- .../Components/Picker/Picker.windows.js | 160 ++++++++++++++++++ .../Components/Picker/Picker.windows.tsx | 127 -------------- .../Components/Picker/PickerWindows.tsx | 17 +- vnext/src/overrides.json | 14 +- 4 files changed, 177 insertions(+), 141 deletions(-) create mode 100644 vnext/src/Libraries/Components/Picker/Picker.windows.js delete mode 100644 vnext/src/Libraries/Components/Picker/Picker.windows.tsx diff --git a/vnext/src/Libraries/Components/Picker/Picker.windows.js b/vnext/src/Libraries/Components/Picker/Picker.windows.js new file mode 100644 index 00000000000..a91c988168d --- /dev/null +++ b/vnext/src/Libraries/Components/Picker/Picker.windows.js @@ -0,0 +1,160 @@ +/** + * 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 + */ + +'use strict'; + +const PickerAndroid = require('./PickerAndroid'); +const PickerIOS = require('./PickerIOS'); +const PickerWindows = require('./PickerWindows').Picker; // [Windows] +const Platform = require('../../Utilities/Platform'); +const React = require('react'); +const UnimplementedView = require('../UnimplementedViews/UnimplementedView'); + +import type {TextStyleProp} from '../../StyleSheet/StyleSheet'; +import type {ColorValue} from '../../StyleSheet/StyleSheetTypes'; + +const MODE_DIALOG = 'dialog'; +const MODE_DROPDOWN = 'dropdown'; + +type PickerItemProps = $ReadOnly<{| + /** + * Text to display for this item. + */ + label: string, + + /** + * The value to be passed to picker's `onValueChange` callback when + * this item is selected. Can be a string or an integer. + */ + value?: ?(number | string), + + /** + * Color of this item's text. + * @platform android + */ + color?: ColorValue, + + /** + * Used to locate the item in end-to-end tests. + */ + testID?: string, +|}>; + +/** + * Individual selectable item in a Picker. + */ +export type {PickerItem}; +class PickerItem extends React.Component { + render() { + // The items are not rendered directly + throw null; + } +} + +type PickerProps = $ReadOnly<{| + children?: React.Node, + style?: ?TextStyleProp, + + /** + * Value matching value of one of the items. Can be a string or an integer. + */ + selectedValue?: ?(number | string), + + /** + * Callback for when an item is selected. This is called with the following parameters: + * - `itemValue`: the `value` prop of the item that was selected + * - `itemIndex`: the index of the selected item in this picker + */ + onValueChange?: ?(itemValue: string | number, itemIndex: number) => mixed, + + /** + * If set to false, the picker will be disabled, i.e. the user will not be able to make a + * selection. + * @platform android + */ + enabled?: ?boolean, + + /** + * On Android, specifies how to display the selection items when the user taps on the picker: + * + * - 'dialog': Show a modal dialog. This is the default. + * - 'dropdown': Shows a dropdown anchored to the picker view + * + * @platform android + */ + mode?: ?('dialog' | 'dropdown'), + + /** + * Style to apply to each of the item labels. + * @platform ios + */ + itemStyle?: ?TextStyleProp, + + /** + * Prompt string for this picker, used on Android in dialog mode as the title of the dialog. + * @platform android + */ + prompt?: ?string, + + /** + * Used to locate this view in end-to-end tests. + */ + testID?: ?string, +|}>; + +/** + * Renders the native picker component on iOS and Android. Example: + * + * this.setState({language: itemValue})}> + * + * + * + */ +class Picker extends React.Component { + /** + * On Android, display the options in a dialog. + */ + static MODE_DIALOG: $TEMPORARY$string<'dialog'> = MODE_DIALOG; + + /** + * On Android, display the options in a dropdown (this is the default). + */ + static MODE_DROPDOWN: $TEMPORARY$string<'dropdown'> = MODE_DROPDOWN; + + static Item: typeof PickerItem = PickerItem; + + static defaultProps: {|mode: $TEMPORARY$string<'dialog'>|} = { + mode: MODE_DIALOG, + }; + + render(): React.Node { + if (Platform.OS === 'ios') { + /* $FlowFixMe(>=0.81.0 site=react_native_ios_fb) This suppression was + * added when renaming suppression sites. */ + return {this.props.children}; + } else if (Platform.OS === 'android') { + return ( + /* $FlowFixMe(>=0.81.0 site=react_native_android_fb) This suppression + * was added when renaming suppression sites. */ + {this.props.children} + ); + } else if (Platform.OS === 'windows') { + return ( + {this.props.children} // [Windows] + ); + } else { + return ; + } + } +} + +module.exports = Picker; diff --git a/vnext/src/Libraries/Components/Picker/Picker.windows.tsx b/vnext/src/Libraries/Components/Picker/Picker.windows.tsx deleted file mode 100644 index 25cc37047ba..00000000000 --- a/vnext/src/Libraries/Components/Picker/Picker.windows.tsx +++ /dev/null @@ -1,127 +0,0 @@ -/** - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. - * @format - */ -'use strict'; - -import * as React from 'react'; -import {requireNativeComponent, processColor} from 'react-native'; -import { - IPickerItemProps, - IPickerProps, - IPickerChangeEvent, -} from './PickerProps'; - -const RCTPicker = requireNativeComponent< - IPickerProps & {items: IPickerItemData[]; selectedIndex: number} ->('RCTPicker'); - -class PickerItem extends React.Component { - public render(): JSX.Element | null { - return null; - } -} - -interface IPickerItemData { - label: string; - // tslint:disable-next-line:no-any - value?: any; - textColor?: number; -} - -// tslint:disable-next-line:interface-name -interface State { - selectedIndex: number; - items: IPickerItemData[]; -} - -type PickerPropsWithChildren = Readonly<{children?: React.ReactNode}> & - Readonly; - -/** - * Picker is a controlled component, which expects the selectedValue prop to be updated - * whenever selection changes, or selection will revert to the prop selectedValue - * - * when using editable=true, onValueChange can be called with a selectedValue of null & - * Index of -1, and text will be provided. - * To maintain the text in the controlled component, props should reflect - * that state by specifying selectedValue of null and specify the text property. - */ -class Picker extends React.Component { - public static Item = PickerItem; - - // tslint:disable-next-line:no-any - private _rctPicker: any; - - public static getDerivedStateFromProps( - props: PickerPropsWithChildren, - ): State { - let selectedIndex = -1; - const items: IPickerItemData[] = []; - React.Children.toArray(props.children).forEach( - (c: React.ReactNode, index: number) => { - const child = (c as unknown) as PickerItem; - if (child.props.value === props.selectedValue) { - selectedIndex = index; - } - items.push({ - value: child.props.value, - label: child.props.label, - textColor: processColor(child.props.color), - }); - }, - ); - return {selectedIndex, items}; - } - - public constructor(props: IPickerProps) { - super(props); - this._rctPicker = React.createRef(); - this.state = { - selectedIndex: 0, - items: [], - }; - } - - public render(): JSX.Element { - const props = {...this.props}; - props.onStartShouldSetResponder = () => true; - props.onResponderTerminationRequest = () => false; - props.style = this.props.style; - - return ( - - {this.props.children} - - ); - } - - private _setRef = (comboBox: any /*RCTPicker*/) => { - this._rctPicker = comboBox; - }; - - private _onChange = (event: IPickerChangeEvent) => { - if (this._rctPicker) { - this._rctPicker.setNativeProps({ - selectedIndex: this.state.selectedIndex, - text: this.props.text, - }); - } - - this.props.onChange && this.props.onChange(event); - this.props.onValueChange && - this.props.onValueChange( - event.nativeEvent.value, - event.nativeEvent.itemIndex, - event.nativeEvent.text, - ); - }; -} - -module.exports = Picker; diff --git a/vnext/src/Libraries/Components/Picker/PickerWindows.tsx b/vnext/src/Libraries/Components/Picker/PickerWindows.tsx index 76431ba2009..22b512df199 100644 --- a/vnext/src/Libraries/Components/Picker/PickerWindows.tsx +++ b/vnext/src/Libraries/Components/Picker/PickerWindows.tsx @@ -19,7 +19,7 @@ const RCTPicker = requireNativeComponent< class PickerItem extends React.Component { public render(): JSX.Element | null { - return null; + throw null; } } @@ -85,20 +85,19 @@ export class Picker extends React.Component { } public render(): JSX.Element { - const props = {...this.props}; - props.onStartShouldSetResponder = () => true; - props.onResponderTerminationRequest = () => false; - props.style = this.props.style; + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const {children, ...propsWihoutChildren} = {...this.props}; + propsWihoutChildren.onStartShouldSetResponder = () => true; + propsWihoutChildren.onResponderTerminationRequest = () => false; return ( - {this.props.children} - + ref={this._setRef} + /> ); } diff --git a/vnext/src/overrides.json b/vnext/src/overrides.json index 5a9e051c70d..c08977e63fd 100644 --- a/vnext/src/overrides.json +++ b/vnext/src/overrides.json @@ -204,12 +204,12 @@ "baseHash": "0b409391c852a1001cee74a84414a13c4fe88f8b" }, { - "type": "derived", - "file": "Libraries\\Components\\Picker\\Picker.windows.tsx", + "type": "patch", + "file": "Libraries\\Components\\Picker\\Picker.windows.js", "baseFile": "Libraries\\Components\\Picker\\Picker.js", "baseVersion": "0.62.0-rc.3", "baseHash": "67a10fef33208d21006d7bd51b4e6fd8d4c9ca5d", - "issue": "LEGACY_FIXME" + "issue": 4611 }, { "type": "derived", @@ -244,8 +244,12 @@ "file": "Libraries\\Components\\Picker\\PickerProps.ts" }, { - "type": "platform", - "file": "Libraries\\Components\\Picker\\PickerWindows.tsx" + "type": "derived", + "file": "Libraries\\Components\\Picker\\PickerWindows.tsx", + "baseFile": "Libraries\\Components\\Picker\\PickerIOS.ios.js", + "baseVersion": "0.62.2", + "baseHash": "f1409e9f69332850927323ecf2cf66a3573ef25e", + "issue": 4611 }, { "type": "patch", From ded192768087664e5955fc6a62a85b8b06a4b201 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Wed, 15 Apr 2020 07:30:29 -0700 Subject: [PATCH 2/2] Change files --- ...act-native-windows-2020-04-15-07-30-29-fix-picker.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 change/react-native-windows-2020-04-15-07-30-29-fix-picker.json diff --git a/change/react-native-windows-2020-04-15-07-30-29-fix-picker.json b/change/react-native-windows-2020-04-15-07-30-29-fix-picker.json new file mode 100644 index 00000000000..39dd48b64f6 --- /dev/null +++ b/change/react-native-windows-2020-04-15-07-30-29-fix-picker.json @@ -0,0 +1,8 @@ +{ + "type": "prerelease", + "comment": "Fix PickerWindows (and DatePickerExample Page)", + "packageName": "react-native-windows", + "email": "ngerlem@microsoft.com", + "dependentChangeType": "patch", + "date": "2020-04-15T14:30:29.574Z" +} \ No newline at end of file