Skip to content

Commit

Permalink
Fix PickerWindows (and DatePickerExample Page)
Browse files Browse the repository at this point in the history
Fixes microsoft#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.
  • Loading branch information
NickGerleman committed Apr 15, 2020
1 parent 48eaa21 commit 39e5653
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 141 deletions.
160 changes: 160 additions & 0 deletions vnext/src/Libraries/Components/Picker/Picker.windows.js
Original file line number Diff line number Diff line change
@@ -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<PickerItemProps> {
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:
*
* <Picker
* selectedValue={this.state.language}
* onValueChange={(itemValue, itemIndex) => this.setState({language: itemValue})}>
* <Picker.Item label="Java" value="java" />
* <Picker.Item label="JavaScript" value="js" />
* </Picker>
*/
class Picker extends React.Component<PickerProps> {
/**
* 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 <PickerIOS {...this.props}>{this.props.children}</PickerIOS>;
} else if (Platform.OS === 'android') {
return (
/* $FlowFixMe(>=0.81.0 site=react_native_android_fb) This suppression
* was added when renaming suppression sites. */
<PickerAndroid {...this.props}>{this.props.children}</PickerAndroid>
);
} else if (Platform.OS === 'windows') {
return (
<PickerWindows {...this.props}>{this.props.children}</PickerWindows> // [Windows]
);
} else {
return <UnimplementedView />;
}
}
}

module.exports = Picker;
127 changes: 0 additions & 127 deletions vnext/src/Libraries/Components/Picker/Picker.windows.tsx

This file was deleted.

17 changes: 8 additions & 9 deletions vnext/src/Libraries/Components/Picker/PickerWindows.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const RCTPicker = requireNativeComponent<

class PickerItem extends React.Component<IPickerItemProps> {
public render(): JSX.Element | null {
return null;
throw null;
}
}

Expand Down Expand Up @@ -85,20 +85,19 @@ export class Picker extends React.Component<IPickerProps, State> {
}

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 (
<RCTPicker
{...props}
{...propsWihoutChildren}
items={this.state.items}
selectedIndex={this.state.selectedIndex}
onChange={this._onChange}
ref={this._setRef}>
{this.props.children}
</RCTPicker>
ref={this._setRef}
/>
);
}

Expand Down
14 changes: 9 additions & 5 deletions vnext/src/overrides.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 39e5653

Please sign in to comment.