Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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"
}
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]
Copy link
Member

@vmoroz vmoroz Apr 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// [Windows] [](start = 77, length = 12)

Could we remove it? It seems to bring no value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been using that to annotate differences in Windows patches. E.g. to ctrl+F for without opening a diff tool.

);
} 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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw null is kind of crazy, but matches upstream behavior/behavior in Picker.js

}
}

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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line doesn't seem to do anything useful. We already do a shallow clone of props including props.style with const props = {...this.props}; above.

// eslint-disable-next-line @typescript-eslint/no-unused-vars
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a setting to disable this check when adjacent to the rest parameter that's off by default, but not in stock RN eslint rules that we're using apparently. I'd be fine changing that as well.

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