Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor picker to be a cross-platform component #2242

Merged
merged 15 commits into from
Apr 12, 2021
8 changes: 4 additions & 4 deletions src/components/Checkbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const propTypes = {
isChecked: PropTypes.bool.isRequired,

// A function that is called when the box/label is clicked on
onCheckboxClick: PropTypes.func.isRequired,
onClick: PropTypes.func.isRequired,

// Text that appears next to check box
label: PropTypes.string,
Expand All @@ -22,17 +22,17 @@ const defaultProps = {

const Checkbox = ({
isChecked,
onCheckboxClick,
onClick,
label,
}) => (
<View style={styles.flexRow}>
<Pressable onPress={() => onCheckboxClick(!isChecked)}>
<Pressable onPress={() => onClick(!isChecked)}>
<View style={[styles.checkboxContainer, isChecked && styles.checkedContainer]}>
<Icon src={Checkmark} fill="white" height={14} width={14} />
</View>
</Pressable>
{label && (
<Pressable onPress={() => onCheckboxClick(!isChecked)}>
<Pressable onPress={() => onClick(!isChecked)}>
<Text style={[styles.ml2, styles.textP]}>
{label}
</Text>
Expand Down
43 changes: 43 additions & 0 deletions src/components/Picker/PickerPropTypes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import PropTypes from 'prop-types';

const propTypes = {
// A callback method that is called when the value changes and it received the selected value as an argument
onChange: PropTypes.func.isRequired,

// Whether or not to show the disabled styles
useDisabledStyles: PropTypes.bool,

// The items to display in the list of selections
items: PropTypes.arrayOf(PropTypes.shape({
// The value of the item that is being selected
value: PropTypes.string.isRequired,

// The text to display for the item
label: PropTypes.string.isRequired,
})).isRequired,

// Something to show as the placeholder before something is selected
placeholder: PropTypes.shape({
// The value of the placeholder item, usually an empty string
value: PropTypes.string,

// The text to be displayed as the placeholder
label: PropTypes.string,
}),

// The value that needs to be selected
value: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),

// An icon to display with the picker
icon: PropTypes.func.isRequired,
};
const defaultProps = {
useDisabledStyles: false,
placeholder: {},
value: null,
};

export {
propTypes,
defaultProps,
};
31 changes: 31 additions & 0 deletions src/components/Picker/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React from 'react';
import RNPickerSelect from 'react-native-picker-select';

import styles from '../../styles/styles';
import pickerDisabledStyles from './pickerDisabledStyles';
import * as pickerPropTypes from './PickerPropTypes';

const Picker = ({
onChange,
items,
useDisabledStyles,
placeholder,
value,
icon,
}) => (
<RNPickerSelect
onValueChange={onChange}
items={items}
style={useDisabledStyles ? pickerDisabledStyles : styles.picker}
useNativeAndroidPickerStyle={false}
placeholder={placeholder}
value={value}
Icon={icon}
/>
);

Picker.propTypes = pickerPropTypes.propTypes;
Picker.defaultProps = pickerPropTypes.defaultProps;
Picker.displayName = 'Picker';

export default Picker;
6 changes: 6 additions & 0 deletions src/components/Picker/pickerDisabledStyles/index.android.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import styles from '../../../styles/styles';

export default {
...styles.picker,
inputAndroid: [styles.picker.inputAndroid, styles.textInput, styles.disabledTextInput],
};
6 changes: 6 additions & 0 deletions src/components/Picker/pickerDisabledStyles/index.ios.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import styles from '../../../styles/styles';

export default {
...styles.picker,
inputIOS: [styles.picker.inputIOS, styles.textInput, styles.disabledTextInput],
};
6 changes: 6 additions & 0 deletions src/components/Picker/pickerDisabledStyles/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import styles from '../../../styles/styles';

export default {
...styles.picker,
inputWeb: [styles.picker.inputWeb, styles.textInput, styles.disabledTextInput],
};
2 changes: 1 addition & 1 deletion src/libs/Navigation/AppNavigator/ModalStackNavigators.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import DetailsPage from '../../../pages/DetailsPage';
import IOURequestPage from '../../../pages/iou/IOURequestPage';
import IOUBillPage from '../../../pages/iou/IOUBillPage';
import SettingsInitialPage from '../../../pages/settings/InitialPage';
import SettingsProfilePage from '../../../pages/settings/ProfilePage';
import SettingsProfilePage from '../../../pages/settings/Profile/ProfilePage';
import SettingsPreferencesPage from '../../../pages/settings/PreferencesPage';
import SettingsPasswordPage from '../../../pages/settings/PasswordPage';
import SettingsPaymentsPage from '../../../pages/settings/PaymentsPage';
Expand Down
3 changes: 2 additions & 1 deletion src/pages/home/report/ReportActionContextMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ const propTypes = {
reportID: PropTypes.number.isRequired,

// The report action this context menu is attached to.
reportAction: PropTypes.shape(ReportActionPropTypes).isRequired,
reportAction: PropTypes.shape(ReportActionPropTypes),

// If true, this component will be a small, row-oriented menu that displays icons but not text.
// If false, this component will be a larger, column-oriented menu that displays icons alongside text in each row.
Expand All @@ -92,6 +92,7 @@ const propTypes = {
};

const defaultProps = {
reportAction: {},
isMini: false,
isVisible: false,
};
Expand Down
10 changes: 5 additions & 5 deletions src/pages/home/report/ReportActionPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ import ReportActionFragmentPropTypes from './ReportActionFragmentPropTypes';

export default {
// Name of the action e.g. ADDCOMMENT
actionName: PropTypes.string.isRequired,
actionName: PropTypes.string,
Copy link
Contributor

Choose a reason for hiding this comment

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

why the switch to making these prop optional?

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 was a prop warning in the console that undefined was being passed. I looked through the code and there is a spot where the report action passed is {} (an empty object) so I just corrected the propTypes here.


// Person who created the action
person: PropTypes.arrayOf(ReportActionFragmentPropTypes).isRequired,
person: PropTypes.arrayOf(ReportActionFragmentPropTypes),

// ID of the report action
sequenceNumber: PropTypes.number.isRequired,
sequenceNumber: PropTypes.number,

// Unix timestamp
timestamp: PropTypes.number.isRequired,
timestamp: PropTypes.number,

// report action message
message: PropTypes.arrayOf(ReportActionFragmentPropTypes).isRequired,
message: PropTypes.arrayOf(ReportActionFragmentPropTypes),
};
13 changes: 4 additions & 9 deletions src/pages/settings/PreferencesPage.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from 'react';
import {View} from 'react-native';
import RNPickerSelect from 'react-native-picker-select';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';

Expand All @@ -17,6 +16,7 @@ import {DownArrow} from '../../components/Icon/Expensicons';
import {setExpensifyNewsStatus} from '../../libs/actions/User';
import ScreenWrapper from '../../components/ScreenWrapper';
import Switch from '../../components/Switch';
import Picker from '../../components/Picker';

const propTypes = {
// The chat priority mode
Expand Down Expand Up @@ -76,18 +76,13 @@ const PreferencesPage = ({priorityMode, user}) => (
Priority Mode
</Text>
<View style={[styles.mb2]}>
{/* empty object in placeholder below to prevent default */}
{/* placeholder from appearing as a selection option. */}
<RNPickerSelect
onValueChange={
<Picker
onChange={
mode => NameValuePair.set(CONST.NVP.PRIORITY_MODE, mode, ONYXKEYS.NVP_PRIORITY_MODE)
}
items={Object.values(priorityModes)}
style={styles.picker}
useNativeAndroidPickerStyle={false}
placeholder={{}}
value={priorityMode}
Icon={() => <Icon src={DownArrow} />}
icon={() => <Icon src={DownArrow} />}
/>
</View>
<Text style={[styles.textLabel, styles.colorMuted]}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
TextInput,
Pressable,
} from 'react-native';
import RNPickerSelect from 'react-native-picker-select';
import Str from 'expensify-common/lib/str';
import moment from 'moment-timezone';
import _ from 'underscore';
Expand All @@ -27,6 +26,7 @@ import LoginField from './LoginField';
import {DownArrow, Upload, Trashcan} from '../../../components/Icon/Expensicons';
import AttachmentPicker from '../../../components/AttachmentPicker';
import CreateMenu from '../../../components/CreateMenu';
import Picker from '../../../components/Picker';

const propTypes = {
/* Onyx Props */
Expand Down Expand Up @@ -315,17 +315,15 @@ class ProfilePage extends Component {
<View style={styles.mb6}>
<Text style={[styles.mb1, styles.formLabel]}>Preferred Pronouns</Text>
<View style={styles.mb1}>
<RNPickerSelect
onValueChange={pronouns => this.setState({pronouns, selfSelectedPronouns: ''})}
<Picker
onChange={pronouns => this.setState({pronouns, selfSelectedPronouns: ''})}
items={this.pronounDropdownValues}
style={styles.picker}
useNativeAndroidPickerStyle={false}
placeholder={{
value: '',
label: 'Select your pronouns',
}}
value={this.state.pronouns}
Icon={() => <Icon src={DownArrow} />}
icon={() => <Icon src={DownArrow} />}
/>
</View>
{this.state.pronouns === CONST.PRONOUNS.SELF_SELECT && (
Expand All @@ -342,27 +340,19 @@ class ProfilePage extends Component {
<LoginField label="Phone Number" type="phone" login={this.state.logins.phone} />
<View style={styles.mb3}>
<Text style={[styles.mb1, styles.formLabel]}>Timezone</Text>
<RNPickerSelect
onValueChange={selectedTimezone => this.setState({selectedTimezone})}
<Picker
onChange={selectedTimezone => this.setState({selectedTimezone})}
items={timezones}
style={this.state.isAutomaticTimezone ? {
...styles.picker,
inputIOS: [styles.picker.inputIOS, styles.textInput, styles.disabledTextInput],
inputAndroid: [
styles.picker.inputAndroid, styles.textInput, styles.disabledTextInput,
],
inputWeb: [styles.picker.inputWeb, styles.textInput, styles.disabledTextInput],
} : styles.picker}
useNativeAndroidPickerStyle={false}
useDisabledStyles={this.state.isAutomaticTimezone}
value={this.state.selectedTimezone}
Icon={() => <Icon src={DownArrow} />}
icon={() => <Icon src={DownArrow} />}
disabled={this.state.isAutomaticTimezone}
/>
</View>
<Checkbox
label="Set my timezone automatically"
isChecked={this.state.isAutomaticTimezone}
onCheckboxClick={this.setAutomaticTimezone}
onClick={this.setAutomaticTimezone}
/>
</View>
<View style={[styles.ph5, styles.pb5]}>
Expand Down