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] [$125] Android - Room - Room name displayed truncated on header and details when using long name #52836

Open
2 of 8 tasks
lanitochka17 opened this issue Nov 20, 2024 · 57 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Nov 20, 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.64-4
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

  1. Open the Expensify app.
  2. Tap on FAB and select "Create Chat"
  3. Tap on "Room"
  4. Create a long name for the room (More than 50 characters) and save it
  5. Verify that the name is displayed completely in header and on room details page

Expected Result:

Room name should be displayed in full in header and on details page, despite using a name with more than 50 characters

Actual Result:

Room name in header and in details is truncated when creating a long room name. (More than 50 characters)

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence
Bug6653366_1730580575095.Truncated.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021859338322732202800
  • Upwork Job ID: 1859338322732202800
  • Last Price Increase: 2024-11-20
  • Automatic offers:
    • twilight2294 | Contributor | 105029064
Issue OwnerCurrent Issue Owner: @joekaufmanexpensify
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 20, 2024
Copy link

melvin-bot bot commented Nov 20, 2024

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

@jacobkim9881
Copy link
Contributor

Proposal

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

Room title line is broken on room details page.

What is the root cause of that problem?

title={StringUtils.lineBreaksToSpaces(reportName)}

The line for room title is broken here.

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

We can use reportName for StringUtils.lineBreaksToSpaces(reportName)

What alternative solutions did you explore? (Optional)

N/A

@joekaufmanexpensify
Copy link
Contributor

It is expected that we truncate in the header, but we are supposed to show the full name in room details.

@joekaufmanexpensify joekaufmanexpensify added the External Added to denote the issue can be worked on by a contributor label Nov 20, 2024
@melvin-bot melvin-bot bot changed the title Android - Room - Room name displayed truncated on header and details when using long name [$250] Android - Room - Room name displayed truncated on header and details when using long name Nov 20, 2024
Copy link

melvin-bot bot commented Nov 20, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 20, 2024
Copy link

melvin-bot bot commented Nov 20, 2024

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

@joekaufmanexpensify joekaufmanexpensify moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Nov 20, 2024
@joekaufmanexpensify
Copy link
Contributor

This is a pretty minor bug, so demoting

@joekaufmanexpensify joekaufmanexpensify changed the title [$250] Android - Room - Room name displayed truncated on header and details when using long name [$125] Android - Room - Room name displayed truncated on header and details when using long name Nov 20, 2024
Copy link

melvin-bot bot commented Nov 20, 2024

Upwork job price has been updated to $125

Copy link
Contributor

@Shahidullah-Muffakir Your proposal will be dismissed because you did not follow the proposal template.

@Shahidullah-Muffakir
Copy link
Contributor

Shahidullah-Muffakir commented Nov 20, 2024

Proposal

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

Room name is truncated in Room Details page.

What is the root cause of that problem?

Improvement
Now the default numberOfLinesTitle 1 is applied

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

<MenuItemWithTopDescription

here pass a new prop for numberOfLinesTitle,
like:
numberOfLinesTitle={5}

What alternative solutions did you explore? (Optional)

@twilight2294
Copy link
Contributor

twilight2294 commented Nov 20, 2024

Edited by proposal-police: This proposal was edited at 2024-11-21 11:29:27 UTC.

Proposal

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

Room name displayed truncated on header and details when using long name

What is the root cause of that problem?

In MenuItemWithTopDescription, we do not pass value for numberOfLinesTitle, so it will default to 1:

<MenuItemWithTopDescription
shouldShowRightIcon={!shouldDisableRename}

numberOfLinesTitle = 1,

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

If we do not want to truncate the room name at all we should pass numberOfLinesTitle={0} in MenuItemWithTopDescription, this makes sure that there is no hardcoded limit on the number of lines the title can have.

We would also need to update the combinedTitleTextStyle in MenuItem, we need to break the line if the word doesn't fit the given line:

