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

[$250] Android/IOS -IOU-The cursor disappears after putting app in background in requesting money #9407

Closed
kbecciv opened this issue Jun 12, 2022 · 14 comments
Assignees
Labels
Daily KSv2 Engineering 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 Improvement Item broken or needs improvement.

Comments

@kbecciv
Copy link

kbecciv commented Jun 12, 2022

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 test app
  2. Tap on a contact
  3. Tap on +
  4. Tap on request Money
  5. Put app in background and Open app again

Expected Result:

The cursor indicating where the next number will appear will be seen after opening app

Actual Result:

The indicator disappears and can’t be found when entering amount for Money requested after putting app in background

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • iOS
  • Android

Version Number: 1.1.75.2

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

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

Notes/Photos/Videos: Any additional supporting documentation

Bug5605578_VID-20220611-WA0005.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause -Exploratory

Slack conversation:

View all open jobs on GitHub

@melvin-bot
Copy link

melvin-bot bot commented Jun 12, 2022

Triggered auto assignment to @Gonals (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@kbecciv kbecciv changed the title Android/IOS - -IOU-The cursor disappears after putting app in background in requesting money Android/IOS -IOU-The cursor disappears after putting app in background in requesting money Jun 12, 2022
@Gonals Gonals added Monthly KSv2 Improvement Item broken or needs improvement. External Added to denote the issue can be worked on by a contributor and removed Daily KSv2 labels Jun 13, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2022

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Monthly KSv2 labels Jun 13, 2022
@Gonals
Copy link
Contributor

Gonals commented Jun 13, 2022

Sounds like a good candidate for External!

@bfitzexpensify
Copy link
Contributor

Upwork job post here

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jun 14, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jun 14, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 14, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jun 14, 2022

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

@melvin-bot melvin-bot bot changed the title Android/IOS -IOU-The cursor disappears after putting app in background in requesting money [$250] Android/IOS -IOU-The cursor disappears after putting app in background in requesting money Jun 14, 2022
@dhairyasenjaliya
Copy link
Contributor

Proposal

  • here in IOUAmountPage in componentDidUpdate we are comparing current currency and updated currency if both are the same then we are just returning without focus, each and every time we go to request/send money this condition will remain true it will return without focus so we need to remove the conditional check from componentDidUpdate

componentDidUpdate(prevProps) {
if (this.props.iou.selectedCurrencyCode === prevProps.iou.selectedCurrencyCode) {
return;
}
this.focusTextInput();
}

Results

iOS

iOS.FIx.mp4

Android

android.mp4

Chrome Web

chrome.mov

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 14, 2022
@parasharrajat
Copy link
Member

@dhairyasenjaliya Thanks for the proposal but this is not a good solution. ComponentDidUpdate will fire many times and focus is a blocking operation which will hurt performance. More importantly, that compoenentDidUpdate logic was added specifically for currency change use-case.

@JosueEchandiaAsto
Copy link
Contributor

@parasharrajat

Proposal

Issue

The textField loses focus when the app goes to background.

Fix

Using appstate listeners we can know exactly when it goes from background to foreground without affecting the current logic, nor creating loops.
Docs: https://reactnative.dev/docs/appstate
Commit: 6f988d6

Result

Simulator.Screen.Recording.-.iPhone.13.-.2022-06-14.at.18.22.28.mp4

@usmanghani127
Copy link

Proposal

Issue

componentDidMount() {
if (this.props.disableKeyboard) {
this.appStateSubscription = AppState.addEventListener(
'change',
this.dismissKeyboardWhenBackgrounded,
);
}
// We are manually managing focus to prevent this issue: https://github.com/Expensify/App/issues/4514
if (!this.props.autoFocus || !this.input) {
return;
}
this.input.focus();
}

The AppState listener is unnecessary here, because there is no need to dismiss a keyboard which is already disabled:

showSoftInputOnFocus={!this.props.disableKeyboard}

Fix

By removing this listener and its relevant code (imports, listener remover), this issue can be fixed

Result

Screen.Recording.2022-06-15.at.11.26.08.mov

@liyamahendra
Copy link
Contributor

liyamahendra commented Jun 15, 2022

Proposal

Since componentDidUpdate is called when the app returns to foreground, moving this.focusTextInput(); to the top solves the issue:

componentDidUpdate(prevProps) {
        this.focusTextInput();
        
        if (this.props.iou.selectedCurrencyCode === prevProps.iou.selectedCurrencyCode) {
            return;
        }
    }
fix.mp4

@parasharrajat are you aware about the reason why we have the if check and what problem it solves?

@marcaaron
Copy link
Contributor

Hmm I feel that this is not a real bug or one that needs to be prioritized right now... but maybe I am missing why it matters.

@bfitzexpensify @Gonals thoughts on just closing this?

@Gonals
Copy link
Contributor

Gonals commented Jun 15, 2022

Hmmm. I'd be ok with just closing this. I don't think I would consider it a bug as a user (and certainly not a relevant one)

@bfitzexpensify
Copy link
Contributor

Yes, agreed. OK, closing it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering 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 Improvement Item broken or needs improvement.
Projects
None yet
Development

No branches or pull requests

9 participants