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 #21219][$250] Android - Group - Inconsistency in showing group name in header in mweb & Android #43068

Closed
2 of 6 tasks
kbecciv opened this issue Jun 4, 2024 · 55 comments
Assignees
Labels
Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Jun 4, 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: 1.4.79
Reproducible in staging?: y
Reproducible in production?: new feature
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4593905
Issue reported by: Applause - Internal Team

Action Performed:

  1. Open site in both mweb / Android
  2. Open group chat with many members
  3. Note the group name in header

Expected Result:

There should be no Inconsistency in showing group name in header in mweb and Android

Actual Result:

Inconsistency in showing group name in header in mweb and Android

Workaround:

n/a

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6502054_1717532864635.group.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d5db97bd918f3107
  • Upwork Job ID: 1798817306448133295
  • Last Price Increase: 2024-07-25
Issue OwnerCurrent Issue Owner: @s77rt
@kbecciv kbecciv added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Jun 4, 2024
Copy link
Contributor

github-actions bot commented Jun 4, 2024

👋 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.

Copy link

melvin-bot bot commented Jun 4, 2024

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

@kbecciv
Copy link
Author

kbecciv commented Jun 4, 2024

We think that this bug might be related to #vip-vsb

@deetergp
Copy link
Contributor

deetergp commented Jun 5, 2024

I agree that this should be consistent between mWeb & native in Android, but I also don't think it's serious enough to block the deploy.

@deetergp deetergp added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API Hourly KSv2 labels Jun 5, 2024
@tienifr
Copy link
Contributor

tienifr commented Jun 5, 2024

Proposal

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

Inconsistency in showing group name in header in mweb and Android

What is the root cause of that problem?

We do not explicit the width for display name, so it's cut off

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

We should add textBreakStrategy='simple' to

https://github.com/Expensify/App/blob/main/src/components/DisplayNames/index.native.tsx

What alternative solutions did you explore? (Optional)

  • NA

Result

image

image

@deetergp deetergp added the External Added to denote the issue can be worked on by a contributor label Jun 6, 2024
@melvin-bot melvin-bot bot changed the title Android - Group - Inconsistency in showing group name in header in mweb & Android [$250] Android - Group - Inconsistency in showing group name in header in mweb & Android Jun 6, 2024
Copy link

melvin-bot bot commented Jun 6, 2024

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

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

melvin-bot bot commented Jun 6, 2024

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

@s77rt
Copy link
Contributor

s77rt commented Jun 7, 2024

@tienifr Thanks for the proposal. We don't need to set an explicit width to the container but making it flexible (grow to fit) makes sense. However your solution causes a regression where the caret is also moved

Screenshot 2024-06-07 at 5 04 28 PM Screenshot 2024-06-07 at 5 04 43 PM

@tienifr
Copy link
Contributor

tienifr commented Jun 7, 2024

@s77rt ah i see, we can fix it with justify-content.

@s77rt
Copy link
Contributor

s77rt commented Jun 8, 2024

@tienifr Can you elaborate on where this style should be applied? After making the text flexible (grow) the justify content styling on the container won't have any effect

@tienifr
Copy link
Contributor

tienifr commented Jun 10, 2024

@s77rt I can't find the better solution. Can we approve the movement of the caret?

@s77rt
Copy link
Contributor

s77rt commented Jun 10, 2024

@tienifr I believe that's an unwanted change.

@tienifr
Copy link
Contributor

tienifr commented Jun 11, 2024

@s77rt I updated my proposal #43068 (comment), pls help check again. Thanks.

@s77rt
Copy link
Contributor

s77rt commented Jun 11, 2024

@tienifr That solution didn't fix the problem for me. It seems that it helps in some cases but not all, e.g. try with Abdelhafidh, admin+ctt@abdelhafidh.com, fgfdgd@azeazerr.ezaa, gefrere@rezrerezre.caea

Screenshot 2024-06-11 at 7 52 06 PM

Copy link

melvin-bot bot commented Jun 13, 2024

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

Copy link

melvin-bot bot commented Jun 17, 2024

@deetergp, @s77rt Eep! 4 days overdue now. Issues have feelings too...

@Krishna2323
Copy link
Contributor

This seems to be a bug in RN. Maybe we can create a minimal reproducible example, open an issue and follow up with a PR

I'm not sure about this because there is already issues for this opened in react native from a long time and there is no solution yet.

Similar issue that we should handle here

@s77rt
Copy link
Contributor

s77rt commented Jul 16, 2024

@Krishna2323 Do you have some links to those issues and/or a reproducible example? We should push to fix this upstream

@Krishna2323
Copy link
Contributor

@s77rt facebook/react-native#15114, there is no ideal solution for this, there are only workarounds like setting flex: 1 or the one I suggested in my proposal, we have using workarounds till now and I don't think it will be fixed in RN soon. So we need to find and apply the change each time the issue occurs.

@s77rt
Copy link
Contributor

s77rt commented Jul 17, 2024

@Krishna2323 This PR facebook/react-native#41770 seems promising and it could fix this bug too

@Krishna2323
Copy link
Contributor

@s77rt, yes, seems like it could fix this issue but I'm not sure if that will be merged anytime soon. What should we do here then? There is one more issue opened here.

@bernhardoj
Copy link
Contributor

I think this kind of issue is being tracked here #21219

@s77rt
Copy link
Contributor

s77rt commented Jul 18, 2024

@deetergp Let's hold on #21219

Copy link

melvin-bot bot commented Jul 18, 2024

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

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

Copy link

melvin-bot bot commented Jul 23, 2024

@deetergp, @s77rt Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot melvin-bot bot added the Overdue label Jul 23, 2024
@s77rt
Copy link
Contributor

s77rt commented Jul 23, 2024

To be held on #21219

@melvin-bot melvin-bot bot removed the Overdue label Jul 23, 2024
Copy link

melvin-bot bot commented Jul 25, 2024

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

Copy link

melvin-bot bot commented Jul 29, 2024

@deetergp, @s77rt Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot melvin-bot bot added the Overdue label Jul 29, 2024
@s77rt
Copy link
Contributor

s77rt commented Jul 29, 2024

@deetergp Let's put this on hold #43068 (comment)

@melvin-bot melvin-bot bot removed the Overdue label Jul 29, 2024
@deetergp deetergp changed the title [$250] Android - Group - Inconsistency in showing group name in header in mweb & Android [HOLD #21219][$250] Android - Group - Inconsistency in showing group name in header in mweb & Android Jul 29, 2024
Copy link

melvin-bot bot commented Aug 2, 2024

@deetergp, @s77rt Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Aug 2, 2024
@deetergp deetergp added Weekly KSv2 and removed Daily KSv2 labels Aug 2, 2024
@melvin-bot melvin-bot bot removed the Overdue label Aug 2, 2024
@deetergp
Copy link
Contributor

deetergp commented Aug 2, 2024

Still on hold so making this weekly for now.

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Third week)

@deetergp
Copy link
Contributor

deetergp commented Aug 5, 2024

If it hasn't been reproducible for the last three weeks, I think we can close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants