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

[PAID - Reporting] IOU - When the name of the Merchant is too long, text is out of field in IOU report #25780

Closed
6 tasks done
lanitochka17 opened this issue Aug 23, 2023 · 41 comments
Assignees
Labels
Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@lanitochka17
Copy link

lanitochka17 commented Aug 23, 2023

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


Issue found when executing PR #25738

Action Performed:

  1. Open staging.new.expensify.com
  2. Go to any chat/report
  3. Click on "+" icon
  4. Click on "Request money"
  5. Enter any amount and click "Next"
  6. Click on "Description"
  7. Enter any text over 300 characters
  8. Click on Save
  9. Click on "Show more"
  10. Click on "Merchant"
  11. Enter any text over 300 characters
  12. Click on Save
  13. Click on "Request"
  14. Click on the message to go to the IOU report

Expected Result:

Name of merchant should be contained inside the text box

Actual Result:

Name of merchant is outside the text box

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.56.18

Reproducible in staging?: Yes

Reproducible in production?: No

If this was caught during regression testing, add the test name, ID and link from TestRail:

Email or phone of affected tester (no customers):

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6174688_bandicam_2023-08-23_10-52-18-635.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team/ @Ahmed-Abdella

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692718440819559

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01ae35c173f09896f0
  • Upwork Job ID: 1696218949815410688
  • Last Price Increase: 2023-09-04
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Aug 23, 2023
@OSBotify
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@melvin-bot
Copy link

melvin-bot bot commented Aug 23, 2023

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

@Pujan92
Copy link
Contributor

Pujan92 commented Aug 23, 2023

Proposal

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

Merchant name overflows in money request preview

What is the root cause of that problem?

We are not breaking the word currently which seems the cause for it.

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

We can add styles.breakAll or styles.breakWord for the merchat Text.

<Text style={[styles.textLabelSupporting, styles.mb1, styles.lh16]}>{requestMerchant}</Text>

@melvin-bot melvin-bot bot added Daily KSv2 and removed Hourly KSv2 labels Aug 23, 2023
@Ahmed-Abdella
Copy link
Contributor

Ahmed-Abdella commented Aug 23, 2023

