[HOLD for payment 2022-12-22] [$1000] mWeb - Workspace - The error text message extends beyond the screen

[kbecciv]

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. Login with the same account on 2 different devices
2. On device 1 (mWeb/Chrome) go to settings > workspaces
3. Open a workspace
4. Now at device 2 (Web) go to the same workspace and delete it
5. Immediately delete the same workspace on device 1 (mWeb/Chrome)

Expected Result:

The text error message does NOT extend beyond the screen

Actual Result:

The error text message extends beyond the screen

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Mobile Web

Version Number: 1.2.38.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Bug5857847_video_2022-12-10_19-01-00.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ada03f592418685a
• Upwork Job ID: 1602380086319108096

[melvin-bot]

Triggered auto assignment to @maddylewis (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[situchan]

Proposal

This is a regression from #13475 which fixes #13466

Here's the solution to fix this GH without reverting #13475:

https://github.com/Expensify/App/blob/main/src/components/OfflineWithFeedback.js#L105-L117

                <View style={StyleUtils.combineStyles(styles.offlineFeedback.error, props.errorRowStyles)}>
-                   <DotIndicatorMessage messages={props.errors} type="error" />
+                   <DotIndicatorMessage style={[styles.flex1]} messages={props.errors} type="error" />
                    <Tooltip text={props.translate('common.close')}>
                        <Pressable
                            onPress={props.onClose}
                            style={[styles.touchableButtonImage, styles.mr0]}
                            accessibilityRole="button"
                            accessibilityLabel={props.translate('common.close')}
                        >
                            <Icon src={Expensicons.Close} />
                        </Pressable>
                    </Tooltip>
                </View>

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker 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 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.

[pecanoro]

Taking over as CME to move things faster.

[melvin-bot]

Current assignee @maddylewis is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01ada03f592418685a

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External)

[melvin-bot]

Current assignee @pecanoro is eligible for the External assigner, not assigning anyone new.

[pecanoro]

@situchan your proposal sounds good, did you make sure your proposal does not get any regressions related to the rest of the one-dot indicators? What about the original issue? Does the message still show?

[situchan]

@situchan your proposal sounds good, did you make sure your proposal does not get any regressions related to the rest of the one-dot indicators? What about the original issue? Does the message still show?

yes I tested all the cases.
The issue happens only when parent view of DotIndicatorMessage has style with flexDirection: row and this case flex: 1 should be applied to DotIndicatorMessage

[pecanoro]

@situchan Great! Since it's a deploy blocker here, I will assign you so we can have a PR asap.

[melvin-bot]

📣 @situchan You have been assigned to this job by @pecanoro!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[pecanoro]

@situchan Since it's a deploy blocker, do you think you can get a PR today in the next couple of hours so that we can merge? Otherwise, I will have to take over since it's blocking us.

[pecanoro]

Also, take into account that there is a bonus if a PR gets merged within 3 business days if that is an incentive to get it ready asap 😄

[chiragsalian]

@situchan so how's the progress here and when can we expect a PR?

[situchan]

@situchan so how's the progress here and when can we expect a PR?

in 30 min

[situchan]

@pecanoro PR is ready

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.38-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix error message style issue on OfflineWithFeedback #13535

If no regressions arise, payment will be issued on 2022-12-20. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@Santhosh-Sellavel / @pecanoro] The PR that introduced the bug has been identified. Link to the PR: Remove extra flex from style #13475
• [@Santhosh-Sellavel / @pecanoro] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: Remove extra flex from style #13475 (comment)
• [@Santhosh-Sellavel / @pecanoro] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@maddylewis] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here: ([HOLD for payment 2022-12-22] [$1000] mWeb - Workspace - The error text message extends beyond the screen #13505 (comment))

[Santhosh-Sellavel]

Regression caused by this #13475, we have it found here already

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.39-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix error message style issue on OfflineWithFeedback #13535

If no regressions arise, payment will be issued on 2022-12-22. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[Santhosh-Sellavel]

@maddylewis bump on payment!

[pecanoro]

No need for "A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner." since there is actually a bullet point for it already that was missed 😅

[maddylewis]

@Santhosh-Sellavel @situchan - can you apply to the job so that i can process payment?
https://www.upwork.com/jobs/~01ada03f592418685a

[situchan]

@maddylewis applied. thank you

[pecanoro]

[@maddylewis] A regression test has been added or updated so that the same bug will not reach production again.

@maddylewis For the regression test you need to follow the steps here to double-check in TestRail.

[Santhosh-Sellavel]

@maddylewis Applied for the job!

[mallenexpensify]

Looks like the PR was merged the same day @situchan was hired (not sure I've seen that before, nice work)
Paid @situchan $1500 which includes the 50% bonus.
Hired @Santhosh-Sellavel can you please accept the job and reply here once you have?

[Santhosh-Sellavel]

Done!

[mallenexpensify]

Paid @Santhosh-Sellavel $1500. Closing. I closed the below issue in favor of this one too, they were fixed by same PR.

#13510 (comment)

[maddylewis]

i appreciate the assist @mallenexpensify - thank you!