[HOLD #10414] [$1000] mWeb/safari - Split- What's it for field is hidden for small devices

[kbecciv]

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. Log in with any account
2. Tap on Fub menu
3. Select Split bill
4. Select any users and tap Next
5. Tap on What's it for? field

Expected Result:

What's it for? field should be visible

Actual Result:

What's it for field is hidden for small devices

Workaround:

Unknown

Platforms:

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

• [ ] Android / native
• [ ] Android / Chrome
• [ ] iOS / native
• [ ] iOS / Safari
• [ ] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: 1.2.77.1

Reproducible in staging?: Yes

Reproducible in production?: Yes

If this was caught during regression testing, add the test name, ID and link from TestRail:

Email or phone of affected tester (no customers):

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

Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/93399543/221976923-f9c63452-3019-4db0-9160-3b15e52fc811.MP4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ccc06df1a8d7be03
• Upwork Job ID: 1631324582561415168
• Last Price Increase: 2023-03-02

[MelvinBot]

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

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• [x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• [x] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• [x] This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• [x] This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[MelvinBot]

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

[MelvinBot]

Current assignee @davidcardoza is eligible for the External assigner, not assigning anyone new.

[davidcardoza]

Makes sense, I was able to reproduce too.

[MelvinBot]

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

[MelvinBot]

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

[Ollyws]

Proposal

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

The view is scrolled up on the IOU modal when the keyboard is opened.

What is the root cause of that problem?

This is a problem on Safari, since we started restricting the height of screenWrapper based on the window height, the window scrolls up when it's resized.

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

We deal with this issue on ReportScreen by using withViewportOffsetTop to adjust the marginTop of screenWrapper, we can do the same in IOUModal.

[s77rt]

Thanks for the proposal @Ollyws. I will review asap. Just one question, are you saying that this issue is a regression from https://github.com/Expensify/App/pull/14392?

[Ollyws]

@s77rt Yeah, more specifically the RNW changes. Revert this commit and the issue doesn't seem to occur.

[s77rt]

@Ollyws Can you help me how did you manage to reproduce the bug? And why this does not effect devices with bigger screens?

[s77rt]

@Ollyws Thanks again for the proposal. I think the RCA is not convincing enough, I believe there is a missing piece. For instance, I can see that the "What's it for?" field get focused but the keyboard does not pull up (something must be wrong here) and only if you pull the keyboard in this state the bug will happen. Now if you try to focus the field again all will be fine.

So, I think it may be a good call to investigate why the input get focused but the keyboard does not show up, fixing this may be enough.

https://user-images.githubusercontent.com/16493223/222554564-b64b49f1-f847-494a-8162-c4bc8eaf7600.mp4

[s77rt]

Not overdue. Waiting for proposal...

[bernhardoj]

Proposal

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

What's it for field is hidden (out of view) when focus on the field (surprisingly only for the first time).

What is the root cause of that problem?

This is related to the regression caused by this PR https://github.com/Expensify/App/pull/14392 where we are resizing the height with the new height when a keyboard opens. However, when we focus on an input, the keyboard will appear, and it will scroll the page (the visualViewport to be exact) (it is explained here too). So, after resizing it smaller, the browser scrolls it (the viewport) which makes it hidden. You can test it by focus the input then type window.scrollY and it will return value > 0. But why it only happens for the first time?

We have this PR https://github.com/Expensify/App/pull/15100 that tricks the browser to think that the input is not in the possibly-covered-by-keyboard (I don't know what to call this) area on focus by switching the opacity to 0 and 1 within 0.1s so the browser does not do the scroll. You can see what issue the PR fixes which works great. Now, when we open the "What's it for" page, we auto focus the field, however with a delay added by this PR https://github.com/Expensify/App/pull/11718 to fix buggy text field cursor position. If there is a delay, the auto focus won't work (the field gets focused, but the keyboard does not show) on Safari iOS (ref) and this happen to many pages that will render the component after a transition is end.

One of the example: https://github.com/Expensify/App/blob/f95292aa3facb205df5afb2c57b9a14eac8db61d/src/pages/NewChatPage.js#L248-L267

So, the focus is called, the opacity switching is done, but the keyboard does not show which makes the "trick" failed. So, when we press the text field again, the keyboard will show, and the page will be scrolled out of view.

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

We need to remove the focus delay because that is only for Android. I have re-tested the issue that the PR fix here https://github.com/Expensify/App/issues/10698 and it does not happen after removing the delay. Need others confirmation here too.

What alternative solutions did you explore? (Optional)

Instead of doing the opacity switching on the css, we can do it on js, specifically on our TextInput component. We need to have a new state called opacity with a default value of 1. Every time we focus/press (onPress & onFocus), we set the opacity to 0, and inside the setState callback, we set a timeout of 10ms to reset it back to 1. This will achieve the same result as the css, but with the addition to press action. Also, because now we do it on js, we can safely remove the css one.

The opacity state will be passed to the style of RNTextInput

[s77rt]

@bernhardoj Thanks for the proposal. Can you elaborate the opacity hack part, I don't see its relation here.

[s77rt]

About the focus problem mentioned here https://github.com/Expensify/App/issues/15572#issuecomment-1452555331. This can be ignored it's out of the scope of this issue and it's reported here already https://github.com/Expensify/App/issues/10414. Let's just fix the fact that after focusing the field is out of view.

@bernhardoj I'm beginning to see the light here :sweat_smile: and the relation of opacity hack, but I don't think we should remove the delay (let's keep that as last resort).

[s77rt]

Okay, so technically this issue is the same as https://github.com/Expensify/App/issues/14716 with the additional challenge which is the delayed autofocus. (i.e. https://github.com/Expensify/App/pull/15100 does not handle the case where we auto focus an input with a delay). So far I see those options:

1. Find a better solution than https://github.com/Expensify/App/pull/15100
2. Hold in favor of https://github.com/Expensify/App/issues/10414
3. Avoid the issue by focusing the input without a a delay

cc @puneetlath @davidcardoza Any thoughts?

[s77rt]

Summary of the RCA: When you focus the input you have 0.01sec to bring up the keyboard. In this 0.01sec we trick the browser not to autoscroll to the input. Currently with delayed auto focus, when the input get focused the keyboard won't show up within 0.01sec (it won't show up at all) due to this bug https://github.com/Expensify/App/issues/10414 so when you try to bring it up manually it will autoscroll to the input and causing the bug.

That being said I vote to hold this on favor of https://github.com/Expensify/App/issues/10414

[bernhardoj]

I don't think this https://github.com/Expensify/App/issues/10414 is going to be solved because that's the behavior of iOS Safari.

I agree we should find a better way to trick the browser, which I've added it on my optional solution.

[s77rt]

@bernhardoj I think your alternative solution may actually work since we can control when the hack to start taking effect, but timing is a critical factor here, also I don't see why we would need onPress, shouldn't onFocus be enough? Did you test the implementation of solution?

[bernhardoj]

The onPress is needed in case of the input is focused but no keyboard is shown yet.

[s77rt]

@bernhardoj In that case I don't think the hack is that much better, onPress won't be needed if https://github.com/Expensify/App/issues/10414 got fixed. I think we may handle this the same way we decide to handle https://github.com/Expensify/App/issues/10414.

[bernhardoj]

I initially think that there will be other cases where the input is focused but the keyboard still shows, for example when we press back to close the keyboard on Android. But this is not the case with iOS, except when we are toggling the keyboard on a Simulator. I don't know if there is any other way to show the keyboard without focusing on an input.

This will still cause the scroll issue because we show the keyboard without focus on anything. So, to be safe, I'm thinking that we need to get away from the opacity "trick" and instead let it scroll but then reset it back. We can add a visualViewport.onscroll listener to reset the scroll to 0 with window.scrollTo(0). Or, we can use the marginTop approach based on visualViewport.offsetTop value. It is the offset of the visual viewport top with the layout viewport top. However, with both solutions, the page header will not be visible for a very short amount of time (out of view because of the scroll) then become visible again.

[s77rt]

@bernhardoj That solution would introduce unnecessary flicker/scroll right? I still think it's better to prevent scroll than allow it then negate it. The above case where the keyboard is always shown is an edge case I suppose.

[puneetlath]

That being said I vote to hold this on favor of https://github.com/Expensify/App/issues/10414

Just to make sure I understand. If https://github.com/Expensify/App/issues/10414 is solved, this issue will be too?

[s77rt]

@puneetlath Yes, that's correct!

[puneetlath]

Ok got it. I think I agree that we should close this in favor of that issue. It seems to have active movement.

[s77rt]

I think we should just put it on hold since they don't have the exact same root cause.

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[s77rt]

@mvtglobally It's still reproducible

[s77rt]

Still on hold...

[s77rt]

On hold

[puneetlath]

Going to make weekly while it's on hold.

[s77rt]

Still on hold

[s77rt]

Same ^

[s77rt]

Still on hold.

[puneetlath]

Still on hold.

[s77rt]

Same, still on hold

[s77rt]

Still on hold

[puneetlath]

Still on hold.

[s77rt]

On hold

[s77rt]

Still on hold

[s77rt]

Same ^

[s77rt]

Same ^

[s77rt]

Still on hold

[bernhardoj]

I don't think this is still reproducible as we already removed the opacity animation hack?

[s77rt]

Oh right, the input is also moved to it's own page (it's in the top) so it won't even auto scroll.

Looks we are good to close this one @puneetlath

[puneetlath]

Nice! Closing!