const combinedTitleTextStyle = StyleUtils.combineStyles(

    const combinedTitleTextStyle = StyleUtils.combineStyles(
        [
.........
            styles.breakWord,
        ],
        titleStyle ?? {},
    );
    

We did the same thing in previous PR's as well:

What alternative solutions did you explore? (Optional)

@NJ-2020

This comment was marked as off-topic.

@jjcoffee
Copy link
Contributor

Let's go with @twilight2294's proposal. We don't want any truncation so it makes sense to make the numberOfLinesTitle unlimited.

The first proposal has the correct RCA, but adds a limit on the numberOfLinesTitle. The last proposal refers to a part of the code that doesn't relate to the room title.

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Nov 21, 2024

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

@Shahidullah-Muffakir
Copy link
Contributor

Let's go with @twilight2294's proposal. We don't want any truncation so it makes sense to make the numberOfLinesTitle unlimited.

The first proposal has the correct RCA, but adds a limit on the numberOfLinesTitle. The last proposal refers to a part of the code that doesn't relate to the room title.

🎀👀🎀 C+ reviewed

@jjcoffee I don't think passing numberOfLinesTitle={0} will work, can we test it please?
and as I mentioned, I gave 5 as an example we can increase the number of lines.

@jjcoffee
Copy link
Contributor

@Shahidullah-Muffakir I have tested it. You can also check out the docs to see that 0 is supported.

@Shahidullah-Muffakir
Copy link
Contributor

Shahidullah-Muffakir commented Nov 21, 2024

@jjcoffee , maybe I am missing something because when I pass numberOfLinesTitle={0} , this is the output, where the complete name of the room is not shown

Screenshot 2024-11-21 at 3 16 21 PM

@twilight2294
Copy link
Contributor

Screenshot 2024-11-21 at 3 36 04 PM

@Shahidullah-Muffakir
Copy link
Contributor

Screen.20Recording.202024-11-21.20at.203.mp4

@Shahidullah-Muffakir
Copy link
Contributor

Room name has a character limit of 100. Considering this, setting numberOfLinesTitle={5} should be sufficient.

ErrorUtils.addErrorMessage(errors, 'roomName', translate('common.error.characterLimitExceedCounter', {length: values.roomName.length, limit: CONST.TITLE_CHARACTER_LIMIT}));

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Nov 24, 2024
@twilight2294
Copy link
Contributor

PR ready for review @jjcoffee

@joekaufmanexpensify
Copy link
Contributor

PR in review

Copy link

melvin-bot bot commented Nov 29, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 30, 2024
@melvin-bot melvin-bot bot changed the title [$125] Android - Room - Room name displayed truncated on header and details when using long name [HOLD for payment 2024-12-07] [$125] Android - Room - Room name displayed truncated on header and details when using long name Nov 30, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Nov 30, 2024
Copy link

melvin-bot bot commented Nov 30, 2024

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

Copy link

melvin-bot bot commented Nov 30, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.68-7 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-07. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Nov 30, 2024

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

@joekaufmanexpensify
Copy link
Contributor

I see we reverted this PR and are working on a fix in #53308 . I am thinking we'll handle payment once the final PR is out. Sound good @twilight2294 @jjcoffee ?

@garrettmknight garrettmknight moved this from Bugs and Follow Up Issues to Hold for Payment in [#whatsnext] #expense Dec 3, 2024
@joekaufmanexpensify joekaufmanexpensify changed the title [HOLD for payment 2024-12-07] [$125] Android - Room - Room name displayed truncated on header and details when using long name [$125] Android - Room - Room name displayed truncated on header and details when using long name Dec 6, 2024
@joekaufmanexpensify
Copy link
Contributor

Removed payment date, as new PR is not out to prod yet.

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Dec 7, 2024
@joekaufmanexpensify
Copy link
Contributor

This is pending #53553 , which is merged but not yet out on staging.

@melvin-bot melvin-bot bot removed the Overdue label Dec 9, 2024
@melvin-bot melvin-bot bot removed the Overdue label Dec 9, 2024
@joekaufmanexpensify
Copy link
Contributor

Second PR deployed last week. We'll handle payment here and it is due tomorrow.

@joekaufmanexpensify joekaufmanexpensify changed the title [$125] Android - Room - Room name displayed truncated on header and details when using long name [hold for payment 2024-12-17] [$125] Android - Room - Room name displayed truncated on header and details when using long name Dec 16, 2024
@joekaufmanexpensify
Copy link
Contributor

We originally worked on this bug in #53041 and then reverted it because it caused a regression. We then fixed the bug again in #53553. 50% regression penalty therefore applies. Payment here is as follows:

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. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
Status: Hold for Payment
Development

No branches or pull requests

10 participants