App icon indicating copy to clipboard operation
App copied to clipboard

[HOLD for payment 2024-12-12] [$2000] Android - Title is not rendered fully in the available space.

Open kbecciv opened this issue 2 years ago • 204 comments

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 the app on Android.
  2. See the chats in the LHN.
  3. Notice a few reports will longer title.

Expected Result:

Title is clipped when reached the right edge of the screen.

Actual Result:

Title is clipped with ellipsis in the middle of screen and there is more available room.

Workaround:

Unknown

Platforms:

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

  • [x] Android / native
  • [ ] Android / Chrome
  • [ ] iOS / native
  • [ ] iOS / Safari
  • [ ] MacOS / Chrome / Safari
  • [ ] MacOS / Desktop

Version Number: 1.3.27-2

Reproducible in staging?: y

Reproducible in production?: y

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

Notes/Photos/Videos: Any additional supporting documentation

Screenshot 2023-06-14 at 6 54 44 PM (1)

Screenshot_20230620_203809_New Expensify

Expensify/Expensify Issue URL:

Issue reported by: @parasharrajat

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686749244980549

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01ef0ae782c96792ac
  • Upwork Job ID: 1671576465794899968
  • Last Price Increase: 2023-11-06
  • Automatic offers:
    • aimane-chnaif | Reviewer | 27596163
    • fabriziobertoglio1987 | Contributor | 27981486
Issue OwnerCurrent Issue Owner: @adelekennedy

kbecciv avatar Jun 21 '23 14:06 kbecciv

Triggered auto assignment to @adelekennedy (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot[bot] avatar Jun 21 '23 14:06 melvin-bot[bot]

Bug0 Triage Checklist (Main S/O)

  • [x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • [x] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • [x] This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • [x] This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

melvin-bot[bot] avatar Jun 21 '23 14:06 melvin-bot[bot]

Proposal

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

Android - Title is not rendered fully in the available space.

What is the root cause of that problem?

The root cause is android use different way to break words, see here and textbreakstrategy explanation here I don't think any textbreakstrategy value will make the text stretch to end of the line, because there is no flex 1 style for text which make Text break instead of stretch in the parent view.

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

remove style optionDisplayNameCompact and add styles.flex1 


const displayNameStyle = StyleUtils.combineStyles([styles.optionDisplayName, styles.pre, styles.flex1, ...textUnreadStyle], props.style);
old proposal

we need change flexGrow: 0, here to be flexGrow: 1, or remove it and use flex: 1 https://github.com/Expensify/App/blob/797298e272bc0867c15dd3e1c76293a0e9009503/src/styles/styles.js#L1401-L1406

before and after.

Screenshot 2023-07-12 at 5 07 24 PM

What alternative solutions did you explore? (Optional)

ahmedGaber93 avatar Jun 21 '23 16:06 ahmedGaber93

yeah - longer chat names are being cut off in the center of the screen with ellipses rather than reaching the border of the screen like we see in IOS

adelekennedy avatar Jun 21 '23 17:06 adelekennedy

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

melvin-bot[bot] avatar Jun 21 '23 17:06 melvin-bot[bot]

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

melvin-bot[bot] avatar Jun 21 '23 17:06 melvin-bot[bot]

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

melvin-bot[bot] avatar Jun 21 '23 17:06 melvin-bot[bot]

Proposal

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

Android - Title is not rendered fully in the available space.

What is the root cause of that problem?

Default Text render startegy for android is highQuality which is causing the issue where it breaks unwantedly the string but we are only showing a single line.

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

We need to provide the prop textBreakStrategy here with textBreakStrategy="simple" which solves the issue. Same we are providing for ResendValidationForm here.

https://github.com/Expensify/App/blob/003e5e563a20459e4252189bd0eaa6a8f60b744c/src/components/DisplayNames/index.native.js#L8-L12

Pujan92 avatar Jun 21 '23 18:06 Pujan92

Proposal

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

Android - Title is not rendered fully in the available space.

What is the root cause of that problem?

It is Android style word breaking IOS work differently that is why we found two diffrent design in the screenshot.

textBreakStrategy="simple" is not the right solution for the following reasons.

  1. textBreakStrategy="simple" use for, not break words when break line
  2. textBreakStrategy="simple" lose the layout quality in Android Read here
  3. it's not working on all devices Read here
  4. In other devices textBreakStrategy="simple" is just removing - at the end and works the same as before

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

I suggest use text.replace(/\s/g, '\xa0') for replacing whitespace by non-breaking unicode space in the variable and it will work fine and it does not lose any layout quality.

We are using different files for web and native so it will not affect our web

https://github.com/Expensify/App/blob/3585d7ba030a05045123ef87ebeccf084db90bbd/src/components/DisplayNames/index.native.js#L6-L15

function DisplayNames(props) {
    return (
        <Text
            accessibilityLabel={props.accessibilityLabel}
            style={props.textStyles}
            numberOfLines={props.numberOfLines}
        >
-           {props.fullTitle}
+            {props.fullTitle.replace(/\s/g, '\xa0')}
        </Text>
    );
}
Result

Screenshot_1687443917

IMG_D674EEB790B1-1

What alternative solutions did you explore? (Optional)

manjesh-yadav avatar Jun 22 '23 14:06 manjesh-yadav

@aimane-chnaif thoughts on the proposal above?

adelekennedy avatar Jun 26 '23 15:06 adelekennedy

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

melvin-bot[bot] avatar Jun 28 '23 16:06 melvin-bot[bot]

@adelekennedy, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] avatar Jun 29 '23 20:06 melvin-bot[bot]

I don't think we need to adjust the bounty just yet - we have proposals above to review

adelekennedy avatar Jun 29 '23 23:06 adelekennedy

@adelekennedy, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] avatar Jul 03 '23 20:07 melvin-bot[bot]

pending proposal review @aimane-chnaif

adelekennedy avatar Jul 05 '23 02:07 adelekennedy

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

melvin-bot[bot] avatar Jul 05 '23 16:07 melvin-bot[bot]

@adelekennedy @aimane-chnaif this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] avatar Jul 05 '23 20:07 melvin-bot[bot]

pending review @aimane-chnaif will you be able to review the above proposals soon?

adelekennedy avatar Jul 07 '23 02:07 adelekennedy

@adelekennedy, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] avatar Jul 10 '23 20:07 melvin-bot[bot]

lil' bump on the above

adelekennedy avatar Jul 10 '23 23:07 adelekennedy

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

melvin-bot[bot] avatar Jul 12 '23 16:07 melvin-bot[bot]

@Pujan92 your textBreakStrategy solution doesn't work for me. I tested on various android devices.

aimane-chnaif avatar Jul 12 '23 16:07 aimane-chnaif

Can everyone update proposals based on latest codebase? No satisfactory proposals yet

aimane-chnaif avatar Jul 12 '23 17:07 aimane-chnaif

@aimane-chnaif Have you tried my proposal? it's working and up to date

manjesh-yadav avatar Jul 12 '23 17:07 manjesh-yadav

@aimane-chnaif Have you tried my proposal? it's working and up to date

I don't like your solution.

aimane-chnaif avatar Jul 12 '23 17:07 aimane-chnaif

Proposal

Updated @aimane-chnaif

ahmedGaber93 avatar Jul 12 '23 18:07 ahmedGaber93

@adelekennedy @aimane-chnaif this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

melvin-bot[bot] avatar Jul 12 '23 20:07 melvin-bot[bot]

Still open for better solution

aimane-chnaif avatar Jul 12 '23 20:07 aimane-chnaif

Proposal

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

Android - Title is not rendered fully in the available space.

What is the root cause of that problem?

Seems the child View with flexRow here not having the full width automatically all the time even the child Text is long when the textStrategy is highQuality. https://github.com/Expensify/App/blob/7490ea6becd0ba87909dd3223428cdde05370ca3/src/components/LHNOptionsList/OptionRowLHN.js#L190

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

As the TextPill is removed now from the row we can take out the extra View which seems not required now. With that the parent View already has contentContainerStyles(flex1) for the full available width. https://github.com/Expensify/App/blob/7490ea6becd0ba87909dd3223428cdde05370ca3/src/components/LHNOptionsList/OptionRowLHN.js#L190

For focus priority mode we want to allow the DisplayName to take width as per the content, to achieve that we need to remove the class styles.optionDisplayNameCompact and add styles.flexShrink0.

https://github.com/Expensify/App/blob/f638162c899795821e8d6fa68735aa47fa6e2aa1/src/components/LHNOptionsList/OptionRowLHN.js#L84

Result

Screenshot_1689259459

Pujan92 avatar Jul 13 '23 14:07 Pujan92

@Pujan92 your textBreakStrategy solution doesn't work for me. I tested on various android devices.

Yes, while posting it was working that time. maybe that time the parent View is taking available width considering the Pill also exists. But now we removed the TextPill, I made a new proposal here.

Pujan92 avatar Jul 13 '23 14:07 Pujan92