[HOLD for payment 2022-12-23] [$2000] [Bug] Keyboard being open/closed changes position of ToS on certain mobile platforms - reported by @gadhiyamanan

[mvtglobally]

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. Open login page on a fresh install OR after clearing app data
2. Enter email/phone number.
3. Open the touch screen keyboard by tapping on the login text input. Notice how the Terms and Licenses don't shift upwards to avoid the keyboard.
4. Press "Continue" to get to the password form.
5. Repeat step 3 with the password input. Notice how the Terms and Licenses doesn't shift upwards to avoid the keyboard.

Expected Result:

• The Terms and Licenses should shift upwards to avoid the keyboard on both the login and password forms.

Actual result:

• The page becomes scrollable, and you can scroll the login/password input out of view or out of view
• The Terms and Licenses are not visible

Workaround:

None

Platform:

Where is this issue occurring?

• iOS Safari
• iOS Native
• Android Chrome

Version Number: 1.2.4-0
Reproducible in staging?: need repro
Reproducible in production?: need repro
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

iOS - Both forms are broken

ios.login.broken.mov

ios.password.broken.mov

Mobile Safari - Both forms are broken

mobile.safari.login.broken.mov

mobile.safari.password.broken.mov

Mobile Chrome - Interestingly, it appears to work on the login form, but not the password form

mobile.chrome.password.broken.mov

mobile.chrome.login.mov

Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1661843853973199

View all open jobs on GitHub

[melvin-bot]

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

[JmillsExpensify]

I'm on iOS, so I'm unable to reproduce. That said, can you clarify exactly what you mean by "instruction goes up?" I'm not following what the issue is here. Thank you!

[gadhiyamanan]

@JmillsExpensify here instruction means By logging in, you agree to the... , when you click on the phone field there is a large gap between the continue button and the instruction. after typing any word, space will remove

it's reproduced on android

[JmillsExpensify]

Ok thanks. Still unable to reproduce since I don't have an android device. That said, I have a suspicion that this is yet another symptom of our keyboard implementation. @roryabraham do you mind confirming if you think my hunch is right? If so, I'll update the issue title, put this issue on hold, and add it to the main tracker.

[roryabraham]

Okay, I was able to reproduce this, going to update the OP to add some additional details then I agree we should add it to the keyboard tracking issue (cc @tgolen)

[JmillsExpensify]

Thanks @roryabraham! I'll handle adding to the keyboard tracking issue.

[roryabraham]

Added clarification and consistent reproduction steps, removed the Needs Reproduction label.

[roryabraham]

Also for transparency I was testing with a physical Pixel 4a

[JmillsExpensify]

Thanks @roryabraham.

[JmillsExpensify]

Still on hold.

[JmillsExpensify]

Still on hold.

[tgolen]

Wow interesting, when you say "right spot" what do you mean?

You can see here where I've added it. If you don't put it in the right spot in the view hierarchy, then you get a bunch of funky layout issues (like everything smooshed into 50 pixels at the top of the page, or the page not spanning the full vertical height).

ideally, we'd want an over-arching solution that can work for all platforms, but at this point, would it be wrong to consider splitting the login/password forms into different components for each platform?

I appreciate the thought, but I think that would be one of the worst solutions 😬 so I would want to avoid that at all costs.

I also find that isShown isn't working for mobile Chrome too (eg. it's always false), which doesn't surprise me a whole lot.

I think all that we should focus on right now are:

• Getting the terms to display above the keyboard for both iOS and Android native on both login and password forms

I've updated my PR to achieve this, but some of the styles need cleaned up. I accomplish it by adding some margin to the bottom of the terms container when the keyboard is open. However, this has several drawbacks:

1. It makes the animation a little janky because the margin isn't added until the keyboard animation ends
2. Android doesn't show the extra margin as nicely as iOS does and leaves extra space below the terms (this could be fixed by only applying the 60px of margin to iOS)

iOS - shows the janky animation

2022-12-12_14-28-54.mp4

Android - shows the extra margin at the bottom

2022-12-12_14-30-53.mp4

[jasperhuangg]

I appreciate the thought, but I think that would be one of the worst solutions 😬 so I would want to avoid that at all costs.

