[HOLD for payment 2022-10-03] [$250] Ellipsis is not showing up in medium screen width - reported by @Tushu17

[mvtglobally]

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

---

Action Performed:

1. Open website in desktop.
2. resize the window to single page.
3. Go to chat > Send a long message.
4. Go to Homepage

Expected Result:

It should show ellipsis for long message.

Actual Result:

It doesn't show ellipsis.

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.1.88-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2022-08-03.at.6.38.07.PM.mov

Expensify/Expensify Issue URL:
Issue reported by: @Tushu17
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1659542182865039

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[melvin-bot]

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

[melvin-bot]

@robertjchen Eep! 4 days overdue now. Issues have feelings too...

[SofiedeVreese]

Should we reassign this one, given you'll be on Sabbatical soon?

[robertjchen]

Thanks for the report- I was able to reproduce this issue locally. Going to open this up for our contributors to tackle 👍

[melvin-bot]

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

[MitchExpensify]

Upwork Job

[melvin-bot]

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

[melvin-bot]

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

[christianwen]

Proposal

Problem

The issue is from the src/CONST.js where we're setting the LAST_MESSAGE_TEXT_MAX_LENGTH as 80 characters. That number is too low because when the app is in tablet mode (medium screen width as mentioned in the issue report), the width that the last message text occupies is around 700px. The width needed to render the last message (which is 80 characters maximum) is far lower than 700px, so to React Native, the truncated last message text totally fit in the Text component width, thus it doesn't append the ellipsis.

Solution

There're 2 solutions that I propose, it's dependent on whether we still want to keep the 'truncated last message text' optimization or not:
(The code below is only in the front-end code, I'll need help from someone from the internal team to modify the back-end code as well since I don't have access to the repo, the logic should be similar)

1. We keep the last message text length as is without truncating it, when we save it to Onyx (and subsequently to our database)

In src/libs/ReportUtils.js

function formatReportLastMessageText(lastMessageText) {
-    return String(lastMessageText).substring(0, CONST.REPORT.LAST_MESSAGE_TEXT_MAX_LENGTH);
+    return String(lastMessageText);
}

or we can remove the formatReportLastMessageText and its usage entirely if we don't need the encapsulation, because it no longer contains any custom logic

2. We increase the LAST_MESSAGE_TEXT_MAX_LENGTH
I don't really prefer this approach because it's really difficult to estimate how much would be the maximum length that the last message text will occupy. It depends on multiple parameters like the characters in the message (i and W has different width although both count as one character), the device that the user is using, custom font size, different languages, ...

We should only consider this if we really want to keep the max last message size (for optimization or some other reasons).
Based on my quick calculation the max length at around 200 characters should be good for most cases.

In src/CONST.js:

-LAST_MESSAGE_TEXT_MAX_LENGTH: 80,
+LAST_MESSAGE_TEXT_MAX_LENGTH: 200,

Issue fixed when implementing each of the above and set the network to offline (because back-end fix is also required so I turn off network to test first)

Please let me know if you have any concerns regarding the above proposal
Thanks!

[rushatgabhane]

But 200 characters is a quite long text that will exceed the 800px breakpoint that we have for tablet, so it will still have the ellipsis

So we should be safe in all usual cases, unless you're trying something weird like modifying the font size to be very very small, then this solution will fail

okayy, I can get onboard with that

[ctkochan22]

@rushatgabhane I appreciate your thoroughness in thinking through the proposal

[ctkochan22]

Lets go with solution 2. @christianwen Can you write up the PR and test on all platforms please?

[christianwen]

@ctkochan22 I'm preparing the PR, I've also applied for the Upwork job, please help accept my proposal on Upwork. My Upwork profile is https://www.upwork.com/freelancers/~01991fd5e5c11ef3ba
Thanks

[ctkochan22]

@MitchExpensify

[melvin-bot]

@ctkochan22, @rushatgabhane, @MitchExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[ctkochan22]

@MitchExpensify can we hire @christianwen pretty please

[MitchExpensify]

Sorry for the delay, catching up from OOO last week! Hired @christianwen, @Tushu17, and invite sent to @rushatgabhane for eventual C+ payment

[christianwen]

thanks @MitchExpensify
@ctkochan22 PR created here #11013 and requested you for review.
Also as mentioned in #10510 (comment), a back-end fix for the same constant is also required so that it will work in online mode (currently we need to turn off network to test the PR).

Please help with this since I don't access to the back-end repo.
Thanks!

[rushatgabhane]

PR seems okay, we should wait for backend changes before merging?
@ctkochan22

[melvin-bot]

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

• fix: 10510 ellipsis is not showing up in medium screen width #11013

If no regressions arise, payment will be issued on 2022-10-03. 🎊

[rushatgabhane]

@laurenreidexpensify i believe we can settle this issue?

[MitchExpensify]

Issuing payment now!

[MitchExpensify]

Just waiting on you to accept @rushatgabhane before closing this out, thanks everyone!

[MitchExpensify]

Done!