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

[Due for payment 2025-03-05] [$250] Company card name is cut off at the end. #56716

Open
3 of 8 tasks
m-natarajan opened this issue Feb 12, 2025 · 28 comments
Open
3 of 8 tasks
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 Needs Reproduction Reproducible steps needed Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Feb 12, 2025

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:
Reproducible in staging?: needs reproduction
Reproducible in production?: needs reproduction
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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: @hungvu193
Slack conversation (hyperlinked to channel name): expensify-bugs

Action Performed:

  1. Go to Workspace > Company Card > Add a card that has long card name.
  2. Observe the card name on mobile device screens.

Expected Result:

The card name shouldn't be cut off

Actual Result:

The card name is cut off

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
Screen.Recording.2025-02-11.at.22.36.32.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021890070956830857932
  • Upwork Job ID: 1890070956830857932
  • Last Price Increase: 2025-02-20
  • Automatic offers:
    • allgandalf | Reviewer | 106259875
Issue OwnerCurrent Issue Owner: @
Issue OwnerCurrent Issue Owner: @alexpensify
@m-natarajan m-natarajan added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Needs Reproduction Reproducible steps needed labels Feb 12, 2025
Copy link

melvin-bot bot commented Feb 12, 2025

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

@MelvinBot
Copy link

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

@m-natarajan
Copy link
Author

Proposal by @hungvu193

Proposal

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

Company card name is cut off at the end.

What is the root cause of that problem?

We're using flex row here:

style={[styles.flexRow, styles.alignItemsCenter, styles.gap3, shouldChangeLayout && styles.mb3]}

But we forgot to add the style for the View here:

This makes the CaretWrapper's text overflow and cut off.

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

  1. Firstly we should add styles.flex1 here
  2. Add styles.flex1 to the CaretWrapper here to make sure it will only fill its space and won't overflow the rest of the layout. We can add style props to CaretWrapper and pass styles.flex1 as style.
  3. Finally add styles.flexShrink1 to the Text here to make sure it won't overflow when the name of the card is too long. (We can also use styles.flex1, but I prefer flexShrink because it will keep the old behavior of the text.

We should also check other places in the App that used CaretWrapper and then apply the same solution above if needed.

(Noting that the above solution fixes the RCA of the issue, but still might need additional changes to match the expected design, ie adding styles.alignItemsCenter here .)

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

What alternative solutions did you explore? (Optional)

N/A

@allgandalf
Copy link
Contributor

allgandalf commented Feb 12, 2025

Go to Workspace > Company Card > Add a card that has long card name.
Observe the card name on mobile device screens.

Image

@alexpensify Please assign me here as C+ as per:

The first C+ to post on the GH issue with reliable reproduction steps and video or screenshots confirming reproduction will be assigned as C+.

@alexpensify
Copy link
Contributor

@allgandalf - Let's confirm if this proposal will fix this issue. Thanks!

@allgandalf
Copy link
Contributor

@alexpensify can you put up external label here? i guess we need that to get the automation to kick in

@alexpensify alexpensify added the External Added to denote the issue can be worked on by a contributor label Feb 13, 2025
@melvin-bot melvin-bot bot changed the title Company card name is cut off at the end. [$250] Company card name is cut off at the end. Feb 13, 2025
Copy link

melvin-bot bot commented Feb 13, 2025

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 13, 2025
Copy link

melvin-bot bot commented Feb 13, 2025

Current assignee @allgandalf is eligible for the External assigner, not assigning anyone new.

@alexpensify
Copy link
Contributor

@allgandalf we should be all set now. Thanks for your help here.

@huult
Copy link
Contributor

huult commented Feb 14, 2025

Proposal

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

Company card name is cut off at the end.

What is the root cause of that problem?

We use flexRow to wrap the text.

style={[styles.flexRow, styles.alignItemsCenter, styles.gap3, shouldChangeLayout && styles.mb3]}

And the View here doesn’t have flex: 1, and CaretWrapper doesn’t have flex: 1 either, which causes the text to be cut off: Without flex: 1, the container doesn’t expand to fill available space, causing the text to be cut off when there’s insufficient room.

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

To fix this issue, we need to add a new style as outlined in the steps below:

  1. Add wrapperStyle={[styles.flex1, !shouldUseNarrowLayout && styles.pr1]} here so that on desktop screens with a long bank name, it will prevent the Assign card and Settings buttons from being pushed out of line, as shown in the image below:
Image

accessibilityLabel={formattedFeedName ?? ''}

    <PressableWithFeedback
        onPress={() => Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS_SELECT_FEED.getRoute(policyID))}
        style={[styles.flexRow, styles.alignItemsCenter, styles.gap3, shouldChangeLayout && styles.mb3]}
        accessibilityLabel={formattedFeedName ?? ''}
        wrapperStyle={[styles.flex1, !isLargeScreenWidth && styles.pr1]}
    >