hehe yup, my brain went there, but it just felt soooo wrong 😆

I also find that isShown isn't working for mobile Chrome too (eg. it's always false), which doesn't surprise me a whole lot.

Yup, the withKeyboardState HOC doesn't work at all with any of the mobile web platforms, because React Native's Keyboard library it uses only works on native platforms :/

I've updated my PR to achieve this, but some of the styles need cleaned up. I accomplish it by adding some margin to the bottom of the terms container when the keyboard is open. However, this has several drawbacks:

Gotcha, thanks for clarifying. Sorry I haven't had the chance to dive into your PR, but my next steps are clear. I'll help you look into those styling issues today. I'll also add myself as a reviewer. Thanks for those screen recordings!

[jasperhuangg]

To clarify, Tim opened a PR earlier that included fixes for this issue on iOS Native. So on Tim's PR, the TermsAndLicenses component successfully avoids the keyboard on the following platforms:

• iOS Native ✅
• Android Native ✅
• Mobile Chrome
  • login form ✅
  • the password form (❌ , but I will outline why we probably shouldn't fix this in another comment below)
• Mobile Safari ❌
  • We have agreed to leave this for now. As long as the view is still scrollable and people can blur the text input, then the usability of the page is not really diminished.

[jasperhuangg]

For the password form on Mobile Chrome, the TermsAndLicenses component does actually shift up to avoid the keyboard. It turns out that there's just too many items occupying space on the screen for it to remain in view.

When I mean "too many items", compare these screenshots of the password form on the same Android emulator on both Android Native (right) and Mobile Chrome (left):

Notice how Mobile Chrome (left) includes a nav bar and a password bar, all which occupy just enough height for the TermsAndLicenses component to get cut off by the keyboard.

We could reduce the margins/padding of everything on the screen to get it to fit, but since you can still scroll down to see the TermsAndLicenses, I think we should just do nothing here like we are for Mobile Safari.

cc @tgolen Do you agree with my read on this one?

[tgolen]

Hey, that's a good comparison and summary, @jasperhuangg. I agree with you that there isn't anything more we need to do.

From talking about it with Shawn in Slack, these changes are probably get thrown out at some point because we are going to be adding a footer with links to these pages which will be below the fold.

Because of that, I think I can go ahead and clean up my PR, and we can merge it with the fixes it contains, and close this out. What do you think?

[jasperhuangg]

Totally agree! I definitely think we should keep the changes from your PR since we'll probably need them later on anyways when we add the footer. Thanks for keeping everyone on the same page and your help on this issue!

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.40-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix the position of the TOS on the sign on page to be above the keyboard #13529

If no regressions arise, payment will be issued on 2022-12-23. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@sobitneupane / @tgolen / @jasperhuangg] The PR that introduced the bug has been identified. Link to the PR: NA, this has been a problem since it was built
• [@sobitneupane / @tgolen / @jasperhuangg] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: NA
• [@sobitneupane / @tgolen / @jasperhuangg] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: NA
• [@JmillsExpensify] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

[tgolen]

I'm checking off most of those as NA since this has been an issue ever since the page was built

[JmillsExpensify]

Great thank you! I'll circle back on the regression test/payment once I return on Tuesday.

[JmillsExpensify]

Alright, circling back on this issue for the payment. It's due tomorrow, so let's make sure all is in order before then. @sobitneupane I've just invited you to the Upwork job. Mind accepting?

[JmillsExpensify]

@gadhiyamanan You're also eligible for the reporting bonus. Please apply to the Upwork job.

[gadhiyamanan]

@JmillsExpensify applied

[JmillsExpensify]

Thanks offer sent!

Also I closed the loop on the regression test, so I'm crossing that off above in the BZ checklist.

[JmillsExpensify]

Summarizing how payments will shape out.

• External issue reporter: @gadhiyamanan $250
• Contributor+ (internal PR review): @sobitneupane $1,000

[JmillsExpensify]

Just realized that I didn't get around to paying this out before the Christmas holiday in the US, so I've just circled back and paid out both Contributors per the payments above. We already have a keyboard regression test is play, so I'm closing out this issue.