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

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

Open
8 tasks done
IuliiaHerets opened this issue Nov 29, 2024 · 24 comments · Fixed by #53311
Open
8 tasks done
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering Weekly KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Nov 29, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com 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): applausetester+232803@applause.expensifail.com
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:

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

Screenshots/Videos

Bug6679791_1732870553838.20241129_165149.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @sonialiap
@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. labels Nov 29, 2024
Copy link

melvin-bot bot commented Nov 29, 2024

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.

Copy link

melvin-bot bot commented Nov 29, 2024

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

@melvin-bot melvin-bot bot added the Daily KSv2 label Nov 29, 2024
Copy link

melvin-bot bot commented Nov 29, 2024

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

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Nov 29, 2024
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.

@etCoderDysto
Copy link
Contributor

etCoderDysto commented Nov 29, 2024

This seems to come from #53041

We are setting numberOfLinesTitle={0} here, because of that the number lines are not truncated

numberOfLinesTitle={0}

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

@twilight2294
Copy link
Contributor

Raising a fix now

@mountiny
Copy link
Contributor

@twilight2294 are you in Slack?

@mountiny
Copy link
Contributor

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

@twilight2294
Copy link
Contributor

@twilight2294 are you in Slack?

Waiting to get added

@mountiny
Copy link
Contributor

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 👍

@twilight2294
Copy link
Contributor

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

@mjasikowski
Copy link
Contributor

Revert PR on the way

@mjasikowski
Copy link
Contributor

Revert PR merged, CP request sent

@mountiny mountiny removed the DeployBlockerCash This issue or pull request should block deployment label Nov 29, 2024
@jjcoffee
Copy link
Contributor

jjcoffee commented Dec 2, 2024

@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 #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.

@shawnborton
Copy link
Contributor

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?

@dannymcclain
Copy link
Contributor

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.

@jjcoffee
Copy link
Contributor

jjcoffee commented Dec 2, 2024

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?

@twilight2294
Copy link
Contributor

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

@twilight2294
Copy link
Contributor

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

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 5, 2024
@melvin-bot melvin-bot bot changed the title Thread - Thread title in report details RHP shows the entire message [HOLD for payment 2024-12-17] Thread - Thread title in report details RHP shows the entire message Dec 10, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Dec 10, 2024
Copy link

melvin-bot bot commented Dec 10, 2024

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

Copy link

melvin-bot bot commented Dec 10, 2024

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

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

Copy link

melvin-bot bot commented Dec 10, 2024

@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]

@sonialiap
Copy link
Contributor

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

next steps:
@joekaufmanexpensify adding you directly since this cropped up from PR for #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

@joekaufmanexpensify
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering Weekly KSv2
Projects
None yet
10 participants