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

Standardize form error styles #12098

Merged
merged 9 commits into from
Nov 3, 2022
Merged
15 changes: 5 additions & 10 deletions src/components/CheckboxWithLabel.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
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';
import InlineErrorText from './InlineErrorText';
import FormHelpMessage from './FormHelpMessage';

const requiredPropsCheck = (props) => {
if (!props.label && !props.LabelComponent) {
Expand Down Expand Up @@ -77,8 +76,6 @@ class CheckboxWithLabel extends React.Component {

this.isChecked = props.value || 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);
}
Expand All @@ -90,8 +87,8 @@ class CheckboxWithLabel extends React.Component {

render() {
return (
<>
<View style={this.wrapperStyles}>
<View style={this.props.style}>
<View style={[styles.flexRow, styles.alignItemsCenter]}>
<Checkbox
isChecked={this.isChecked}
onPress={this.toggleCheckbox}
Expand Down Expand Up @@ -121,10 +118,8 @@ class CheckboxWithLabel extends React.Component {
{this.LabelComponent && (<this.LabelComponent />)}
</TouchableOpacity>
</View>
<InlineErrorText styles={[styles.ml8]}>
{this.props.errorText}
</InlineErrorText>
</>
<FormHelpMessage message={this.props.errorText} />
</View>
);
}
}
Expand Down
67 changes: 31 additions & 36 deletions src/components/FormAlertWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,14 @@ 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 TextLink from './TextLink';
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 */
Expand Down Expand Up @@ -52,38 +50,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 => (
<View style={props.containerStyles}>
{props.isAlertVisible && (
<View style={[styles.flexRow, styles.alignItemsCenter, styles.mb3]}>
<Icon src={Expensicons.Exclamation} fill={colors.red} />
<View style={[styles.flexRow, styles.ml2, styles.flexWrap, styles.flex1]}>
{!_.isEmpty(props.message) && props.isMessageHtml && <RenderHTML html={`<muted-text>${props.message}</muted-text>`} />}

{!_.isEmpty(props.message) && !props.isMessageHtml && <Text style={styles.mutedTextLabel}>{props.message}</Text>}

{_.isEmpty(props.message) && (
<>
<Text style={styles.mutedTextLabel}>
{`${props.translate('common.please')} `}
</Text>
<TextLink
style={styles.label}
onPress={props.onFixTheErrorsLinkPressed}
>
{props.translate('common.fixTheErrors')}
</TextLink>
<Text style={styles.mutedTextLabel}>
{` ${props.translate('common.inTheFormBeforeContinuing')}.`}
</Text>
</>
)}
</View>
</View>
)}
{props.children(props.network.isOffline)}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm lost. Why did we need this? Can't we just subscribe to the Onyx network key instead of rendering a child?

@marcaaron that was already existing code before this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok thanks, let's see maybe it was me who added it lol

</View>
);
const FormAlertWrapper = (props) => {
let children;
if (_.isEmpty(props.message)) {
children = (
<Text style={[styles.formError, styles.mb0]}>
{`${props.translate('common.please')} `}
<TextLink
style={styles.label}
onPress={props.onFixTheErrorsLinkPressed}
>
{props.translate('common.fixTheErrors')}
</TextLink>
{` ${props.translate('common.inTheFormBeforeContinuing')}.`}
</Text>
);
} else if (props.isMessageHtml) {
children = <RenderHTML html={`<muted-text>${props.message}</muted-text>`} />;
Copy link
Member

Choose a reason for hiding this comment

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

You will have to modify the style for muted-text

mixedUAStyles: styles.mutedTextLabel,
.

Also, please add a new story that usage RenderHTML variant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@parasharrajat this is done
Also added video for this case

}
return (
<View style={props.containerStyles}>
{props.isAlertVisible && (
<FormHelpMessage message={props.message} style={[styles.mb3]}>
{children}
</FormHelpMessage>
)}
{props.children(props.network.isOffline)}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm lost. Why did we need this? Can't we just subscribe to the Onyx network key instead of rendering a child?

</View>
);
};

FormAlertWrapper.propTypes = propTypes;
FormAlertWrapper.defaultProps = defaultProps;
Expand Down
56 changes: 56 additions & 0 deletions src/components/FormHelpMessage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import React from 'react';
import _ from 'underscore';
import PropTypes from 'prop-types';
import {View} from 'react-native';
import Icon from './Icon';
import * as Expensicons from './Icon/Expensicons';
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. Ignored when children is not empty */
message: PropTypes.string,

/** Children to render next to dot indicator */
children: PropTypes.node,

/** Indicates whether to show error or hint */
isError: PropTypes.bool,

/** Container style props */
style: stylePropTypes,
};

const defaultProps = {
message: '',
children: null,
isError: true,
style: [],
};

const FormHelpMessage = (props) => {
if (_.isEmpty(props.message) && _.isEmpty(props.children)) {
return null;
}

return (
<View style={[styles.flexRow, styles.alignItemsCenter, styles.mt2, styles.mb1, ...props.style]}>
{props.isError && <Icon src={Expensicons.DotIndicator} fill={colors.red} />}
<View style={[styles.flex1, styles.ml2]}>
{props.children || (
<Text style={[props.isError ? styles.formError : styles.formHelp, styles.mb0]}>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<Text style={[props.isError ? styles.formError : styles.formHelp, styles.mb0]}>
<Text style={[props.isError ? styles.formError : styles.formHelp]}>

Do we need mb0 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes because both styles.formError and styles.formHelp have marginBottom: 4 and this bottom margin is already applied in parent View here (styles.mb1)
And also for vertically centered with red dot

{props.message}
</Text>
)}
</View>
</View>
);
};

FormHelpMessage.propTypes = propTypes;
FormHelpMessage.defaultProps = defaultProps;
FormHelpMessage.displayName = 'FormHelpMessage';

export default FormHelpMessage;
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
6 changes: 2 additions & 4 deletions src/components/Picker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -95,9 +95,7 @@ class Picker extends PureComponent {
onInputChange={this.onInputChange}
/>
</View>
<InlineErrorText styles={[styles.mh3]}>
{this.props.errorText}
</InlineErrorText>
<FormHelpMessage message={this.props.errorText} />
</>
);
}
Expand Down
6 changes: 2 additions & 4 deletions src/components/RadioButtonWithLabel.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -75,9 +75,7 @@ const RadioButtonWithLabel = (props) => {
{LabelComponent && (<LabelComponent />)}
</TouchableOpacity>
</View>
<InlineErrorText>
{props.errorText}
</InlineErrorText>
<FormHelpMessage message={props.errorText} />
</>
);
};
Expand Down
6 changes: 2 additions & 4 deletions src/components/TextInput/BaseTextInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -305,9 +305,7 @@ class BaseTextInput extends Component {
</TouchableWithoutFeedback>
</View>
{!_.isEmpty(inputHelpText) && (
<Text style={[formHelpStyles, styles.mt1, styles.ph3]}>
{inputHelpText}
</Text>
<FormHelpMessage isError={!_.isEmpty(this.props.errorText)} message={inputHelpText} />
)}
</View>
{/*
Expand Down
9 changes: 8 additions & 1 deletion src/stories/FormAlertWithSubmitButton.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ const Template = args => <FormAlertWithSubmitButton {...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',
Expand All @@ -26,7 +28,12 @@ Default.args = {
},
};

Default.args = defaultArgs;
const html = '<em>This is</em> a <strong>test</strong>. None of <h1>these strings</h1> should display <del>as</del> <div>HTML</div>.';
HtmlError.args = {...defaultArgs, isMessageHtml: true, message: html};

export default story;
export {
Default,
HtmlError,
};