@lanitochka17 @stitesExpensify Hi all, I reported this earlier here (https://expensify.slack.com/archives/C049HHMV9SM/p1692718440819559), but it didn't make it to github yet

@situchan
Copy link
Contributor

@lanitochka17 this was first reported by me - #25738 (comment)

@Ahmed-Abdella
Copy link
Contributor

Ahmed-Abdella commented Aug 23, 2023

Important Notice : issue #25738 has nothing to do with this issue this issue was reproducible before fixing #25738, they are compltely two different issues.
The other issue was about throwing an error afte along text with or without spaces, to make sure try a long merchant with spaces it won't break styles.
The case here is about the text overflowing when it is long and have no spaces in it. And number of characters has no effect here any text that take a space longer than the width of the container will overflow

@alphaboss1104
Copy link
Contributor

Proposal

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

Merchant name overflows in money request preview

What is the root cause of that problem?

We are not breaking the word currently which seems the cause for it.

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

We can wrapping the text document in here:

          <View style={[styles.flexRow]}>
+               <View style={[styles.flex1]}>
                    <Text style={[styles.textLabelSupporting, styles.mb1, styles.lh16]}>{requestMerchant}</Text>
+               </View>                      
          </View>

What alternative solutions did you explore? (Optional)

Using numberOfLines and ellipsizeMode properties in the Text component to limit the text and indicate there is more.

@Ahmed-Abdella
Copy link
Contributor

@situchan could you please check again, I reported this more than 24hrs ago. this was before #25738 got fixed. they are different please check again.

@situchan
Copy link
Contributor

@situchan could you please check again, I reported this more than 24hrs ago. this was before #25738 got fixed. they are different please check again.

yeah, you reported first. I just wanted to highlight that this is not regression from #25738 and we're already aware of.

@marcaaron marcaaron removed the DeployBlockerCash This issue or pull request should block deployment label Aug 23, 2023
@marcaaron
Copy link
Contributor

Gonna demote this one. Seems like a regular bug and could not reproduce it on web.

@mountiny
Copy link
Contributor

I think we should include the fix for this issue in this PR #21664 or the PR actually might be fixing it already

@Ahmed-Abdella
Copy link
Contributor

@marcaaron reproduced on Web, just make sure you have no spaces in the long merchant namescreenshot_from_2023-08-22_18-26-49.png

@Ahmed-Abdella
Copy link
Contributor

@mountiny I don't think the PR you mentioned fix this, it is about the description input. This issue is about the Merchant and how it is displayed. The description has no issues here. Please check this I reported this earlier in slack (https://expensify.slack.com/archives/C049HHMV9SM/p1692718440819559?thread_ts=1692718440.819559&cid=C049HHMV9SM)

@mountiny
Copy link
Contributor

We could update the merchant field in the PR the same way as description

@Ahmed-Abdella
Copy link
Contributor

@mountiny I am sure that they are not related, if you check the video above you will find the description doesn't overflow here and the PR is not merged yet. The description is working fine. The PR fixing the description input. This issue is how we display the Merchant. Completely different issues.

@Ahmed-Abdella
Copy link
Contributor

We should completly ignore description here, it is displayed correctly. and it should removed from the steps in the report. It has no affect here.

@Ahmed-Abdella
Copy link
Contributor

If we fix it the same way as description it will make the Merchant input accept multi lines, and won't fix how the Merchant displayed

@Ahmed-Abdella
Copy link
Contributor

@mountiny If you have time, Could you please help me on this, I reported this earlier in slack, shouldn't I considered the reporter ?

@mountiny
Copy link
Contributor

Updated you as a reporter, please try to post in less comments

@melvin-bot melvin-bot bot added the Overdue label Aug 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 28, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 28, 2023

Triggered auto assignment to @strepanier03 (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Aug 28, 2023

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

@stitesExpensify
Copy link
Contributor

This can be handled externally. Not overdue.

@melvin-bot melvin-bot bot removed the Overdue label Aug 28, 2023
@Ahmed-Abdella
Copy link
Contributor

@stitesExpensify This issue is not reproducible anymore, It got fixed here #21664 after @mountiny asked the contributer to do so. Will I be eligible for the reporting bonus in that case or not as I am the original reporter for this issue and It got fixed depending on that report

@mollfpr
Copy link
Contributor

mollfpr commented Aug 30, 2023

Yup, it isn't reproducible in the latest DEV.

Screenshot 2023-08-30 at 23 58 52

@stitesExpensify
Copy link
Contributor

I'll leave that up to @mountiny. Would this issue have been fixed without @Ahmed-Abdella report, or was it only fixed because of this issue?

@situchan
Copy link
Contributor

This was planned to fix before this GH is created - #25738 (comment)

@Ahmed-Abdella
Copy link
Contributor

@situchan
Copy link
Contributor

yup agree based on #22513 (comment)
But not agree based on #22513 (comment)

@Ahmed-Abdella
Copy link
Contributor

@situchan could you please clarify this, the other comment you mentioned has nothing to do with this issue

@situchan
Copy link
Contributor

I just took example.
In that issue, I didn't get compensation though reported earlier because it didn't contribute to fixing the issue.
But it's edge case and depend on BZ members' decision.
Usually, I have seen so far those cases are still eligible for reporting bonus.

@mountiny
Copy link
Contributor

I think we can pay the rpeorting bonus here, I have asked the contributor to include the fix in existing PR.

@melvin-bot melvin-bot bot added the Overdue label Sep 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 4, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot
Copy link

melvin-bot bot commented Sep 5, 2023

@strepanier03, @mollfpr, @stitesExpensify Huh... This is 4 days overdue. Who can take care of this?

@stitesExpensify
Copy link
Contributor

@strepanier03 can we please pay @Ahmed-Abdella for reporting this, and then we can close it out?

@melvin-bot melvin-bot bot removed the Overdue label Sep 5, 2023
@strepanier03
Copy link
Contributor

@stitesExpensify - Sure thing, I'll work on that now.

@strepanier03
Copy link
Contributor

@Ahmed-Abdella - I have hired you for the job in Upwork, please confirm and let me know so I can pay you out.

@Ahmed-Abdella
Copy link
Contributor

@strepanier03 I accepted the offer, Thanks!

@strepanier03
Copy link
Contributor

Great, thank you! Working on paying now.

@strepanier03
Copy link
Contributor

Payment summary

  • - Bug Reporter - @Ahmed-Abdella - $250 paid via Upwork
  • - Contributor - N/A
  • - C+ - N/A

@strepanier03 strepanier03 changed the title [$1000] IOU - When the name of the Merchant is too long, text is out of field in IOU report [PAID - Reporting] IOU - When the name of the Merchant is too long, text is out of field in IOU report Sep 5, 2023
@strepanier03
Copy link
Contributor

Closing now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests