[HOLD for payment 2024-06-06] [$250] [Banner] text overflow on native

[rushatgabhane]

Coming from #41199 (review)

Problem

On native platforms, the text inside Banner component overflows.

Use the linked PR to test it.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01da04c416bad8e2f6
• Upwork Job ID: 1792579160211070976
• Last Price Increase: 2024-05-20
• Automatic offers:
• Krishna2323 | Contributor | 0

Issue Owner

Current Issue Owner: @lschurr

[melvin-bot]

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[Krishna2323]

Proposal

Please re-state the problem that we are trying to solve in this issue.

[Banner] text overflow on native

What is the root cause of that problem?

This is a know issue in react native and is caused by using flexRow on parent view.

What changes do you think we should make in order to solve the problem?

We need to add {flex: 1, flexWrap: 'wrap'} style to Text inside Banner. Or { flexShrink: 1 }.

App/src/components/Banner.tsx

Lines 99 to 103 in e8ae3c5

	<Text
	style={textStyles}
	onPress={onPress}
	suppressHighlighting
	>

We also need to apply styles.justifyContentCenter to main View container.

App/src/components/Banner.tsx

Line 74 in e8ae3c5

<View

Or we can move <Tooltip text={translate('common.close')}> into the same view container which renders the icon and text and then refactor some stylings.

What alternative solutions did you explore? (Optional)

Result

[melvin-bot]

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

[melvin-bot]

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

[rushatgabhane]

@sobitneupane jumping the gun here
@Krishna2323 could you please raise a PR? It's blocking PR #41199 (review)

[rushatgabhane]

🎀 👀 🎀

[melvin-bot]

Triggered auto assignment to @francoisl, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Krishna2323]

@rushatgabhane, should I raise the PR before assignment?

[rushatgabhane]

my bad, we should wait for @francoisl

[francoisl]

LGTM, feel free to send the PR now.

[melvin-bot]

📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• fix: [Banner] text overflow on native. #42398

If no regressions arise, payment will be issued on 2024-06-06. 🎊

For reference, here are some details about the assignees on this issue:

• @Krishna2323 requires payment automatic offer (Contributor)

[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:

• [@francoisl] The PR that introduced the bug has been identified. Link to the PR:
• [@francoisl] 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:
• [@francoisl] 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:
• [@Krishna2323] Determine if we should create a regression test for this bug.
• [@Krishna2323] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@lschurr] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[rushatgabhane]

@lschurr could you please assign me the issue for c+

[rushatgabhane]

1. The PR that introduced the bug has been identified. Link to the PR: Render banner in Concierge chat #11363

2. 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: https://github.com/Expensify/App/pull/11363/files#r1621464572

3. 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: N.A. PR was 2 years old

4. Determine if we should create a regression test for this bug. Yes

5. If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again

      1. Open the app in two devices.
      2. In the first device login with user A.
      3. Click fab > Start chat > Room.
      4. Type the room name and set "Who can post." to "Admin only" then complete creating proccess.
      5. Invite user B to the room.
      6. In the other device login with user B and go to the room and Verify that "admins only can post" message is displayed on chat footer rather than welcome message.
      7. Verify the banner text is wrapped into multiple lines and does not overflow on small width devices
   

[rushatgabhane]

@lschurr could you please attach payment summary, thank you!

[lschurr]

Looks like the automation didn't work.

Payment summary:

• Contributor: @Krishna2323 $250 (paid in Upwork)
• Contributor+: @rushatgabhane $250 (please request in NewDot)

[lschurr]

Requested QA test be added. Closing this one out.

[JmillsExpensify]

$250 approved for @rushatgabhane