The form error message isn't showing up correctly in spanish - reported by @Tushu17

[mvtglobally]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open App
2. Go to preference and change preferred language to spanish.
3. Go to Request call page/Connect bank account page.
4. Submit unfilled form

Expected Result:

Error message shouldn't leave any blank space on side.

Actual Result:

It is leaving blank space on side.

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.2.7-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:
Issue reported by: @Tushu17
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1663317764966639

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @sonialiap (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[Tushu17]

Proposal

we need to keep Text component as a parent in

App/src/components/FormAlertWrapper.js

Lines 66 to 80 in 3e03f08

	<>
	<Text style={styles.mutedTextLabel}>
	{`${props.translate('common.please')} `}
	</Text>
	<TextLink
	style={styles.label}
	onPress={props.onFixTheErrorsPressed}
	>
	{props.translate('common.fixTheErrors')}
	</TextLink>
	<Text style={styles.mutedTextLabel}>
	{` ${props.translate('common.inTheFormBeforeContinuing')}.`}
	</Text>
	</>
	)}

-                           <>
                            <Text style={styles.mutedTextLabel}>
                                {`${props.translate('common.please')} `}
-                            </Text>
                            <TextLink
                                style={styles.label}
                                onPress={props.onFixTheErrorsPressed}
                            >
                                {props.translate('common.fixTheErrors')}
                            </TextLink>
-                            <Text style={styles.mutedTextLabel}>
                                {` ${props.translate('common.inTheFormBeforeContinuing')}.`}
                            </Text>
-                        </>

OR

-                          <>
+                         <Text>
                            <Text style={styles.mutedTextLabel}>
                                {`${props.translate('common.please')} `}
                            </Text>
                            <TextLink
                                style={styles.label}
                                onPress={props.onFixTheErrorsPressed}
                            >
                                {props.translate('common.fixTheErrors')}
                            </TextLink>
                            <Text style={styles.mutedTextLabel}>
                                {` ${props.translate('common.inTheFormBeforeContinuing')}.`}
                            </Text>
-                        </>
+                       </Text>

[akshayasalvi]

Proposal

I would go with replacing the <> with View. So in FormAlertWrapper, I would do this:

+ <View style={[styles.dInline, styles.w100]}>
- <>
    <Text style={styles.mutedTextLabel}>
        {`${props.translate('common.please')} `}
    </Text>
    <TextLink
        style={styles.label}
        onPress={props.onFixTheErrorsPressed}
    >
        {props.translate('common.fixTheErrors')}
    </TextLink>
    <Text style={styles.mutedTextLabel}>
        {` ${props.translate('common.inTheFormBeforeContinuing')}.`}
    </Text>
+ </View>
- <>

Because based on my experience, having nested Text especially with link, have issues with accessibility.

[melvin-bot]

Triggered auto assignment to @bondydaa (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[bondydaa]

hmm I've had a few style/design things come up on various views (this is the other one #11148)

cc @Expensify/design can you take a look and confirm how the spacing should look for Brick Road errors on this and #11148

Then we can take care of all of these in one go instead of playing wack-a-mole with styles on one view that then breaks another view that then breaks another view.

[melvin-bot]

Triggered auto assignment to @shawnborton (Design), see these Stack Overflow questions for more details.

[melvin-bot]

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[shawnborton]

Well I think two things are happening here:

• In the example above, the blue link is causing a line break after it, and thus the weird space afterwards
• We should switch over to the new red brick road/dot indicator for this error message

[akshayasalvi]

@shawnborton Yes, because the blue link is using TextLink component and hence I recommended to use dLine style in my proposal #11410 (comment).

Also for the second point do you mean replacing the red exclamation mark with the red brick road? That would mean the replacing the following line in FormAlertWrapper as mentioned in the code block.

App/src/components/FormAlertWrapper.js

Line 59 in 6748e8a

<Icon src={Expensicons.Exclamation} fill={colors.red} />

- <Icon src={Expensicons.Exclamation} fill={colors.red} />
+ <Icon
+      src={Expensicons.DotIndicator}
+      fill={colors.red}
+      height={variables.iconSizeSmall}
+     width={variables.iconSizeSmall}
+  />

[shawnborton]

Instead of replacing FormAlertWrapper with the code you mentioned, is there a way we can just reuse the red dot indicator component? cc @iwiznia @jasperhuangg for thoughts there.

[akshayasalvi]

I can do that if that's what is expected, but FormAlertWrapper is used on all the forms,

1. that would require QA for all the forms.
2. FormAlertWrapper supports three types of messages (and DotIndicator only supports an array of Text messages):
   a. HTML messages with RenderHTML
   b. Messages passed in props shown as Text
   c. No message passed (used in Form), with default error message as a combination of Text, TextLink and Text.
3. We don't support blue TextLink format in DotIndicatorMessage

We can either change the whole component with all the above points or just put the Red indicator like previous proposal.

[iwiznia]

hmmmm besides the points made above, the behavior also feels quite different for both components, so not sure if unifying them is good.

[shawnborton]

Got it. I'm mostly just saying that the exact style/format of the red dot indicator with message should be used here, so maybe there is a way to DRY it up. Either way works for me though, but from a design standpoint, they should look the same.

[akshayasalvi]

So what is expected here @iwiznia @shawnborton ? Are we good with the red dot indicator here #11410 (comment)?

[iwiznia]

I think so

[Puneet-here]

I think we should refactor the DotIndicatorMessage so that we can reuse DotIndicatorMessage at most places. We are also going to use it for normal errors so it makes sense to refactor it. We just need to move FormAlertWrapper render functionality to DotIndicatorMessage

[iwiznia]

If we can do that without many hacks, sounds good. The main problem I see is that both use different formats (one supports just one message and apparently html (?) and the other supports multiple ones and no html.

[Puneet-here]

I believe it can be done without hacks because we can use props to tell about what type of message DotIndicatorMessage needs to show or we can check what type of message is being sent inside DotIndicatorMessage to decide how we wanna render it. For example when we want a label with links we send a prop LabelComponent and the the message in the form of component to the CheckboxLabel and then it renders it as it is.

one supports just one message and apparently html (?) and the other supports multiple ones and no html

IIRC DotIndicatorMessage currently renders the error messages passed as an object and the FormAlertWrapper renders html, strings and a message with link in it when no error message is being passed.

[bondydaa]

Ionatan has been OOO for most of this week so will wait for him to get back to help us decide how we want to approach this.

[iwiznia]

If we can reuse, then sounds better, but not exactly sure I follow the LabelComponent proposal

[shawnborton]

Not overdue, but what's the latest update here?

[0xmiros]

@shawnborton I think #11908 will fix this issue automatically so I'd rather say put on HOLD until #12098 is merged.
Also, we're waiting for your input on that PR

[shawnborton]

That works for me! Will comment over there shortly.

[bondydaa]

should we just close this out then since it's covered by #11908? i'm just going to do that, feel free to reopen if necessary.

[Tushu17]

Reporting compensation is pending here.
cc- @shawnborton @bondydaa

[JmillsExpensify]

I'll handle that, one second.

[JmillsExpensify]

Upwork issue is here for reporting: https://www.upwork.com/jobs/~01fce977f1f5d66b7a. Please apply and I'll issue payment.

[Tushu17]

Please apply and I'll issue payment.

Okay applied, Thanks @JmillsExpensify

[JmillsExpensify]

Offer extended via Upwork.

[Tushu17]

@JmillsExpensify Thanks, Offer accepted

[JmillsExpensify]

Thanks! I issued payment in Upwork, so I'm closing the issue again.