!isLargeScreenWidth && styles.pr1 to make space with assign card and setting button

2 Add flex: 1 here to makes the expand to fill available space in its parent container.

update to:

    <View style={styles.flex1}>
  1. Similarly to the View above, we need to add flex: 1 to CaretWrapper. For this component, we can create new props to avoid impacting other instances where CaretWrapper is used

    <CaretWrapper customStyle={styles.flex1}>

<View style={[styles.flex1, styles.flexRow, styles.gap1, styles.alignItemsCenter]}>

    <View style={[styles.flexRow, styles.gap1, styles.alignItemsCenter, customStyle]}>

4.flexShrink1: 1 is added here to prevent the component from growing beyond its natural size in a flex container(in case bank name short).

<Text style={styles.textStrong}>{formattedFeedName}</Text>

    <Text style={[styles.flexShrink1, styles.textStrong]}>

Test branch

Screen.Recording.2025-02-14.at.15.38.36.mp4

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

None

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

If we cut the text and add ..., then we use the numberOfLines prop for the Text

@melvin-bot melvin-bot bot added the Overdue label Feb 17, 2025
Copy link

melvin-bot bot commented Feb 17, 2025

@alexpensify, @allgandalf Whoops! This issue is 2 days overdue. Let's get this updated quick!

@allgandalf
Copy link
Contributor

reviewing the proposals today

@melvin-bot melvin-bot bot removed the Overdue label Feb 17, 2025
@allgandalf
Copy link
Contributor

@hungvu193 's proposal LGTM

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Feb 19, 2025

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

@huult
Copy link
Contributor

huult commented Feb 19, 2025

@allgandalf The proposal you selected causes the bug and is not sufficient to fix this issue. Could you review my proposal?

@huult
Copy link
Contributor

huult commented Feb 20, 2025

@allgandalf

The proposal you selected causes the bug and is not sufficient to fix this issue.

Here is the issue I mentioned. My proposal fixes it and covers all cases and platforms.

Image

@hungvu193
Copy link
Contributor

@allgandalf

The proposal you selected causes the bug and is not sufficient to fix this issue.

Here is the issue I mentioned. My proposal fixes it and covers all cases and platforms.

Image

The only different between your proposal and mine is styles.pr1 on the header right?

@huult
Copy link
Contributor

huult commented Feb 20, 2025

wrapperStyle={[styles.flex1, !isLargeScreenWidth && styles.pr1]} Here is everything we need. If not, it may cause a bug.

@allgandalf
Copy link
Contributor

@huult I think proposal from @hungvu193 was the first to point out RCA and outline a solution, these points are generally covered during PR phase, and imo, proposals need to outline a basic idea of the changes required and details can be looked at during the PR phase .

Thanks for understanding 😄

@hungvu193
Copy link
Contributor

wrapperStyle={[styles.flex1, !isLargeScreenWidth && styles.pr1]} Here is everything we need. If not, it may cause a bug.

In fact, that case will never happen, I believe you inspected the element to change the title, the card name has 100 characters limit.

I also note in my proposal that, additional changes might be needed and my proposal only fixed the RCA of the issue.

@huult
Copy link
Contributor

huult commented Feb 20, 2025

Because I saw this case was the same as this ticket (#55218), I pointed it out, identified the correct RCA, and resolved the issue. However, another contributor proposed a different case, and their proposal was selected. I feel it is the same as this case, so I am confused and asking for clarification.

Anyway, I agree with @allgandalf. Thanks, @hungvu193

Copy link

melvin-bot bot commented Feb 20, 2025

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Feb 24, 2025
@allgandalf
Copy link
Contributor

not overdue, pending assignment, deeds here

@melvin-bot melvin-bot bot removed the Overdue label Feb 24, 2025
@marcochavezf
Copy link
Contributor

Thanks for the collaboration guys, and apologies for the delay here. I agree with @allgandalf's decision, assigning @hungvu193

Also thanks @huult for bringing up the clarification, I'm confident you will find success in the next issue 🚀

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 24, 2025
Copy link

melvin-bot bot commented Feb 24, 2025

📣 @allgandalf 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

@hungvu193 hungvu193 mentioned this issue Feb 25, 2025
50 tasks
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 Weekly KSv2 labels Feb 25, 2025
@melvin-bot melvin-bot bot changed the title [$250] Company card name is cut off at the end. [Due for payment 2025-03-05] [$250] Company card name is cut off at the end. Feb 26, 2025
Copy link

melvin-bot bot commented Feb 26, 2025

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Feb 26, 2025
Copy link

melvin-bot bot commented Feb 26, 2025

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.6-1 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 2025-03-05. 🎊

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

Copy link

melvin-bot bot commented Feb 26, 2025

@hungvu193 / @allgandalf @alexpensify @hungvu193 / @allgandalf 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]

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 Needs Reproduction Reproducible steps needed Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants