From 817f63712cd86b4980045f17df7d4c61622694a2 Mon Sep 17 00:00:00 2001 From: miroslav Date: Mon, 24 Oct 2022 14:31:05 +0100 Subject: [PATCH 1/8] Standardize form error styles --- src/components/CheckboxWithLabel.js | 6 ++-- src/components/FormAlertWrapper.js | 44 +++++++++-------------- src/components/FormHelpMessage.js | 44 +++++++++++++++++++++++ src/components/Picker/index.js | 6 ++-- src/components/RadioButtonWithLabel.js | 6 ++-- src/components/TextInput/BaseTextInput.js | 6 ++-- src/styles/styles.js | 6 ++++ 7 files changed, 75 insertions(+), 43 deletions(-) create mode 100644 src/components/FormHelpMessage.js diff --git a/src/components/CheckboxWithLabel.js b/src/components/CheckboxWithLabel.js index 6ac3180e174c..5ee5ddd779df 100644 --- a/src/components/CheckboxWithLabel.js +++ b/src/components/CheckboxWithLabel.js @@ -5,7 +5,7 @@ import _ from 'underscore'; import styles from '../styles/styles'; import Checkbox from './Checkbox'; import Text from './Text'; -import InlineErrorText from './InlineErrorText'; +import FormHelpMessage from './FormHelpMessage'; const requiredPropsCheck = (props) => { if (!props.label && !props.LabelComponent) { @@ -117,9 +117,7 @@ class CheckboxWithLabel extends React.Component { {this.LabelComponent && ()} - - {this.props.errorText} - + {this.props.errorText} ); } diff --git a/src/components/FormAlertWrapper.js b/src/components/FormAlertWrapper.js index 921a76c771fc..2b19facb81cf 100644 --- a/src/components/FormAlertWrapper.js +++ b/src/components/FormAlertWrapper.js @@ -3,16 +3,13 @@ import {View} from 'react-native'; import PropTypes from 'prop-types'; import React from 'react'; import {withNetwork} from './OnyxProvider'; -import Icon from './Icon'; -import * as Expensicons from './Icon/Expensicons'; import RenderHTML from './RenderHTML'; import TextLink from './TextLink'; -import Text from './Text'; -import colors from '../styles/colors'; import compose from '../libs/compose'; import networkPropTypes from './networkPropTypes'; import styles from '../styles/styles'; import withLocalize, {withLocalizePropTypes} from './withLocalize'; +import FormHelpMessage from './FormHelpMessage'; const propTypes = { /** Wrapped child components */ @@ -55,31 +52,24 @@ const defaultProps = { const FormAlertWrapper = props => ( {props.isAlertVisible && ( - - - - {!_.isEmpty(props.message) && props.isMessageHtml && ${props.message}`} />} + + {!_.isEmpty(props.message) && props.isMessageHtml && ${props.message}`} />} - {!_.isEmpty(props.message) && !props.isMessageHtml && {props.message}} + {!_.isEmpty(props.message) && !props.isMessageHtml && props.message} - {_.isEmpty(props.message) && ( - <> - - {`${props.translate('common.please')} `} - - - {props.translate('common.fixTheErrors')} - - - {` ${props.translate('common.inTheFormBeforeContinuing')}.`} - - - )} - - + {_.isEmpty(props.message) && ( + <> + {`${props.translate('common.please')} `} + + {props.translate('common.fixTheErrors')} + + {` ${props.translate('common.inTheFormBeforeContinuing')}.`} + + )} + )} {props.children(props.network.isOffline)} diff --git a/src/components/FormHelpMessage.js b/src/components/FormHelpMessage.js new file mode 100644 index 000000000000..e7bc3d8a2e18 --- /dev/null +++ b/src/components/FormHelpMessage.js @@ -0,0 +1,44 @@ +import React from 'react'; +import _ from 'underscore'; +import PropTypes from 'prop-types'; +import {View} from 'react-native'; +import styles from '../styles/styles'; +import Icon from './Icon'; +import * as Expensicons from './Icon/Expensicons'; +import colors from '../styles/colors'; +import Text from './Text'; + +const propTypes = { + children: PropTypes.node.isRequired, + + isError: PropTypes.bool, + + // eslint-disable-next-line react/forbid-prop-types + style: PropTypes.arrayOf(PropTypes.object), +}; + +const defaultProps = { + isError: true, + style: [], +}; + +const FormHelpMessage = (props) => { + if (_.isEmpty(props.children)) { + return null; + } + + return ( + + {props.isError && } + + {props.children} + + + ); +}; + +FormHelpMessage.propTypes = propTypes; +FormHelpMessage.defaultProps = defaultProps; +FormHelpMessage.displayName = 'DotIndicatorMessage'; + +export default FormHelpMessage; diff --git a/src/components/Picker/index.js b/src/components/Picker/index.js index 4dc676966827..a7b61beddcb9 100644 --- a/src/components/Picker/index.js +++ b/src/components/Picker/index.js @@ -5,7 +5,7 @@ import PropTypes from 'prop-types'; import BasePicker from './BasePicker'; import Text from '../Text'; import styles from '../../styles/styles'; -import InlineErrorText from '../InlineErrorText'; +import FormHelpMessage from '../FormHelpMessage'; const propTypes = { /** Picker label */ @@ -95,9 +95,7 @@ class Picker extends PureComponent { onInputChange={this.onInputChange} /> - - {this.props.errorText} - + {this.props.errorText} ); } diff --git a/src/components/RadioButtonWithLabel.js b/src/components/RadioButtonWithLabel.js index 630981970c2c..b9b75616eaf2 100644 --- a/src/components/RadioButtonWithLabel.js +++ b/src/components/RadioButtonWithLabel.js @@ -5,7 +5,7 @@ import _ from 'underscore'; import styles from '../styles/styles'; import RadioButton from './RadioButton'; import Text from './Text'; -import InlineErrorText from './InlineErrorText'; +import FormHelpMessage from './FormHelpMessage'; const propTypes = { /** Whether the radioButton is checked */ @@ -75,9 +75,7 @@ const RadioButtonWithLabel = (props) => { {LabelComponent && ()} - - {props.errorText} - + {props.errorText} ); }; diff --git a/src/components/TextInput/BaseTextInput.js b/src/components/TextInput/BaseTextInput.js index 2f970bbb12e4..81c36d4df31e 100644 --- a/src/components/TextInput/BaseTextInput.js +++ b/src/components/TextInput/BaseTextInput.js @@ -18,6 +18,7 @@ import variables from '../../styles/variables'; import Checkbox from '../Checkbox'; import getSecureEntryKeyboardType from '../../libs/getSecureEntryKeyboardType'; import CONST from '../../CONST'; +import FormHelpMessage from '../FormHelpMessage'; class BaseTextInput extends Component { constructor(props) { @@ -198,7 +199,6 @@ class BaseTextInput extends Component { const inputProps = _.omit(this.props, _.keys(baseTextInputPropTypes.propTypes)); const hasLabel = Boolean(this.props.label.length); const inputHelpText = this.props.errorText || this.props.hint; - const formHelpStyles = this.props.errorText ? styles.formError : styles.formHelp; const placeholder = (this.props.prefixCharacter || this.state.isFocused || !hasLabel || (hasLabel && this.props.forceActiveLabel)) ? this.props.placeholder : null; const textInputContainerStyles = _.reduce([ styles.textInputContainer, @@ -305,9 +305,7 @@ class BaseTextInput extends Component { {!_.isEmpty(inputHelpText) && ( - - {inputHelpText} - + {inputHelpText} )} {/* diff --git a/src/styles/styles.js b/src/styles/styles.js index 472bb0751300..e7e1787cf504 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -869,6 +869,12 @@ const styles = { marginBottom: 8, }, + formHelpMessage: { + display: 'flex', + flexDirection: 'row', + alignItems: 'center', + }, + formHelp: { color: themeColors.textSupporting, fontSize: variables.fontSizeLabel, From ae1a24e535c3b38e535d44f329a4dca893d9363c Mon Sep 17 00:00:00 2001 From: miroslav Date: Mon, 24 Oct 2022 20:22:31 +0100 Subject: [PATCH 2/8] update FormHelpMessage props --- src/components/CheckboxWithLabel.js | 2 +- src/components/FormAlertWrapper.js | 54 ++++++++++++----------- src/components/FormHelpMessage.js | 18 +++++--- src/components/Picker/index.js | 2 +- src/components/RadioButtonWithLabel.js | 2 +- src/components/TextInput/BaseTextInput.js | 2 +- 6 files changed, 45 insertions(+), 35 deletions(-) diff --git a/src/components/CheckboxWithLabel.js b/src/components/CheckboxWithLabel.js index 5ee5ddd779df..0e2421b78442 100644 --- a/src/components/CheckboxWithLabel.js +++ b/src/components/CheckboxWithLabel.js @@ -117,7 +117,7 @@ class CheckboxWithLabel extends React.Component { {this.LabelComponent && ()} - {this.props.errorText} + ); } diff --git a/src/components/FormAlertWrapper.js b/src/components/FormAlertWrapper.js index 2b19facb81cf..ef29fc0e2388 100644 --- a/src/components/FormAlertWrapper.js +++ b/src/components/FormAlertWrapper.js @@ -49,31 +49,35 @@ const defaultProps = { // // This component takes other components as a child prop. It will then render any wrapped components as a function using "render props", // and passes it a (bool) isOffline parameter. Child components can then use the isOffline variable to determine offline behavior. -const FormAlertWrapper = props => ( - - {props.isAlertVisible && ( - - {!_.isEmpty(props.message) && props.isMessageHtml && ${props.message}`} />} - - {!_.isEmpty(props.message) && !props.isMessageHtml && props.message} - - {_.isEmpty(props.message) && ( - <> - {`${props.translate('common.please')} `} - - {props.translate('common.fixTheErrors')} - - {` ${props.translate('common.inTheFormBeforeContinuing')}.`} - - )} - - )} - {props.children(props.network.isOffline)} - -); +const FormAlertWrapper = (props) => { + let message; + if (_.isEmpty(props.message)) { + message = ( + <> + {`${props.translate('common.please')} `} + + {props.translate('common.fixTheErrors')} + + {` ${props.translate('common.inTheFormBeforeContinuing')}.`} + + ); + } else if (!props.isMessageHtml) { + message = props.message; + } + return ( + + {props.isAlertVisible && ( + + {_.isEmpty(message) && ${props.message}`} />} + + )} + {props.children(props.network.isOffline)} + + ); +}; FormAlertWrapper.propTypes = propTypes; FormAlertWrapper.defaultProps = defaultProps; diff --git a/src/components/FormHelpMessage.js b/src/components/FormHelpMessage.js index e7bc3d8a2e18..1e4a887a2da2 100644 --- a/src/components/FormHelpMessage.js +++ b/src/components/FormHelpMessage.js @@ -9,7 +9,9 @@ import colors from '../styles/colors'; import Text from './Text'; const propTypes = { - children: PropTypes.node.isRequired, + message: PropTypes.node, + + children: PropTypes.node, isError: PropTypes.bool, @@ -18,27 +20,31 @@ const propTypes = { }; const defaultProps = { + message: null, + children: null, isError: true, style: [], }; const FormHelpMessage = (props) => { - if (_.isEmpty(props.children)) { + if (_.isEmpty(props.message) && _.isEmpty(props.children)) { return null; } return ( {props.isError && } - - {props.children} - + {!_.isEmpty(props.message) ? ( + + {props.message} + + ) : {props.children}} ); }; FormHelpMessage.propTypes = propTypes; FormHelpMessage.defaultProps = defaultProps; -FormHelpMessage.displayName = 'DotIndicatorMessage'; +FormHelpMessage.displayName = 'FormHelpMessage'; export default FormHelpMessage; diff --git a/src/components/Picker/index.js b/src/components/Picker/index.js index a7b61beddcb9..d9b6c00a2635 100644 --- a/src/components/Picker/index.js +++ b/src/components/Picker/index.js @@ -95,7 +95,7 @@ class Picker extends PureComponent { onInputChange={this.onInputChange} /> - {this.props.errorText} + ); } diff --git a/src/components/RadioButtonWithLabel.js b/src/components/RadioButtonWithLabel.js index b9b75616eaf2..7665cb3f9159 100644 --- a/src/components/RadioButtonWithLabel.js +++ b/src/components/RadioButtonWithLabel.js @@ -75,7 +75,7 @@ const RadioButtonWithLabel = (props) => { {LabelComponent && ()} - {props.errorText} + ); }; diff --git a/src/components/TextInput/BaseTextInput.js b/src/components/TextInput/BaseTextInput.js index 81c36d4df31e..fe29ff79bf25 100644 --- a/src/components/TextInput/BaseTextInput.js +++ b/src/components/TextInput/BaseTextInput.js @@ -305,7 +305,7 @@ class BaseTextInput extends Component { {!_.isEmpty(inputHelpText) && ( - {inputHelpText} + )} {/* From 3b4f59f4d9444f87011b3ae34d3d398190092a04 Mon Sep 17 00:00:00 2001 From: miroslav Date: Mon, 24 Oct 2022 21:52:34 +0100 Subject: [PATCH 3/8] add comments in FormHelpMessage propTypes --- src/components/FormHelpMessage.js | 14 +++++++++----- src/styles/styles.js | 6 ------ 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/components/FormHelpMessage.js b/src/components/FormHelpMessage.js index 1e4a887a2da2..bac194a8587f 100644 --- a/src/components/FormHelpMessage.js +++ b/src/components/FormHelpMessage.js @@ -2,21 +2,25 @@ import React from 'react'; import _ from 'underscore'; import PropTypes from 'prop-types'; import {View} from 'react-native'; -import styles from '../styles/styles'; import Icon from './Icon'; import * as Expensicons from './Icon/Expensicons'; -import colors from '../styles/colors'; import Text from './Text'; +import colors from '../styles/colors'; +import styles from '../styles/styles'; +import stylePropTypes from '../styles/stylePropTypes'; const propTypes = { + /** Error or hint text */ message: PropTypes.node, + /** Children to render next to dot indicator. Ignored when message is not empty */ children: PropTypes.node, + /** Indicates whether to show error or hint */ isError: PropTypes.bool, - // eslint-disable-next-line react/forbid-prop-types - style: PropTypes.arrayOf(PropTypes.object), + /** Container style props */ + style: stylePropTypes, }; const defaultProps = { @@ -32,7 +36,7 @@ const FormHelpMessage = (props) => { } return ( - + {props.isError && } {!_.isEmpty(props.message) ? ( diff --git a/src/styles/styles.js b/src/styles/styles.js index e7e1787cf504..472bb0751300 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -869,12 +869,6 @@ const styles = { marginBottom: 8, }, - formHelpMessage: { - display: 'flex', - flexDirection: 'row', - alignItems: 'center', - }, - formHelp: { color: themeColors.textSupporting, fontSize: variables.fontSizeLabel, From dcd79a62b5d6031b3238072a062c58fd4349fb1e Mon Sep 17 00:00:00 2001 From: miroslav Date: Thu, 27 Oct 2022 11:09:24 +0100 Subject: [PATCH 4/8] update FormHelpMessage component from feedback --- src/components/FormAlertWrapper.js | 17 +++++++++-------- src/components/FormHelpMessage.js | 16 +++++++++------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/components/FormAlertWrapper.js b/src/components/FormAlertWrapper.js index ef29fc0e2388..ed6c16810485 100644 --- a/src/components/FormAlertWrapper.js +++ b/src/components/FormAlertWrapper.js @@ -4,6 +4,7 @@ import PropTypes from 'prop-types'; import React from 'react'; import {withNetwork} from './OnyxProvider'; import RenderHTML from './RenderHTML'; +import Text from './Text'; import TextLink from './TextLink'; import compose from '../libs/compose'; import networkPropTypes from './networkPropTypes'; @@ -50,10 +51,10 @@ const defaultProps = { // This component takes other components as a child prop. It will then render any wrapped components as a function using "render props", // and passes it a (bool) isOffline parameter. Child components can then use the isOffline variable to determine offline behavior. const FormAlertWrapper = (props) => { - let message; + let children; if (_.isEmpty(props.message)) { - message = ( - <> + children = ( + {`${props.translate('common.please')} `} { {props.translate('common.fixTheErrors')} {` ${props.translate('common.inTheFormBeforeContinuing')}.`} - + ); - } else if (!props.isMessageHtml) { - message = props.message; + } else if (props.isMessageHtml) { + children = ${props.message}`} />; } return ( {props.isAlertVisible && ( - - {_.isEmpty(message) && ${props.message}`} />} + + {children} )} {props.children(props.network.isOffline)} diff --git a/src/components/FormHelpMessage.js b/src/components/FormHelpMessage.js index bac194a8587f..dc6c8e5d114c 100644 --- a/src/components/FormHelpMessage.js +++ b/src/components/FormHelpMessage.js @@ -11,7 +11,7 @@ import stylePropTypes from '../styles/stylePropTypes'; const propTypes = { /** Error or hint text */ - message: PropTypes.node, + message: PropTypes.string, /** Children to render next to dot indicator. Ignored when message is not empty */ children: PropTypes.node, @@ -24,7 +24,7 @@ const propTypes = { }; const defaultProps = { - message: null, + message: '', children: null, isError: true, style: [], @@ -38,11 +38,13 @@ const FormHelpMessage = (props) => { return ( {props.isError && } - {!_.isEmpty(props.message) ? ( - - {props.message} - - ) : {props.children}} + + {props.children || ( + + {props.message} + + )} + ); }; From 236d9c27d69c806a64a9790b9f1310d58d295012 Mon Sep 17 00:00:00 2001 From: miroslav Date: Thu, 27 Oct 2022 11:10:32 +0100 Subject: [PATCH 5/8] add another 4px top margin between indicator message and input --- src/components/FormHelpMessage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/FormHelpMessage.js b/src/components/FormHelpMessage.js index dc6c8e5d114c..090d9fb15b10 100644 --- a/src/components/FormHelpMessage.js +++ b/src/components/FormHelpMessage.js @@ -36,7 +36,7 @@ const FormHelpMessage = (props) => { } return ( - + {props.isError && } {props.children || ( From 26e594885754e98510c94690e232eb176ec68000 Mon Sep 17 00:00:00 2001 From: miroslav Date: Fri, 28 Oct 2022 16:50:03 +0100 Subject: [PATCH 6/8] update comment on FormHelpMessage --- src/components/FormHelpMessage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/FormHelpMessage.js b/src/components/FormHelpMessage.js index 090d9fb15b10..d2ffcf62fa03 100644 --- a/src/components/FormHelpMessage.js +++ b/src/components/FormHelpMessage.js @@ -10,10 +10,10 @@ import styles from '../styles/styles'; import stylePropTypes from '../styles/stylePropTypes'; const propTypes = { - /** Error or hint text */ + /** Error or hint text. Ignored when children is not empty */ message: PropTypes.string, - /** Children to render next to dot indicator. Ignored when message is not empty */ + /** Children to render next to dot indicator */ children: PropTypes.node, /** Indicates whether to show error or hint */ From 0477a6ef020779e608b02753b056f952f54fc951 Mon Sep 17 00:00:00 2001 From: miroslav Date: Mon, 31 Oct 2022 17:39:25 +0100 Subject: [PATCH 7/8] move error inside wrapper on CheckboxWithLabel component --- src/components/CheckboxWithLabel.js | 9 +++------ src/components/FormHelpMessage.js | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/components/CheckboxWithLabel.js b/src/components/CheckboxWithLabel.js index 0e2421b78442..71438f6d760a 100644 --- a/src/components/CheckboxWithLabel.js +++ b/src/components/CheckboxWithLabel.js @@ -1,7 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import {View, TouchableOpacity} from 'react-native'; -import _ from 'underscore'; import styles from '../styles/styles'; import Checkbox from './Checkbox'; import Text from './Text'; @@ -71,8 +70,6 @@ class CheckboxWithLabel extends React.Component { this.isChecked = props.defaultValue || props.isChecked; this.LabelComponent = props.LabelComponent; - this.defaultStyles = [styles.flexRow, styles.alignItemsCenter]; - this.wrapperStyles = _.isArray(props.style) ? [...this.defaultStyles, ...props.style] : [...this.defaultStyles, props.style]; this.toggleCheckbox = this.toggleCheckbox.bind(this); } @@ -84,8 +81,8 @@ class CheckboxWithLabel extends React.Component { render() { return ( - <> - + + - + ); } } diff --git a/src/components/FormHelpMessage.js b/src/components/FormHelpMessage.js index d2ffcf62fa03..3a78b279c962 100644 --- a/src/components/FormHelpMessage.js +++ b/src/components/FormHelpMessage.js @@ -36,7 +36,7 @@ const FormHelpMessage = (props) => { } return ( - + {props.isError && } {props.children || ( From 8700155db5330e692ef8774432a8ab8b23773fc2 Mon Sep 17 00:00:00 2001 From: miroslav Date: Tue, 1 Nov 2022 21:41:48 +0100 Subject: [PATCH 8/8] add RenderHTML to storybook --- .../HTMLEngineProvider/BaseHTMLEngineProvider.js | 2 +- src/stories/FormAlertWithSubmitButton.stories.js | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js index 459e904a0bba..c644db26d153 100755 --- a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js +++ b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js @@ -29,7 +29,7 @@ const customHTMLElementModels = { }), 'muted-text': defaultHTMLElementModels.div.extend({ tagName: 'muted-text', - mixedUAStyles: styles.mutedTextLabel, + mixedUAStyles: {...styles.formError, ...styles.mb0}, }), comment: defaultHTMLElementModels.div.extend({ tagName: 'comment', diff --git a/src/stories/FormAlertWithSubmitButton.stories.js b/src/stories/FormAlertWithSubmitButton.stories.js index 9747c447cf0d..eb58b7b59322 100644 --- a/src/stories/FormAlertWithSubmitButton.stories.js +++ b/src/stories/FormAlertWithSubmitButton.stories.js @@ -17,7 +17,9 @@ const Template = args => ; // Arguments can be passed to the component by binding // See: https://storybook.js.org/docs/react/writing-stories/introduction#using-args const Default = Template.bind({}); -Default.args = { +const HtmlError = Template.bind({}); + +const defaultArgs = { isAlertVisible: true, onSubmit: () => {}, buttonText: 'Submit', @@ -26,7 +28,12 @@ Default.args = { }, }; +Default.args = defaultArgs; +const html = 'This is a test. None of

these strings

should display as
HTML
.'; +HtmlError.args = {...defaultArgs, isMessageHtml: true, message: html}; + export default story; export { Default, + HtmlError, };