-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[HOLD for payment March 4th - $500] Expense description isn’t truncated, runs outside the expense preview #34952
Comments
Triggered auto assignment to @slafortune ( |
Job added to Upwork: https://www.upwork.com/jobs/~0149fde024360524d1 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan ( |
Looks like a regression from #34010 |
This comment was marked as outdated.
This comment was marked as outdated.
📣 @Shahidullah-Muffakir! 📣
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
📣 @noumantahir! 📣
|
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Expense description isn’t truncated, runs outside the expense preview What is the root cause of that problem?
Here, we're rendering html but there's no text wrapping logic like others: Lines 1776 to 1778 in 9c552ab
What changes do you think we should make in order to solve the problem?wrap RenderHTML with View with style: Additionally, we can set maxWidth: '100%', whiteSpace if needed |
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
@noumantahir thanks for the proposal but the root cause is not correct. The solution also doesn't make sense. |
ProposalPlease re-state the problem that we are trying to solve in this issue.Expense description isn’t truncated, runs outside the expense preview What is the root cause of that problem?The parent view of
What changes do you think we should make in order to solve the problem?This issue will occur whenever we use
Result![]() AlternativeWe can directly apply styles to the components that overflow, like in this case link. Line 105 in 1b1eeb8
|
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@mkhutornyi's proposal looks good to me. @Krishna2323 your proposal is variation of previous one and I don't think we should alter BaseHTMLEngineProvider. This is widely used and already working fine in other places of the app for years. 🎀 👀 🎀 C+ reviewed |
Triggered auto assignment to @NikkiWines, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@situchan, it works because we wrap and style the parent view with word break styles, although its a variation but it will make sure we don't face this issue next time. Like we recently did because of using |
@slafortune, @NikkiWines, @situchan Eep! 4 days overdue now. Issues have feelings too... |
@mkhutornyi will you be able to more this forward in the next day or two? |
yes once assigned. I am not assigned yet |
Yep, sorry for the delay. Agreed that @mkhutornyi's proposal looks good 👍 |
📣 @situchan 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @mkhutornyi 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@slafortune @NikkiWines @mkhutornyi @situchan this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
@mkhutornyi what's the latest on the PR for this issue? |
PR will be ready for review by tomorrow |
Looks like the automation didn't work - I've updated the title
|
Regression Test Proposal
|
Test rail done and paid 👍 |
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.30-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: @JmillsExpensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1705924528924469
Action Performed:
Expected Result:
The link should not extend outside the expense preview
Actual Result:
The link isn’t truncated and extends outside the expense preview
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: