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

[$500] IOU Report preview with links are broken #36339

Closed
1 of 6 tasks
m-natarajan opened this issue Feb 12, 2024 · 14 comments
Closed
1 of 6 tasks

[$500] IOU Report preview with links are broken #36339

m-natarajan opened this issue Feb 12, 2024 · 14 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 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

@m-natarajan
Copy link

m-natarajan commented Feb 12, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 1.4.40-0
Reproducible in staging?: y
Reproducible in production?: y
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
Expensify/Expensify Issue URL:
Issue reported by: @Santhosh-Sellavel
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707600516151779

Action Performed:

Break down in numbered steps

Expected Result:

The preview background covers all content or all content displayed within the preview area

Actual Result:

The description link goes out to the preview background

Workaround:

unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
Screenshot 2024-02-11 at 2 47 04 AM
Screenshot 2024-02-11 at 2 57 09 AM
Screen Shot 2024-02-12 at 9 37 11 AM

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~011c26b958d064c598
  • Upwork Job ID: 1757052575231959040
  • Last Price Increase: 2024-02-12
  • Automatic offers:
    • ntdiary | Reviewer | 0
    • jjcoffee | Contributor | 0
@m-natarajan m-natarajan added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 12, 2024
Copy link

melvin-bot bot commented Feb 12, 2024

Job added to Upwork: https://www.upwork.com/jobs/~011c26b958d064c598

@melvin-bot melvin-bot bot changed the title IOU Report preview with links are broken [$500] IOU Report preview with links are broken Feb 12, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 12, 2024
Copy link

melvin-bot bot commented Feb 12, 2024

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

Copy link

melvin-bot bot commented Feb 12, 2024

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

@abzokhattab
Copy link
Contributor

abzokhattab commented Feb 12, 2024

Proposal

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

The preview background covers all content or all content displayed within the preview area

What is the root cause of that problem?

there is an inconsistency between the description and the merchant in the preview

where we use RenderHTML for the description and Text component for the merchant

)}
{shouldShowDescription && <RenderHTML html={parser.replace(merchantOrDescription)} />}
{shouldShowMerchant && <Text style={[styles.textLabelSupporting, styles.textNormal]}>{merchantOrDescription}</Text>}
</View>
{isBillSplit && participantAccountIDs.length > 0 && requestAmount && requestAmount > 0 && (
<Text style={[styles.textLabel, styles.colorMuted, styles.ml1, styles.amountSplitPadding]}>

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

we need to unify them to use Text component for both:

                                    {shouldShowDescription && <Text style={[styles.textLabelSupporting, styles.textNormal]}>{shouldShowDescription}</Text>}
                                    {shouldShowMerchant && <Text style={[styles.textLabelSupporting, styles.textNormal]}>{merchantOrDescription}</Text>}

Or we can keep the RenderHTML and add styles.breakWord which prevents the text overflow :

 <View style={styles.breakWord}> <RenderHTML html={parser.replace(merchantOrDescription)} /> </View> 

@neonbhai
Copy link
Contributor

neonbhai commented Feb 12, 2024

Proposal

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

IOU Report preview with links are broken

What is the root cause of that problem?

We are missing the basic styles for RenderHTML styles.renderHTML here:

{shouldShowDescription && <RenderHTML html={parser.replace(merchantOrDescription)} />}

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

We should wrap RenderHTML with a view containing styles.renderHTML here:

<View style={styles.renderHTML}> 
    <RenderHTML html={parser.replace(merchantOrDescription)} /> 
</View>

Result

Before:

Screenshot 2024-02-13 at 4 39 39 AM

After:

Screenshot 2024-02-13 at 4 39 26 AM

@jjcoffee
Copy link
Contributor

jjcoffee commented Feb 12, 2024

@lschurr FYI @ntdiary is OOO until 18th Feb. I'd be happy to take this over in the meantime.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 12, 2024
Copy link

melvin-bot bot commented Feb 12, 2024

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

Offer link
Upwork job

Copy link

melvin-bot bot commented Feb 12, 2024

📣 @jjcoffee 🎉 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 📖

@lschurr lschurr added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 12, 2024
@lschurr
Copy link
Contributor

lschurr commented Feb 12, 2024

Great, assigned to you @jjcoffee

@jjcoffee
Copy link
Contributor

This is a tight one! But it looks like @neonbhai's proposal got in there first (based on the edit timestamps) with the idea to wrap with a View with the relevant word-break styles to stop overflowing.

We don't want to switch to using a Text component (as suggested in @abzokhattab's proposal) instead as we need to support HTML here.

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Feb 13, 2024

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

@mkhutornyi
Copy link
Contributor

Sorry for late. This is dupe of #34952

@jjcoffee
Copy link
Contributor

@mkhutornyi No worries, thanks for flagging! @lschurr looks like we can close this!

@lschurr
Copy link
Contributor

lschurr commented Feb 13, 2024

Great, thanks!

@lschurr lschurr closed this as completed Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 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

8 participants