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

[$500] mWeb - Login - When magic code page is displayed, many of the page elements are missing #37090

Closed
1 of 6 tasks
lanitochka17 opened this issue Feb 22, 2024 · 25 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 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

Comments

@lanitochka17
Copy link

lanitochka17 commented Feb 22, 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.43-7
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): vdargentotest+mweb022024.2@gmail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Go to https://staging.new.expensify.com/r/4098419659819887
  2. Navigate to the top of the conversation
  3. Tap on sign in
  4. Enter an email address and tap on "Continue" button
  5. Verify that only the top part of the page is fully visible
  6. Tap on any of the input fields
  7. Verify that the whole page is now completely visible

Expected Result:

The magic code page should have all of its elements fully visible without the need to tap on the inputs

Actual Result

Many of the page elements are missing until an input field is tapped

Workaround:

Unknown

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

Bug6387422_1708548142823.Fqoa6468_1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01acd63de5f6f153b1
  • Upwork Job ID: 1760889422764716032
  • Last Price Increase: 2024-03-08
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 22, 2024
Copy link

melvin-bot bot commented Feb 22, 2024

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

@lanitochka17
Copy link
Author

@bz FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@kadiealexander kadiealexander added the External Added to denote the issue can be worked on by a contributor label Feb 23, 2024
@melvin-bot melvin-bot bot changed the title mWeb - Login - When magic code page is displayed, many of the page elements are missing [$500] mWeb - Login - When magic code page is displayed, many of the page elements are missing Feb 23, 2024
Copy link

melvin-bot bot commented Feb 23, 2024

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

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

melvin-bot bot commented Feb 23, 2024

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

@shahinyan11
Copy link
Contributor

Proposal

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

mWeb - Login - When magic code page is displayed, many of the page elements are missing

What is the root cause of that problem?

Regression

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

Revert this part of code to previous version. Example below

  onBlur={() =>
      // This delay is to avoid the validate being called before google iframe is rendered to
      // avoid error message appearing after pressing google signin button.
      setTimeout(() => {
          if (firstBlurred.current || !Visibility.isVisible() || !Visibility.hasFocus()) {
              return;
          }
          firstBlurred.current = true;
          validate(login);
      }, 500)
  }

What alternative solutions did you explore? (Optional)

@jeremy-croff
Copy link
Contributor

Proposal

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

When we go through the modal sign in flow, we break the layout when hiding the keyboard, and navigating to the next screen. In this ticket for example you can observe they hide the keyboard, and click next. The following page's layout is not adjusted and has green space where the keyboard is supposed to be. This is how the bug is reproduced in the sign in modal ( not scrolling on the report )

What is the root cause of that problem?

this signin modal has shouldEnableMaxHeight for a layout that changes.. this prevents it from responding to the layout change.

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

We can remove shouldEnableMaxHeight on the signin modal in order for it to respond to the changes again.

What alternative solutions did you explore? (Optional)

@getusha
Copy link
Contributor

getusha commented Feb 23, 2024

Regression

@shahinyan11 could you specify which PR caused the regression?

@jeremy-croff
Copy link
Contributor

onBlur={() =>
// This delay is to avoid the validate being called before google iframe is rendered to
// avoid error message appearing after pressing google signin button.
setTimeout(() => {
if (firstBlurred.current || !Visibility.isVisible() || !Visibility.hasFocus()) {
return;
}
firstBlurred.current = true;
validate(login);
}, 500)
}

I'm still able to reproduce it with the rca steps I listed, after reverting this change.

@shahinyan11
Copy link
Contributor

shahinyan11 commented Feb 23, 2024

Sorry my bad. @jeremy-croff is right , The RC and solution in my proposal Is not correct

@getusha
Copy link
Contributor

getusha commented Feb 23, 2024

this signin modal has shouldEnableMaxHeight for a layout that changes.. this prevents it from responding to the layout change.

@jeremy-croff looks like this was present for over 6 months.

@bernhardoj
Copy link
Contributor

Looks like caused by #33546

Removing shouldEnableMaxHeight from useWindowDimensions solves the issue

const {windowHeight, isSmallScreenWidth} = useWindowDimensions(shouldEnableMaxHeight);

@shubham1206agra
Copy link
Contributor

@bernhardoj Can you test the same on latest main and confirm that the problem persist?

@bernhardoj
Copy link
Contributor

yes, still reproducible on latest main

@suneox
Copy link
Contributor

suneox commented Feb 23, 2024

let me check on this case why the input still focus but keyboard not open, may be it related to this proposal

@melvin-bot melvin-bot bot added the Overdue label Feb 26, 2024
@getusha
Copy link
Contributor

getusha commented Feb 26, 2024

will be handled by @suneox, as a regression.

@melvin-bot melvin-bot bot removed the Overdue label Feb 26, 2024
@suneox
Copy link
Contributor

suneox commented Feb 26, 2024

will be handled by @suneox, as a regression.

Sure I have created PR and waiting for review

@getusha
Copy link
Contributor

getusha commented Feb 28, 2024

@shubham1206agra i believe this PR is awaiting your review #37147

@shubham1206agra
Copy link
Contributor

Yes I will review it shortly

Copy link

melvin-bot bot commented Mar 1, 2024

📣 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 Mar 1, 2024
@getusha
Copy link
Contributor

getusha commented Mar 3, 2024

#37147 merged and deployed to staging

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@melvin-bot melvin-bot bot added the Overdue label Mar 6, 2024
@kadiealexander
Copy link
Contributor

@getusha I think no one is needing payment here as this was a regression of #33546 and was fixed and reviewed by the same people assigned to the OG issue, could you please confirm?

@melvin-bot melvin-bot bot removed the Overdue label Mar 6, 2024
@getusha
Copy link
Contributor

getusha commented Mar 7, 2024

I think no one is needing payment here as this was a regression of #33546 and was fixed and reviewed by the same people assigned to the OG issue,

Yes, we can close it.

Copy link

melvin-bot bot commented Mar 7, 2024

@kadiealexander @getusha 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!

Copy link

melvin-bot bot commented Mar 8, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 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
Projects
None yet
Development

No branches or pull requests

9 participants