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

Invite member - Multiline text overlaps with label #5924

Closed
isagoico opened this issue Oct 18, 2021 · 18 comments
Closed

Invite member - Multiline text overlaps with label #5924

isagoico opened this issue Oct 18, 2021 · 18 comments
Assignees
Labels
Daily KSv2 DeployBlockerCash This issue or pull request should block deployment Engineering

Comments

@isagoico
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Go to https://staging.new.expensify.com
  2. Log in with expensifail account
  3. Select any workspace
  4. Go to Manage members and Click invite
  5. Invite 3 users and scroll the box

Expected Result:

Emails should not overlap with the label.

Actual Result:

Text is overlapping with the label.

Workaround:

None needed, visual issue.

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.8-0

Reproducible in staging?: Yes
Reproducible in production?: No

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

image

Bug5282774_Recording__446.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause
Slack conversation:

View all open jobs on GitHub

@isagoico isagoico added DeployBlockerCash This issue or pull request should block deployment Engineering Daily KSv2 labels Oct 18, 2021
@MelvinBot
Copy link

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

@OSBotify
Copy link
Contributor

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

@isagoico
Copy link
Author

Opened this issue separately because of this comment #5904 (comment). Added the deploy blocker label since the issue is not reproducible in Prod

@Julesssss
Copy link
Contributor

We're close to merging a PR that completely replaces this page.

Because it's about to be replaced I think we should ignore this bug, and because it already exists in prod I'm going to close the issue.

@parasharrajat
Copy link
Member

@Julesssss but this issue still exists with the multiple Textinput and it's a regression from #5704.

@Julesssss
Copy link
Contributor

Ah, thanks I hadn't seen this. As the issue exists elsewhere let's keep it open until the regression is fixed 👍

@Julesssss Julesssss reopened this Oct 18, 2021
@isagoico
Copy link
Author

Looks like the Paypal and Phone number modal are having a extremely similar issue too (commented here) Let me know if this will be fixed here or a new issue is needed for this.

image

@parasharrajat
Copy link
Member

Well @isagoico This is a different issue and I know the solution won't fix it.

@parasharrajat
Copy link
Member

@AlfredoAlc Tagging you since this is the regression we are trying to solve in #5704.

@roryabraham
Copy link
Contributor

roryabraham commented Oct 20, 2021

@parasharrajat @AlfredoAlc just catching up here, but is there anything wrong with this approach: (draft PR: #5958). Seems to fix this issue but I'm not sure if there might be any other side-effects.

@roryabraham
Copy link
Contributor

roryabraham commented Oct 20, 2021

@AlfredoAlc I just read this comment and that solution seems pretty good to me. I'm not sure if this will work, but have you investigated suppressing the auto-fill styles in css using the autofill pseudoclass? Something like this (ideally using our theme colors instead):

input:autofill,
input:autofill:hover,
input:autofill:hover,
input:autofill:active, {
    background-color: white;
}

Note: The user agent style sheets of many browsers use !important in their :-webkit-autofill style declarations, making them non-overrideable by webpages without resorting to JavaScript hacks.

If it came down to it we could also maybe override the style in JavaScript, but at that point I would probably say it's not worth it and we should just accept the ugly style with autofilled inputs and move on.

@tylerkaraszewski
Copy link
Contributor

I'm unassigning myself and tagging external I guess. I don't know why this is hourly, maybe we should remove that?

@tylerkaraszewski tylerkaraszewski removed their assignment Oct 20, 2021
@tylerkaraszewski tylerkaraszewski added the External Added to denote the issue can be worked on by a contributor label Oct 20, 2021
@MelvinBot
Copy link

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

@MelvinBot MelvinBot added the Daily KSv2 label Oct 20, 2021
@roryabraham
Copy link
Contributor

It was just hourly because it's a deploy blocker, but we should have a solution in this PR.

@parasharrajat
Copy link
Member

I believe there is no need to export this as this is a regression from other PR which is yet to be deployed to PROD and under regression period.

@roryabraham
Copy link
Contributor

Agree, removing the external label and unassigning @mallenexpensify. I was able to QA this fix on staging web but not iOS yet, so I'll assign this to myself so I can fix iOS staging deploys and come back to QA this on iOS.

@isagoico can you please QA #5959 on Android staging?

@roryabraham roryabraham removed External Added to denote the issue can be worked on by a contributor Hourly KSv2 labels Oct 21, 2021
@mallenexpensify
Copy link
Contributor

While I'm here... I've seen the overlapping text with label a couple places. Will that be fixed (most) everywhere once this is fixed and closed?

@roryabraham
Copy link
Contributor

This has been confirmed fixed on 1.8.8-8, closing this out. @mallenexpensify if you find other instances of a similar bug in 1.8.8-8 or above, please reopen and/or comment in #4515

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 DeployBlockerCash This issue or pull request should block deployment Engineering
Projects
None yet
Development

No branches or pull requests

8 participants