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.

Sign up for GitHub

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

Highlight all errors in VBA flow - Company step #5073

Merged
merged 15 commits into from
Sep 14, 2021
Merged

Highlight all errors in VBA flow - Company step #5073

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
80735c2
Highlight all errors company step
aldo-expensify Sep 3, 2021
79d07ed
Display error for Company Type in Company Step
aldo-expensify Sep 3, 2021
046abd2
Add error outline for StatePicker
aldo-expensify Sep 9, 2021
bf88c8e
Merge branch 'main' of github.com:Expensify/App into aldo_show-all-er…
aldo-expensify Sep 9, 2021
48c9c18
Merge branch 'main' - Solve conflicts
aldo-expensify Sep 10, 2021
21bfaa0
Fix indentation and remove empty line
aldo-expensify Sep 10, 2021
c2a569d
Follow style guidelines, refactoring
aldo-expensify Sep 13, 2021
77f2320
Remove error highlighting from ExpensiPicker
aldo-expensify Sep 13, 2021
6db9f9c
Merge branch 'main'
aldo-expensify Sep 13, 2021
2b15b07
Fix incorrect merge
aldo-expensify Sep 13, 2021
ddd6ac1
Update import after propTypes file rename
aldo-expensify Sep 14, 2021
87c1358
Remove redundant validations
aldo-expensify Sep 14, 2021
03cc2e5
Consider false a present value for required fields
aldo-expensify Sep 14, 2021
f3467e1
Put back newlines
aldo-expensify Sep 14, 2021
52aef15
Update isRequiredFulfilled boolean handling
aldo-expensify Sep 14, 2021
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
53 changes: 36 additions & 17 deletions src/components/ExpensiPicker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,19 @@ const propTypes = {

/** Should the picker appear disabled? */
isDisabled: PropTypes.bool,

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

/** Should the input be styled for errors */
hasError: PropTypes.bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: It kinda feels like we're tying the prop to UI here, I would have thought the prop should simply describe state, rather than what we do with that state. But I could be wrong, and this is not a huge deal I guess.

Copy link
Contributor

Choose a reason for hiding this comment

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

What would the alternative be here? FWIW this seems to be how we're handling things in the ExpensiTextInput

App/src/components/ExpensiTextInput/propTypes.js

Lines 13 to 17 in 3e11093

/** Error text to display */
errorText: PropTypes.string,
/** Should the input be styled for errors */
hasError: PropTypes.bool,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NAB: It kinda feels like we're tying the prop to UI here, I would have thought the prop should simply describe state, rather than what we do with that state. But I could be wrong, and this is not a huge deal I guess.

Are you saying that the comment should be updated? or the itself prop is not well defined?

};

const defaultProps = {
label: '',
isDisabled: false,
errorText: '',
hasError: false,
};

class ExpensiPicker extends PureComponent {
Expand All @@ -27,27 +35,38 @@ class ExpensiPicker extends PureComponent {

render() {
const {
label, isDisabled, ...pickerProps
errorText,
hasError,
label,
isDisabled,
...pickerProps
} = this.props;
return (
<View
style={[
styles.expensiPickerContainer,
this.state.isOpen && styles.borderColorFocus,
isDisabled && styles.inputDisabled,
]}
>
{label && (
<>
<View
style={[
styles.expensiPickerContainer,
this.state.isOpen && styles.borderColorFocus,
isDisabled && styles.inputDisabled,
(errorText || hasError) && styles.borderColorDanger,
]}
>
{label && (
<Text style={[styles.expensiPickerLabel, styles.textLabelSupporting]}>{label}</Text>
)}
<Picker
onOpen={() => this.setState({isOpen: true})}
onClose={() => this.setState({isOpen: false})}
disabled={isDisabled}
)}
<Picker
onOpen={() => this.setState({isOpen: true})}
onClose={() => this.setState({isOpen: false})}
disabled={isDisabled}
// eslint-disable-next-line react/jsx-props-no-spreading
{...pickerProps}
/>
</View>
{...pickerProps}
/>

</View>
{Boolean(errorText) && (
<Text style={[styles.formError, styles.mt1]}>{errorText}</Text>
)}
</>
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/ExpensiTextInput/propTypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const defaultProps = {
label: '',
errorText: '',
placeholder: '',
error: false,
hasError: false,
containerStyles: [],
translateX: -22,
inputStyle: [],
Expand Down
5 changes: 5 additions & 0 deletions src/components/StatePicker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@ const propTypes = {
/** The value that needs to be selected */
value: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),

/** Should the input be styled for errors */
hasError: PropTypes.bool,

...withLocalizePropTypes,
};

const defaultProps = {
value: '',
hasError: false,
};

const StatePicker = props => (
Expand All @@ -31,6 +35,7 @@ const StatePicker = props => (
onChange={props.onChange}
value={props.value}
label={props.translate('common.state')}
hasError={props.hasError}
/>
);

Expand Down
3 changes: 3 additions & 0 deletions src/languages/en.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,9 @@ export default {
taxID: 'Please enter a valid Tax ID Number',
website: 'Please enter a valid website',
zipCode: 'Please enter a valid zip code',
phoneNumber: 'Please enter a valid phone number',
companyName: 'Please enter a valid legal business name',
addressCity: 'Please enter a valid city',
addressStreet: 'Please enter a valid address street that is not a PO Box',
addressState: 'Please select a valid state',
incorporationDate: 'Please enter a valid incorporation date',
Expand Down
3 changes: 3 additions & 0 deletions src/languages/es.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,9 @@ export default {
taxID: 'Ingrese un número de identificación fiscal válido',
website: 'Ingrese un sitio web válido',
zipCode: 'Ingrese un código postal válido',
phoneNumber: 'Ingrese un teléfono válido',
companyName: 'Ingrese un nombre comercial legal válido',
addressCity: 'Ingrese una ciudad válida',
addressStreet: 'Ingrese una calle de dirección válida que no sea un apartado postal',
addressState: 'Por favor, selecciona un estado',
incorporationDate: 'Ingrese una fecha de incorporación válida',
Expand Down
Loading