[HOLD for payment 2023-12-15] [HOLD for payment 2023-11-13] [$4000] mWeb - Request/Split Bill - After pressing the back arrow to "Send/Request money" screen, the keyboard flashes

[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:

• Access staging.new.expensify.com on Android
• Log in
• Tap + > Send Money or Request Money
• Tap on the Currency Sign
• The Keybard should open (if not open it clicking on the input)
• Choose a currency and enter an amount
• Tap "Next" button
• Tap back arrow

Expected Result:

When returning to the previous screen, the keyboard flashes

Actual Result:

After pressing the back arrow to the "Send money" or "Request money" screen, the keyboard flashes

Workaround:

Unknown

Platforms:

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

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

Version Number: 1.3.4.0

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/233863699-86384545-64ba-4e6d-9d4d-98ce554713c5.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/~01c69f6c8136159450
• Upwork Job ID: 1653068116835205120
• Last Price Increase: 2023-10-17
• Automatic offers:
• fedirjh | Reviewer | 27281218

[MelvinBot]

Triggered auto assignment to @Christinadobrzyn (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

[Christinadobrzyn]

I think this is a dupe of https://github.com/Expensify/App/issues/17805

@parasharrajat or @dangrous would you be able to confirm?

[parasharrajat]

I don't think it is a duplicate. Here we are saying that Numberpad on the Amount page is pushed up due to the keyboard while going back.

[s77rt]

This is a dupe of https://github.com/Expensify/App/issues/15270 We have a PR https://github.com/Expensify/App/pull/16443 that is merged but didn't really fix that instance

[Christinadobrzyn]

Thanks @parasharrajat and @s77rt - will we try to fix this in https://github.com/Expensify/App/pull/16443 or should we open this up as a new job?

[parasharrajat]

That depends on whether this will be solved on #16443 or not.

[s77rt]

Given that the exact instance was highlighted in the video below (from linked issue) I think it should be fixed on that PR

https://user-images.githubusercontent.com/93399543/219829185-e4c5cf6a-9b44-4c3e-b2bb-d76af8187e5f.mp4

[s77rt]

Actually the behaviour is slightly different. It looks better than it used to be, so I don't know for sure. It may be best to hear from PR reviewers/assignees.

[Christinadobrzyn]

Checking on a regression here https://github.com/Expensify/App/pull/16443#issuecomment-1522063464 - if not a regression we'll continue with this as a separate job

[Christinadobrzyn]

Sounds like this isn't a regression from the other fix but I'm not able to reproduce this.

Steps:

• Access staging.new.expensify.com on Android
• Log in
• Open Send Money or Request Money screen
• Tap on the Currency Sign
• The Keybard should open (if not open it clicking on the input)
• Choose a currency and enter an amount
• Tap "Next" button
• Tap back arrow

The keyboard loads at the bottom of the screen as expected. I'll see if anyone else can reproduce this. https://expensify.slack.com/archives/C9YU7BX5M/p1682716124592779

[kbecciv]

@Christinadobrzyn I am able to reproduce on SG21/Chrome with build 1.3.8.5

https://user-images.githubusercontent.com/93399543/235256101-39170de2-5da5-439e-a5fc-883837b850e8.mp4

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[Christinadobrzyn]

I'm able to reproduce it! Adding external

[MelvinBot]

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

[tgolen]

All right, let's get some proposals!

[MelvinBot]

Upwork job price has been updated to $2000

[Christinadobrzyn]

Since this was reported a few weeks ago but we just recently posted the Upwork, I'm going to raise the price to get some eyes on a fix.

[hoangzinh]

Proposal

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

mWeb - Request/Split Bill - After pressing the back arrow to "Send/Request money" screen, the keyboard flashes

What is the root cause of that problem?

• This is current styling of amountView in Amount screen

  

  The flexBasic is current 0%, which means amountView will grown from 0.

• In the Select participant screen, there is soft-keyboard showing up.

  

=> When we're in the Select participant screen and navigate back to Amount screen in native devices, Soft-keyboard will hide but there is an amination time. That mean, in the first time, Amount screen only has almost half-screen, then will expand to full when soft-keyboard is completely hide => leads to current effect.

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

I have an idea to have another option prop (alike existing prop shouldEnableMaxHeight) but to lock height to windowHeight in this line. But I have 2 issues when using windowHeight:

• As @fedirjh pointed out here, on mweb, current props.windowHeight actually refers to the height of the window's viewport, rather than the entire window height.
• On mweb, we don't know when the keyboard is shown, because it's not actually listen to Keyboard events in web

So I came with an idea to have another component state windowHeightWithoutKeyboardShown to store windowHeight without Keyboard shown. I think we can put it into ScreenWrapper component. Then in componentDidUpdate of this component, we can update this new state with windowHeight, unless:

• windowHeight is not change comparing with prevProps
• or windowWidth is not changed but previous windowHeight is greater than current windowHeight a MIN_COMMON_KEYBOARD_HEIGHT unit (I couldn't find a correct value in internet but I think most of keyboard will greater than 150)
  • I need to check windowWidth is not changed to avoid case when Device is rotated to horizontal view
  • I assume if windowHeight is suddenly change the height to more than 150 unit, it can only because it's opening Keyboard.

Then I can use this new state to lock the height as I mentioned above.

Recording result in mweb Android + IOS:

https://github.com/Expensify/App/assets/9639873/1b6514be-c17b-4bb7-acad-84df718cd907 https://github.com/Expensify/App/assets/9639873/0ab18ea2-4793-46a9-8e30-22619ec1a90d

Code reference:

```JSX // init windowHeightWithoutKeyboardShown = props.windowHeight in the initialize method ... // then having this lifecycle method componentDidUpdate(prevProps) { if (prevProps.windowHeight === this.props.windowHeight || (prevProps.windowWidth === this.props.windowWidth && prevProps.windowHeight > this.props.windowHeight + 150)) { return; } this.setState({windowHeightWithoutKeyboardShown: this.props.windowHeight}); } ... // in render method const height = this.props.shouldLockHeight ? this.state.windowHeightWithoutKeyboardShown : undefined; ```

[melvin-bot[bot]]

@tgolen @fedirjh @Christinadobrzyn 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!

[tgolen]

@hoangzinh thanks for the details on the different options. I'd like to see a video of option 4 if you could add one please? It also sounds like option 1 would be mostly fine. I think the delay would not be noticeable since there is an animation happening. Could you please provide a video of that one too?

[hoangzinh]

@tgolen I have a typo in option 4, it should be "NOT completely solve issue". Btw, I will send both recordings of option 1 & 4 in next couple of hours

[hoangzinh]

• Option 1:

https://user-images.githubusercontent.com/9639873/236866922-40f17981-c9bb-42d8-b822-581c353ecf7f.mov

https://user-images.githubusercontent.com/9639873/236866935-dadcf9ea-c93e-459d-8ba0-1511dfaa085f.mov

• Option 4:

https://user-images.githubusercontent.com/9639873/236866976-94e2f4d9-1538-4bcf-8735-1bc9d5cba221.mov

https://user-images.githubusercontent.com/9639873/236866991-a129fb34-9df5-45d8-b606-d1dfa7ea7c91.mov

cc @tgolen

[tgolen]

Thanks! I like option 4 the best out of those two. The delay in option 1 is VERY noticeable, unfortunately.

@Expensify/design I'd love to get some of your thoughts an opinions on this issue.

[shawnborton]

Hmm Option 4 does not feel very smooth to me, is there any way to avoid the growing/shrinking effect that is happening there? Basically, this part feels very janky:

Why can't we just keep that view at full height when we navigate back to it?

[fedirjh]

@hoangzinh Can you please post a video of Android / Chrome as well

[hoangzinh]

@fedirjh it seems my option 4 with set flexBasic doesn't work in Android/Chrome.

[hoangzinh]

@fedirjh Have you had experience with window height (more specifically, it's props.windowHeight in HOC withWindowDimensions) of Android / Chrome before?

[fedirjh]

@hoangzinh I'm not sure I understand the specific context of your question. If you could please provide more details about what you're trying to accomplish or any issues you're facing, I or anyone from the team can give you more targeted assistance.

I should note that on the web, props.windowHeight can be a little misleading, as it actually refers to the height of the window's viewport, rather than the entire window height. https://github.com/Expensify/react-native-web/blob/ccfd936f274ca2105745f9edbbb4aad80725e181/packages/react-native-web/src/exports/Dimensions/index.js#L65-L66

If you have any further questions or concerns, please don't hesitate to ask. Additionally, I encourage you to ask for more context in our Slack channel for future questions, as this will help ensure that everyone on the team can provide you with the most accurate and helpful information possible.

[hoangzinh]

@fedirjh I'm really sorry when made an unclear question. I asked this question because you asked to post video for Android/Chrome so I thought you have experienced with it before so I would like to confirm something.

Btw, I got an answer via comment above already. Thanks Fedirjh

[hoangzinh]

I couldn't think a better solution. But I have a last idea and put it into my proposal here https://github.com/Expensify/App/issues/17866#issuecomment-1537286009

Look forward to hearing feedbacks from team.

[fedirjh]

@hoangzinh Thanks for the update , your solution make sense to me. I haven’t got the time to test it today. will do it tomorrow.

[melvin-bot[bot]]

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

[Christinadobrzyn]

Hey @fedirjh, Just checking to see if you've had a chance to review @hoangzinh's proposal or if you'd like to see more? Thanks!

[fedirjh]

Reviewing shortly

[fedirjh]

@hoangzinh The results are not promising 😞 , On chrome web there is still a janky animation , on android native , the app layout is broken.

Chrome mWeb	Native Android

[melvin-bot[bot]]

@tgolen @fedirjh @Christinadobrzyn this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[fedirjh]

We don’t have any satisfactory proposal yet, awaiting more proposals

[melvin-bot[bot]]

Thanks for the heads up @fedirjh I'll increase the job price to get more proposals. Upwork job price has been updated to $4000

[Ollyws]

Proposal

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

The request money screen jumps when we go back with the keyboard open

What is the root cause of that problem?

They keyboard reduces the size of the viewport when it's open, suddenly expanding it when it closes which resizes the components.

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

In MoneyRequestModal we can use onLayout to assign the height of ScreenWrapper (a child view would also work) to a state variable (at this point the keyboard will be closed). We can then use this state variable to set the minHeight of ScreenWrapper when certain conditions are met (in this case DeviceCapabilities.canUseTouchScreen() && currentStep === Steps.MoneyRequestAmount;)

The onLayout will need an early return if we have already assigned the state variable, something like:

onLayout={(event) => {
    if (screenWrapperHeight) {
        return;
    }
    setScreenWrapperHeight(event.nativeEvent.layout.height);
}}

I suggest we apply this to ScreenWrapper and add onLayout={this.props.onLayout} to the highest View in the heirarchy there, and assign {minHeight: this.props.minHeight} to the KeyboardAvoidingView

The result looks like this:

https://github.com/Expensify/App/assets/11609254/4beb6c45-734d-4436-8ab3-b6f8655be507

What alternative solutions did you explore? (Optional)

I have explored another solution using only CSS:

Firstly we will need to set a minHeight on this view (I'm using 200px here)

Then we need to set a flexWrap: 'wrap-reverse' style on this view

This will give us a dynamic where when the window is too small this happens:

https://github.com/Expensify/App/assets/11609254/5e65774c-65f5-45af-9c8a-7828a45bfd7a

Which leads to a much smoother transition when the keyboard is closed:

https://github.com/Expensify/App/assets/11609254/4369e427-eb3e-4768-bab2-f61e29ba2a1a

In this example I'm using a fixed minHeight of 200px, as you can see there is a little bump upward when it is resized. If we wanted to completely eliminate that we could measure the height of the textInput container view beforehand as use that as the minHeight.

[rojiphil]

Proposal

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

After pressing the back arrow to "Send/Request money" screen, the keyboard is shown for a short moment along with the Numberpad until the keyboard is dismissed.

What is the root cause of that problem?

The root cause of the problem is that we are not waiting for the keyboard to be dismissed while navigating back to the previous step.

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

For complete code reference/testing, please refer to the code changes in my test branch here.

We can solve the problem in the following way.

1) Before going away from the screen, we can ensure that the keyboard is dismissed and, then, perform navigation: We already have a beforeRemove event listener where we are already dismissing the keyboard. We can prevent the navigation until interactions are done. Also, we can implement a ScreenWrapper context which can hold the state isHideKeyboard. The TextInput can useContext and disable editable prop to trigger keyboard dismissal. One of the above approaches or both can be used. I prefer disabling 'editable' prop as it is a generic approach for all platforms.

Main changes in src/components/ScreenWrapper/index.js

-            this.beforeRemoveSubscription = this.props.navigation.addListener('beforeRemove', () => {
+            this.beforeRemoveSubscription = this.props.navigation.addListener('beforeRemove', (e) => {
                if (!this.props.isKeyboardShown) {
                    return;
                }
                Keyboard.dismiss();
+                // Set the screen wrapper context provider's value. 
+                this.setState({isHideKeyboard: true});
+                // Prevent default behavior of leaving the screen
+                e.preventDefault();
+                const interactionManager = InteractionManager.runAfterInteractions;
+                interactionManager().then(() => {
+                    this.props.navigation.dispatch(e.data.action);
                });

+            <ScreenWrapperContext.Provider
+                value={{
+                    isHideKeyboard: this.state.isHideKeyboard,
+                }}
+            >

Main Changes in src/components/TextInput/BaseTextInput.js

+    const screenWrapperContext = useContext(ScreenWrapperContext);
+    const [isHideKeyboard, setHideKeyboard] = useState(false);

+    useEffect(() => {
+        console.log('screenWrapperContext update in basetextinput');
+        if (screenWrapperContext && screenWrapperContext.isHideKeyboard !== isHideKeyboard) {
+            console.log('setHideKeyboard[' + screenWrapperContext.isHideKeyboard + ']');
+            setHideKeyboard(screenWrapperContext.isHideKeyboard);
+        }
+    }, [screenWrapperContext]);

-                                editable={isEditable}
+                                editable={isEditable && !isHideKeyboard}

2) Currently, withKeyboardState cannot identify the presence/absence of keyboard on mobile browsers. isKeyboardShown can be always set for mobile browsers. Alternatively, we can also use the window dimensions and identify the presence/absence of keyboard.

Changes in src/components/withKeyboardState.js

+        // Reference:
+        //      https://github.com/necolas/react-native-web/blob/master/packages/react-native-web/src/exports/Keyboard/index.js
+        //      https://github.com/necolas/react-native-web/blob/master/packages/react-native-web/src/modules/dismissKeyboard/index.js
+        // react-native-web does not have support for Keyboard listeners.
+        // Further, Keyboard.dismiss only applies if there is currently focused text input field.
+        // So, it is perfectly safe to set isKeyboardShown for mobile browsers.
+        if (Browser.isMobile()) {
+            this.setState({isKeyboardShown: true});
+        }

Result Videos

Android/Native

https://github.com/Expensify/App/assets/3004659/0db869d0-b03f-4456-952e-286f54c1227e

Android/Chrome

https://github.com/Expensify/App/assets/3004659/40a306a1-df4c-4f44-92ed-1201a4973f47

What alternative solutions did you explore? (Optional)

[fedirjh]

@Ollyws I believe your solution is the same as @hoangzinh alternative solution , but with different implementation, from your recording , there is still some janky animation

[fedirjh]

@rojiphil React Native Web does not have a Keyboard. It is a mock of the React Native API, which can be found at https://github.com/necolas/react-native-web/blob/master/packages/react-native-web/src/exports/Keyboard/index.js. Therefore, the solution you proposed is not feasible.

[Ollyws]

@fedirjh Thanks for the comment. When you say "janky" you're referring to the fact that the first three buttons are rendered for a second with a blank box underneath? I've come up with another CSS-only solution which eliminates this, it's added to the 'Alternative solutions' section of my proposal.

[rojiphil]

@fedirjh Thanks for your feedback. As you pointed out, we cannot depend purely on Keyboard to resolve this. I have further updated the proposal here. Please review and let me know your feedback.

[fedirjh]

When you say "janky" you're referring to the fact that the first three buttons are rendered for a second with a blank box underneath?

The View is rendered before the keyboard is fully visible and it is noticeable.

Your alternative solution looks promising. We sill have another kind of janky animation, can we fix that ?

[fedirjh]

we should ensure that the keyboard is dismissed before navigating to the previous step.

@rojiphil This solution was suggested by @hoangzinh before , check option 1 in https://github.com/Expensify/App/issues/17866#issuecomment-1538598227 . The delay was very noticeable for that option.

[Ollyws]

@fedirjh Are you referring to the scanline distortion? That's just artifacting from my emulator it won't appear on a real device. (or possibly on your emulator)