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

7535 refactor picker compatible form #7807

Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
4ef2b51
install @storybook/addon-react-native-web
LucioChavezFuentes Feb 15, 2022
def46e5
align error text to label
LucioChavezFuentes Feb 15, 2022
01fa795
Refactor BasePicker to Picker new props
LucioChavezFuentes Feb 15, 2022
a5db0db
Create BasePicker's Web version file
LucioChavezFuentes Feb 15, 2022
68d619f
Extend focus method of View ref to scroll
LucioChavezFuentes Feb 15, 2022
d940295
refactor Picker to new proTypes
LucioChavezFuentes Feb 15, 2022
c243e76
Add Picker component to Form.stories
LucioChavezFuentes Feb 15, 2022
857ca43
create Picker.stories.js
LucioChavezFuentes Feb 16, 2022
5353111
change default 'value' prop to 'undefined'
LucioChavezFuentes Feb 16, 2022
dc3ba4c
update value propType, pass value prop
LucioChavezFuentes Feb 16, 2022
37451fa
remove unneccesary operator
LucioChavezFuentes Feb 16, 2022
82c9cd2
Revert "change default 'value' prop to 'undefined'"
LucioChavezFuentes Feb 17, 2022
addc22a
update comment about scroll to Picker on Web
LucioChavezFuentes Feb 17, 2022
0724ae4
update comment about ref, remove unneeded operator
LucioChavezFuentes Feb 18, 2022
094efba
convert index to index.native, index.web to index
LucioChavezFuentes Feb 18, 2022
170fc70
update @react-native-picker/picker
LucioChavezFuentes Feb 21, 2022
ff316d8
Merge branch 'Expensify:main' into 7535_refactor-picker-compatible-form
LucioChavezFuentes Feb 24, 2022
7fb4b0f
remove false param on scrollIntoView
LucioChavezFuentes Feb 28, 2022
58d70b5
Merge branch '7535_refactor-picker-compatible-form' of https://github…
LucioChavezFuentes Feb 28, 2022
cf69266
Merge branch 'main' into 7535_refactor-picker-compatible-form
LucioChavezFuentes Mar 1, 2022
73ef24c
Merge branch 'main' to fix storybook
LucioChavezFuentes Mar 2, 2022
e9a33e2
update react native picker
LucioChavezFuentes Mar 2, 2022
b8158b9
correct InputIDProps name
LucioChavezFuentes Mar 2, 2022
2da10cf
add style prop to InlineErrorText, add style Picker
LucioChavezFuentes Mar 2, 2022
a893f1a
remove alternative logic to Picker ref
LucioChavezFuentes Mar 2, 2022
57ab9f7
fix minor naming and empty lines
LucioChavezFuentes Mar 2, 2022
bc1310d
fix coding style errors
LucioChavezFuentes Mar 11, 2022
f12725a
merge main fix conflicts
LucioChavezFuentes Mar 11, 2022
189ca7c
rename onInputChange on Picker.stories
LucioChavezFuentes Mar 11, 2022
4287a7a
Merge branch 'main' into 7535_refactor-picker-compatible-form
LucioChavezFuentes Mar 11, 2022
5dcec43
Merge branch 'main' into 7535_refactor-picker-compatible-form
LucioChavezFuentes Mar 11, 2022
eb0013d
Merge branch '7535_refactor-picker-compatible-form' of https://github…
LucioChavezFuentes Mar 11, 2022
3f0f3da
merge main, fix inline error conflicts
LucioChavezFuentes Mar 29, 2022
7da320a
remove old picker dependency on picker-select
LucioChavezFuentes Mar 29, 2022
5060819
Merge branch 'main' into 7535_refactor-picker-compatible-form
LucioChavezFuentes Mar 31, 2022
bcc4c66
rename onChange to onInputChange on all Pickers
LucioChavezFuentes Mar 31, 2022
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
1 change: 1 addition & 0 deletions .storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module.exports = {
addons: [
'@storybook/addon-essentials',
'@storybook/addon-a11y',
'@storybook/addon-react-native-web',
],
staticDirs: [
'./public',
Expand Down
19 changes: 16 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"@react-native-firebase/crashlytics": "^12.3.0",
"@react-native-firebase/perf": "^12.3.0",
"@react-native-masked-view/masked-view": "^0.2.4",
"@react-native-picker/picker": "^1.9.11",
"@react-native-picker/picker": "^2.3.1",
"@react-navigation/compat": "^5.3.20",
"@react-navigation/drawer": "6.1.8",
"@react-navigation/native": "6.0.8",
Expand Down Expand Up @@ -127,6 +127,7 @@
"@react-native-community/eslint-config": "^2.0.0",
"@storybook/addon-a11y": "^6.4.12",
"@storybook/addon-essentials": "^6.4.12",
"@storybook/addon-react-native-web": "0.0.18",
"@storybook/addons": "^6.4.12",
"@storybook/react": "^6.4.12",
"@storybook/theming": "^6.4.12",
Expand Down
4 changes: 3 additions & 1 deletion src/components/InlineErrorText.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import Text from './Text';
const propTypes = {
/** Text to display */
children: PropTypes.string,
style: PropTypes.arrayOf(PropTypes.object),
Copy link
Member

Choose a reason for hiding this comment

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

Please add comment

};

const defaultProps = {
children: 'Error',
style: [],
};

const InlineErrorText = (props) => {
Expand All @@ -19,7 +21,7 @@ const InlineErrorText = (props) => {
}

return (
<Text style={[styles.formError, styles.mt1]}>{props.children}</Text>
<Text style={[styles.formError, styles.mt1, ...props.style]}>{props.children}</Text>
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
);
};

Expand Down
57 changes: 38 additions & 19 deletions src/components/Picker/BasePicker/index.js
Original file line number Diff line number Diff line change
@@ -1,30 +1,49 @@
import React from 'react';
import RNPickerSelect from 'react-native-picker-select';
import _ from 'underscore';

import styles from '../../../styles/styles';
import * as basePickerPropTypes from './basePickerPropTypes';
import basePickerStyles from './basePickerStyles';

const BasePicker = props => (
<RNPickerSelect
onValueChange={props.onChange}
items={props.items}
style={props.size === 'normal' ? basePickerStyles(props.disabled, props.hasError, props.focused) : styles.pickerSmall}
useNativeAndroidPickerStyle={false}
placeholder={props.placeholder}
value={props.value}
Icon={() => props.icon(props.size)}
disabled={props.disabled}
fixAndroidTouchableBug
onOpen={props.onOpen}
onClose={props.onClose}
pickerProps={{
onFocus: props.onOpen,
onBlur: props.onClose,
}}
/>
);
class BasePicker extends React.Component {
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
constructor(props) {
super(props);

this.state = {
selectedValue: this.props.value || this.props.defaultValue,
};
}

handleChange = (value) => {
Copy link
Member

Choose a reason for hiding this comment

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

use the function myFunction keyword rather than const myFunction

Reference: https://github.com/Expensify/App/blob/main/STYLE.md#functions-1

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch!

Copy link
Contributor Author

@LucioChavezFuentes LucioChavezFuentes Mar 11, 2022

Choose a reason for hiding this comment

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

Also I found I can not use handle word https://github.com/Expensify/App/blob/main/STYLE.md#event-handlers, I'll fix it accordingly.

this.props.onChange(value);
this.setState({selectedValue: value});
}

render() {
const hasError = !_.isEmpty(this.props.errorText);
return (
<RNPickerSelect
onValueChange={this.handleChange}
items={this.props.items}
style={this.props.size === 'normal' ? basePickerStyles(this.props.disabled, hasError, this.props.focused) : styles.pickerSmall}
useNativeAndroidPickerStyle={false}
placeholder={this.props.placeholder}
value={this.state.selectedValue}
Icon={() => this.props.icon(this.props.size)}
disabled={this.props.disabled}
fixAndroidTouchableBug
onOpen={this.props.onOpen}
onClose={this.props.onClose}
pickerProps={{
onFocus: this.props.onOpen,
onBlur: this.props.onBlur,
ref: this.props.innerRef,
}}
/>
);
}
}

BasePicker.propTypes = basePickerPropTypes.propTypes;
BasePicker.defaultProps = basePickerPropTypes.defaultProps;
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
33 changes: 27 additions & 6 deletions src/components/Picker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import BasePicker from './BasePicker';
import Text from '../Text';
import styles from '../../styles/styles';
import InlineErrorText from '../InlineErrorText';
import * as FormUtils from '../../libs/FormUtils';

const propTypes = {
/** Picker label */
Expand All @@ -14,22 +15,39 @@ const propTypes = {
/** Should the picker appear disabled? */
isDisabled: PropTypes.bool,

/** Should the input be styled for errors */
hasError: PropTypes.bool,
/** Input value */
value: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
luacmartins marked this conversation as resolved.
Show resolved Hide resolved

/** Error text to display */
errorText: PropTypes.string,

/** Customize the Picker container */
containerStyles: PropTypes.arrayOf(PropTypes.object),

/** Indicates that the input is being used with the Form component */
isFormInput: PropTypes.bool,

/**
* The ID used to uniquely identify the input
*
* @param {Object} props - props passed to the input
* @returns {Object} - returns an Error object if isFormInput is supplied but inputID is falsey or not a string
*/
inputID: props => FormUtils.validateInputIDProps(props),

/** Saves a draft of the input value when used in a form */
shouldSaveDraft: PropTypes.bool,
};

const defaultProps = {
label: '',
isDisabled: false,
hasError: false,
errorText: '',
containerStyles: [],
isFormInput: false,
inputID: undefined,
shouldSaveDraft: false,
value: undefined,
};

class Picker extends PureComponent {
Expand All @@ -48,6 +66,7 @@ class Picker extends PureComponent {
style={[
styles.pickerContainer,
this.props.isDisabled && styles.inputDisabled,
...this.props.containerStyles,
]}
>
{this.props.label && (
Expand All @@ -58,13 +77,14 @@ class Picker extends PureComponent {
onClose={() => this.setState({isOpen: false})}
disabled={this.props.isDisabled}
focused={this.state.isOpen}
hasError={this.props.hasError}
errorText={this.props.errorText}
value={this.props.value}
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
// eslint-disable-next-line react/jsx-props-no-spreading
{...pickerProps}
/>
</View>
{!_.isEmpty(this.props.errorText) && (
<InlineErrorText>
<InlineErrorText style={[styles.ml3]}>
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

This should also be applied from right edge as well.

Suggested change
<InlineErrorText style={[styles.ml3]}>
<InlineErrorText style={[styles.mh3]}>

{this.props.errorText}
</InlineErrorText>
)}
Expand All @@ -76,4 +96,5 @@ class Picker extends PureComponent {
Picker.propTypes = propTypes;
Picker.defaultProps = defaultProps;

export default Picker;
// eslint-disable-next-line react/jsx-props-no-spreading
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
export default React.forwardRef((props, ref) => <Picker {...props} innerRef={ref} key={props.inputID} />);
61 changes: 59 additions & 2 deletions src/stories/Form.stories.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import {View} from 'react-native';
import TextInput from '../components/TextInput';
import Picker from '../components/Picker';
import AddressSearch from '../components/AddressSearch';
import Form from '../components/Form';
import * as FormActions from '../libs/actions/FormActions';
Expand All @@ -14,7 +15,7 @@ import styles from '../styles/styles';
const story = {
title: 'Components/Form',
component: Form,
subcomponents: {TextInput, AddressSearch},
subcomponents: {TextInput, AddressSearch, Picker},
};

const Template = (args) => {
Expand Down Expand Up @@ -46,6 +47,49 @@ const Template = (args) => {
containerStyles={[styles.mt4]}
isFormInput
/>
<View>
<Picker
label="Fruit"
inputID="pickFruit"
containerStyles={[styles.mt4]}
shouldSaveDraft
items={[
{
label: 'Select a Fruit',
value: '',
},
{
label: 'Orange',
value: 'orange',
},
{
label: 'Apple',
value: 'apple',
},
]}
isFormInput
/>
</View>
<Picker
label="Another Fruit"
inputID="pickAnotherFruit"
containerStyles={[styles.mt4]}
items={[
{
label: 'Select a Fruit',
value: '',
},
{
label: 'Orange',
value: 'orange',
},
{
label: 'Apple',
value: 'apple',
},
]}
isFormInput
/>
</Form>
);
};
Expand All @@ -68,6 +112,12 @@ const defaultArgs = {
if (!values.accountNumber) {
errors.accountNumber = 'Please enter an account number';
}
if (!values.pickFruit) {
errors.pickFruit = 'Please select a fruit';
}
if (!values.pickAnotherFruit) {
errors.pickAnotherFruit = 'Please select a fruit';
}
return errors;
},
onSubmit: (values) => {
Expand All @@ -83,13 +133,20 @@ const defaultArgs = {
draftValues: {
routingNumber: '00001',
accountNumber: '1111222233331111',
pickFruit: 'orange',
pickAnotherFruit: 'apple',
},
};

Default.args = defaultArgs;
Loading.args = {...defaultArgs, formState: {isSubmitting: true}};
ServerError.args = {...defaultArgs, formState: {isSubmitting: false, serverErrorMessage: 'There was an unexpected error. Please try again later.'}};
InputError.args = {...defaultArgs, draftValues: {routingNumber: '', accountNumber: ''}};
InputError.args = {
...defaultArgs,
draftValues: {
routingNumber: '', accountNumber: '', pickFruit: '', pickAnotherFruit: '',
},
};

export default story;
export {
Expand Down
Loading