App icon indicating copy to clipboard operation
App copied to clipboard

[HOLD for payment 2024-12-17] Thread - Thread title in report details RHP shows the entire message

Open IuliiaHerets opened this issue 1 year ago • 22 comments

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: 9.0.68-2 Reproducible in staging?: Y Reproducible in production?: N If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to any chat.
  3. Send a long message.
  4. Right click on the message > Reply in thread.
  5. Click on the thread header.

Expected Result:

The thread title in report details RHP should have limit for the lines it will display.

Actual Result:

The thread title in report details RHP shows the entire message and user has to scroll down to see the buttons.

Workaround:

Unknown

Platforms:

  • [x] Android: Standalone
  • [x] Android: HybridApp
  • [x] Android: mWeb Chrome
  • [x] iOS: Standalone
  • [x] iOS: HybridApp
  • [x] iOS: mWeb Safari
  • [x] MacOS: Chrome / Safari
  • [x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/b696a2fe-f0d9-414b-ba7a-15b0ee4e7ccd

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @sonialiap

IuliiaHerets avatar Nov 29 '24 10:11 IuliiaHerets

Triggered auto assignment to @sonialiap (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.

melvin-bot[bot] avatar Nov 29 '24 10:11 melvin-bot[bot]

Triggered auto assignment to @mjasikowski (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

melvin-bot[bot] avatar Nov 29 '24 10:11 melvin-bot[bot]

💬 A slack conversation has been started in #expensify-open-source

melvin-bot[bot] avatar Nov 29 '24 10:11 melvin-bot[bot]

:wave: 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.

github-actions[bot] avatar Nov 29 '24 10:11 github-actions[bot]

This seems to come from https://github.com/Expensify/App/pull/53041

We are setting numberOfLinesTitle={0} here, because of that the number lines are not truncated https://github.com/Expensify/App/blob/4206c86c8e2a8b1872f3b1fd1071cdc4d5260a54/src/pages/ReportDetailsPage.tsx#L747

we should set it to 2 to truncate it to 2 lines, or remove the numberOfLinesTitle prop to truncate it to one line

etCoderDysto avatar Nov 29 '24 10:11 etCoderDysto

Raising a fix now

twilight2294 avatar Nov 29 '24 10:11 twilight2294

@twilight2294 are you in Slack?

mountiny avatar Nov 29 '24 10:11 mountiny

@twilight2294 its not clear what the fix is, we should discuss with the design team

mountiny avatar Nov 29 '24 10:11 mountiny

@twilight2294 are you in Slack?

Waiting to get added

twilight2294 avatar Nov 29 '24 10:11 twilight2294

We will revert @twilight2294 and then you can handle this case in the PR again and discuss with the design team the best course of action, no need to rush to some solutions we do not have agreement on 👍

mountiny avatar Nov 29 '24 10:11 mountiny

No problem!, Should I put up a revert PR ?

twilight2294 avatar Nov 29 '24 10:11 twilight2294

Revert PR on the way

mjasikowski avatar Nov 29 '24 11:11 mjasikowski

Revert PR merged, CP request sent

mjasikowski avatar Nov 29 '24 11:11 mjasikowski

@Expensify/design Do you have any thoughts on how to display the thread title in the report details RHP? Specifically should it be truncated, and to how many lines?

For context: In https://github.com/Expensify/App/issues/52836 we were going to display report titles full-length, since rooms and groups have a natural length limit. For threads though the title length is pretty much unlimited, so we probably want to handle it differently.

jjcoffee avatar Dec 02 '24 09:12 jjcoffee

I think we can just pick a reasonable limit, something that might feel similar to the character-enforced limit we use elsewhere. So maybe that should be like 2-3 lines or something?

Are we able to pick an amount of lines and truncate that way?

shawnborton avatar Dec 02 '24 13:12 shawnborton

I agree with Shawn—2-3 lines seems totally reasonable to me. If we can't pick the amount of lines, I bet we can estimate a character length that gives us something similar.

dannymcclain avatar Dec 02 '24 14:12 dannymcclain

Thanks for the feedback, guys! We can indeed just choose the number of lines and truncate based on that.

@twilight2294 Could you proceed with 2 lines and we'll see how that looks?

jjcoffee avatar Dec 02 '24 14:12 jjcoffee

I will have a PR up tomorrow, sorry missed this today

twilight2294 avatar Dec 03 '24 18:12 twilight2294

@jjcoffee just to be sure, I have a draft up, can you please check if that is what we need?

twilight2294 avatar Dec 04 '24 12:12 twilight2294

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

melvin-bot[bot] avatar Dec 10 '24 22:12 melvin-bot[bot]

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

  • https://github.com/Expensify/App/pull/53553

If no regressions arise, payment will be issued on 2024-12-17. :confetti_ball:

melvin-bot[bot] avatar Dec 10 '24 22:12 melvin-bot[bot]

@mjasikowski @sonialiap @mjasikowski The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

melvin-bot[bot] avatar Dec 10 '24 22:12 melvin-bot[bot]

I'm OOO Dec 16-20, adding a leave buddy

next steps: @joekaufmanexpensify adding you directly since this cropped up from PR for https://github.com/Expensify/App/issues/52836. I don't believe a payment will be needed here because the original issue hasn't been paid but adding a buddy just in case

sonialiap avatar Dec 13 '24 11:12 sonialiap

Correct, this was a regression from my issue. No payment will be needed here.

joekaufmanexpensify avatar Dec 13 '24 13:12 joekaufmanexpensify

I actually think we can go ahead and close this one as it was a regression of https://github.com/Expensify/App/issues/52836 so no payment needed here. Will handle payment for the original issue there!

joekaufmanexpensify avatar Dec 16 '24 15:12 joekaufmanexpensify