[HOLD #11768] [$1000] Android - Split Bill - Keyboard is not dismissed for a brief moment after changing currency

[kavimuru]

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

---

Actions Performed:

1. Go to any chat
2. Go To Split Bill
3. Change Currency

Expected Result:

Keyboard dismissed

Actual Result:

Keyboard Appears For A brief Second.

Workaround:

unknown

Platform:

Where is this issue occurring?

• Android

Version Number: 1.2.37-2
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Record_2022-12-08-05-56-42.mp4

az_recorder_20221208_181448.mp4

Expensify/Expensify Issue URL:
Issue reported by: @syedsaroshfarrukhdot
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1670461227037229

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0170d39fa04a453e03
• Upwork Job ID: 1601056856164556800

[melvin-bot]

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

[syedsaroshfarrukhdot]

Proposal:-

In IOUCurrencySelection.js we need to pass Keyboard.dismiss(); in confirmCurrencySelection function.

diff --git a/src/pages/iou/IOUCurrencySelection.js b/src/pages/iou/IOUCurrencySelection.js
index b7affe898..6525a205f 100644
--- a/src/pages/iou/IOUCurrencySelection.js
+++ b/src/pages/iou/IOUCurrencySelection.js
@@ -1,4 +1,5 @@
 import React, {Component} from 'react';
+import { Keyboard } from 'react-native';
 import PropTypes from 'prop-types';
 import {withOnyx} from 'react-native-onyx';
 import _ from 'underscore';
@@ -105,6 +106,7 @@ class IOUCurrencySelection extends Component {
      */
     confirmCurrencySelection(option) {
         IOU.setIOUSelectedCurrency(option.currencyCode);
+        Keyboard.dismiss();
         Navigation.goBack();
     }
 

After Fix:-

Screen.Recording.2022-12-08.at.6.19.56.AM.mov

[JmillsExpensify]

Ok, I can reproduce this.

Screen.Recording.2022-12-08.at.21.29.28.mov

[JmillsExpensify]

Here's my take: This is a pretty cosmetic bug in my opinion, as it doesn't really affect the user experience and the keyboard does dismiss automatically. So I think the value of fixing this big is pretty low.

That said, if we can find a simple solution for this then it'll be worth it. Let's open this up for proposals and see where we get.

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0170d39fa04a453e03

[melvin-bot]

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

[melvin-bot]

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

[JmillsExpensify]

@eVoloshchak We have two proposals above already. While those technically shouldn't have been posted yet, we'll fix that in the future by locking the issue. For now, let's go ahead and review those. Thanks!

[dhairyasenjaliya]

Proposal

• We need to dismiss the keyboard at 2 places at IOUCurrencySelection.js
• As selecting currency as well as closing currency modal has this keyboard has not dismissed the issue
• Previous Proposal does not handle it so adding in this proposal (it seems the old proposal was edited after this proposal to dismiss the keyboard on close modal as well)

    confirmCurrencySelection(option) {
        IOU.setIOUSelectedCurrency(option.currencyCode);
+        Keyboard.dismiss();
        Navigation.goBack();
    }

    render() {
        const headerMessage = this.state.searchValue.trim() && !this.state.currencyData.length ? this.props.translate('common.noResultsFound') : '';
        return (
            <ScreenWrapper>
                <HeaderWithCloseButton
                    title={this.props.translate('iOUCurrencySelection.selectCurrency')}
                    onCloseButtonPress={() => {
+                        Keyboard.dismiss();
                        Navigation.goBack();
                    }}
                />
                <OptionsSelector
                    sections={this.getSections()}
                    onSelectRow={this.confirmCurrencySelection}
                    value={this.state.searchValue}
                    onChangeText={this.changeSearchValue}
                    placeholderText={this.props.translate('common.search')}
                    headerMessage={headerMessage}
                />
            </ScreenWrapper>
        );
    }

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[syedsaroshfarrukhdot]

Updated Proposal

As keyboard dismiss issue presists in multiple occasions when ever Navigation.goBack() is used instead of dismissing keyboard every time we can simply dismiss keyboard in Navigation.js goBack function and it will be handled for all cases.

diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js
index e937e5015..05336e251 100644
--- a/src/libs/Navigation/Navigation.js
+++ b/src/libs/Navigation/Navigation.js
@@ -109,6 +109,7 @@ function goBack(shouldOpenDrawer = true) {
         return;
     }
 
+    Keyboard.dismiss();
     navigationRef.current.goBack();
 }

After Fix:-

Screen.Recording.2022-12-09.at.10.15.16.AM.mov

[s77rt]

Proposal

diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js
index e937e5015..8c484ce1e 100644
--- a/src/libs/Navigation/Navigation.js
+++ b/src/libs/Navigation/Navigation.js
@@ -1,6 +1,5 @@
 import _ from 'underscore';
 import lodashGet from 'lodash/get';
-import {Keyboard} from 'react-native';
 import {DrawerActions, getPathFromState, StackActions} from '@react-navigation/native';
 import Onyx from 'react-native-onyx';
 import Log from '../Log';
@@ -66,7 +65,6 @@ function openDrawer() {
     }
 
     navigationRef.current.dispatch(DrawerActions.openDrawer());
-    Keyboard.dismiss();
 }
 
 /**
diff --git a/src/libs/Navigation/NavigationRoot.js b/src/libs/Navigation/NavigationRoot.js
index 775d89f31..9dba24585 100644
--- a/src/libs/Navigation/NavigationRoot.js
+++ b/src/libs/Navigation/NavigationRoot.js
@@ -1,5 +1,6 @@
 import React from 'react';
 import PropTypes from 'prop-types';
+import {Keyboard} from 'react-native';
 import {NavigationContainer, DefaultTheme, getPathFromState} from '@react-navigation/native';
 import {useFlipper} from '@react-navigation/devtools';
 import Navigation, {navigationRef} from './Navigation';
@@ -60,7 +61,10 @@ const NavigationRoot = (props) => {
                     style={styles.navigatorFullScreenLoading}
                 />
             )}
-            onStateChange={parseAndLogRoute}
+            onStateChange={() => {
+                Keyboard.dismiss();
+                parseAndLogRoute();
+            }}
             onReady={props.onReady}
             theme={navigationTheme}
             ref={navigationRef}

Details

This issue is very similar to this one #11970
I have already proposed a solution there and it will fix this issue as well. So I'm just posting the same proposal here.

Keyboard.dismiss(); should be called at the navigation root and not on Navigation.js goBack() as suggested by @syedsaroshfarrukhdot because this does not cover all the cases.

Maybe we should put this on hold instead?

[JmillsExpensify]

Maybe we should put this on hold instead?

That's an interesting point. though the linked issue is actually on hold for the navigation reboot, so I'm more inclined to also hold this issue on #11768 as well. I'm actually going to preemptively do that, though still welcome thoughts from others. @eVoloshchak @techievivek

[techievivek]

Agree with @s77rt the better solution will be to keep the logic in NavigationRoot.js file. But since we are revamping our navigation logic let's keep this on HOLD and we can re-visit the proposal once we start the implementation phase of the project. Thanks

[JmillsExpensify]

Removed the daily since this will be on hold for a while.

[JmillsExpensify]

Given that we're taking our time on the navigation reboot, I switched out weekly for monthly.

[syedsaroshfarrukhdot]

@JmillsExpensify Am I eligible for reporting bonus ?

[JmillsExpensify]

Sorry I missed your question in this issue, though I believe I answered it in another one. We pay out the reporting bonus when PRs are merged and the regression period has past. So essentially, when a bug is fixed, all contributors are paid out at the same time. Thank you!

[mallenexpensify]

Removed Help Wanted because job is on hold, please add back when it's off hold

[techievivek]

On hold for navigation reboot project.

[0xmiroslav]

This can be closed in favour of recent issues active.

Duplicated issues created:
#15085 (native)
#15270 (mWeb)

[techievivek]

Interesting @0xmiroslav, I just reviewed a similar proposal today for this issue.
CC @narefyev91 can you confirm if we can close this issue in favour of the #15085

[syedsaroshfarrukhdot]

@techievivek Just a thought if we end up fixing the issue in #15085 will i be eligible for reporting bonus ? As i reported this issue initially issue #15085 is a dupe of this issue ?

[MelvinBot]

📣 @syedsaroshfarrukhdot! 📣

Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[syedsaroshfarrukhdot]

Contributor details
Your Expensify account email: syedsaroshfarrukh@gmail.com
Upwork Profile Link: upwork.com/freelancers/~01438157cc0d3d99e2

[MelvinBot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[mallenexpensify]

@syedsaroshfarrukhdot it appears this bug, which you reported, precedes the two you linked so you would be eligible for the bonus.

[syedsaroshfarrukhdot]

@syedsaroshfarrukhdot it appears this bug, which you reported, precedes the two you linked so you would be eligible for the bonus.

Thank you. Will apply on the job when time comes. 😃

[syedsaroshfarrukhdot]

@mallenexpensify as it seems issue was resolved in #15085 and it have been closed. Shouldn't we close this issue as well ?

[JmillsExpensify]

I'm not longer able to reproduce this issue, so I'm going to comment in the linked issue and clear up the reporting bonus.

[syedsaroshfarrukhdot]

@JmillsExpensify I think we can close this issue now as according to this comment everyone agrees this issue is no longer reproducible and fixed.

[syedsaroshfarrukhdot]

@JmillsExpensify bumping it for closing and reporting bonus payment.

[mallenexpensify]

@syedsaroshfarrukhdot can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01d87b154a08dd83c6

[syedsaroshfarrukhdot]

@mallenexpensify Accepted on Upwork

[mallenexpensify]

Paid, sorry it took a minute @syedsaroshfarrukhdot , thanks for the bumps. Think this can be closed now, reopen if neded.

[JmillsExpensify]

Thanks for closing the loop at on this one @mallenexpensify!