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

IOU - Long email or name is overflowing the border of the modal #3172

Closed
isagoico opened this issue May 27, 2021 · 30 comments
Closed

IOU - Long email or name is overflowing the border of the modal #3172

isagoico opened this issue May 27, 2021 · 30 comments
Assignees
Labels
Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2

Comments

@isagoico
Copy link

isagoico commented May 27, 2021

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Expected Result:

Email or name should be wrapped so there's no overflowing at the edge of the modal

Actual Result:

Long email or name is overflowing the border of the modal

Action Performed:

Prerequisite: Account A should have a long email or name.

  1. Log in as account A and request money to account B
  2. Log in as account B and navigate to the conversation with account A
  3. Click on Pay

Workaround:

No need, visual issue.

Platform:

Where is this issue occurring?

Web ✔️
iOS
Android
Desktop App
Mobile Web

Version Number: 1.0.55-0
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

image

Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/168926

View all open jobs on Upwork

https://www.upwork.com/jobs/~0103190102106903c6

@isagoico isagoico added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels May 27, 2021
@MelvinBot
Copy link

Triggered auto assignment to @kadiealexander (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label May 27, 2021
@kadiealexander
Copy link
Contributor

Reproduced this issue in the web app:

image

@kadiealexander kadiealexander removed their assignment May 27, 2021
@MelvinBot
Copy link

Triggered auto assignment to @ctkochan22 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@isagoico
Copy link
Author

Issue reproducible today during KI retests

1 similar comment
@isagoico
Copy link
Author

isagoico commented Jun 7, 2021

Issue reproducible today during KI retests

@ctkochan22 ctkochan22 added Improvement Item broken or needs improvement. Weekly KSv2 and removed Daily KSv2 labels Jun 10, 2021
@ctkochan22
Copy link
Contributor

I don't think this is a daily. As its not blocking anything. But let me know otherwise

@ctkochan22 ctkochan22 removed their assignment Jun 10, 2021
@isagoico
Copy link
Author

Issue reproducible today during KI retests

@isagoico
Copy link
Author

Issue reproducible today during KI retests.

@iwiznia
Copy link
Contributor

iwiznia commented Jun 25, 2021

Not sure why this wasn't added an external label, adding it now.

@iwiznia iwiznia added the External Added to denote the issue can be worked on by a contributor label Jun 25, 2021
@MelvinBot
Copy link

Triggered auto assignment to @NicMendonca (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@MelvinBot MelvinBot added Daily KSv2 and removed Weekly KSv2 labels Jun 25, 2021
@isagoico
Copy link
Author

Issue reproducible during KI retests.

@rdjuric
Copy link
Contributor

rdjuric commented Jun 28, 2021

Proposal

This also happens in our chat if the name is long enough (or the screen small enough)
teste1

Both screens use our ReportActionItemFragment of type TEXT to render the display name. We can fix it by specifying width: 100% in the containerStyle we pass to our ToolTip component. I'll get passed to our Hooverable (the component that wraps our displayName) and fix the issue in both screens.
Screen Shot 2021-06-28 at 2 42 47 PM
Screen Shot 2021-06-28 at 2 42 39 PM

@MelvinBot
Copy link

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

@NicMendonca NicMendonca added Weekly KSv2 and removed Daily KSv2 labels Jun 28, 2021
@devpanther
Copy link

Proposal

This a styling issue and it is caused by the flex-shrink attached, once this is removed or actually made to shrink rather than overflow. We can solve this issue and other places it might be appearing

Screenshot 2021-06-29 at 7 07 11 PM

@Luke9389
Copy link
Contributor

@rdjuric Can you clarify what you meant to say with this?

I'll get passed to our Hooverable

@rdjuric
Copy link
Contributor

rdjuric commented Jun 29, 2021

@rdjuric Can you clarify what you meant to say with this?

Sorry! I meant to say that our ToolTip component can take a containerStyle prop, and that it passes it to our Hoverable component and to the View that wraps our text here:
https://github.com/Expensify/Expensify.cash/blob/5add245777dbcb7826f2444601debe15249bf7f1/src/components/Tooltip/index.js#L177-L188

@Luke9389
Copy link
Contributor

Ah, OK cool, thanks!

So you're thinking we should just put width: 100% in that containerStyle?

@rdjuric
Copy link
Contributor

rdjuric commented Jun 29, 2021

Ah, OK cool, thanks!

So you're thinking we should just put width: 100% in that containerStyle?

Yes, that will make our ToolTip component stay within the width of its parent. As our text is already styled correctly, correcting our ToolTip style made the text wrap correctly.

@parasharrajat
Copy link
Member

I am pretty sure that we want it to be trimmed with ellipsis... cc @shawnborton

@rdjuric
Copy link
Contributor

rdjuric commented Jun 29, 2021

@parasharrajat
Yea, I think that would look better too. And we would just need to add numberOfLines={1) to our Text (after fixing the ToolTip width)

@shawnborton
Copy link
Contributor

I agree, I would just trim these with an ellipsis.

@Luke9389
Copy link
Contributor

Ok, well @rdjuric you can go ahead and spin up the PR given that you've submitted you're proposal. 👍

@NicMendonca
Copy link
Contributor

@rdjuric can you please apply to the job in Upwork so we can start the contract? Thanks!

@rdjuric
Copy link
Contributor

rdjuric commented Jun 30, 2021

@NicMendonca Done!

@isagoico
Copy link
Author

isagoico commented Jul 5, 2021

Issue reproducible during KI retests.

@isagoico
Copy link
Author

Issue reproducible during KI retests

@isagoico
Copy link
Author

isagoico commented Jul 19, 2021

Issue not reproducible during KI retests

@rdjuric
Copy link
Contributor

rdjuric commented Jul 19, 2021

I can't repro this on staging. Can you give more details @isagoico?

Screen Shot 2021-07-19 at 12 06 18 PM

@isagoico
Copy link
Author

This was a mistake on my part. Issue was not reproducible during KI retests 🎉 fixing the comment.

@NicMendonca
Copy link
Contributor

Thanks everyone!

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 Improvement Item broken or needs improvement. Weekly KSv2
Projects
None yet
Development

No branches or pull requests