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

[HOLD for payment 2022-09-16] [$250] Changing input field on Mobile App closes the keyboard - reported by @Tushu17 #9234

Closed
mvtglobally opened this issue May 31, 2022 · 64 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Reviewing Has a PR in review

Comments

@mvtglobally
Copy link

mvtglobally commented May 31, 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 mobile App(Ios/Android)
  2. Go to settings > profile
  3. Click/touch on First name
  4. then touch on Last name

Expected Result:

It should not close the keyboard.

Actual Result:

Changing Input field closes the keyboard then user have to touch on the input field again to open the keyboard

Workaround:

unknown

Platform:

Where is this issue occurring?

  • iOS
  • Android

Version Number: 1.1.68-0
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: Any additional supporting documentation

Screen.Recording.2022-04-26.at.1.35.39.AM.mov

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

Job Post https://www.upwork.com/jobs/~01e74f3da7a543363a

View all open jobs on GitHub

@mvtglobally mvtglobally added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels May 31, 2022
@melvin-bot
Copy link

melvin-bot bot commented May 31, 2022

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

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label May 31, 2022
@kevinksullivan
Copy link
Contributor

@Tushu17 hired for reporting.

@melvin-bot melvin-bot bot added the Overdue label Jun 2, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jun 3, 2022

@kevinksullivan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

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

melvin-bot bot commented Jun 3, 2022

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

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

melvin-bot bot commented Jun 3, 2022

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

@melvin-bot melvin-bot bot changed the title Changing input field on Mobile App closes the keyboard - reported by @Tushu17 [$250] Changing input field on Mobile App closes the keyboard - reported by @Tushu17 Jun 3, 2022
@jayeshmangwani
Copy link
Contributor

Proposal

Cause:

We are using a Text component in TextInputLabel and this issue is happening when we press on the Text labels, otherwise, it is working fine, and here we have to disable the touchable of the label so that textinput's touchable can work

Solution:

we can disable the touch events here by

<Animated.Text
onLayout={({nativeEvent}) => {
this.setState({width: nativeEvent.layout.width});
}}
style={[
styles.textInputLabel,
styles.textInputLabelTransformation(
this.props.labelTranslateY,
this.props.labelScale.interpolate({
inputRange: [styleConst.ACTIVE_LABEL_SCALE, styleConst.INACTIVE_LABEL_SCALE],
outputRange: [-(this.state.width - (this.state.width * styleConst.ACTIVE_LABEL_SCALE)) / 2, 0],
}),
this.props.labelScale,
),
]}

+pointerEvents="none"

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

Proposal

Problem

The issue is when either of Firstname or Lastname Input field is focused with keyboard visible, if you try to focus on the other input field, the keyboard which was visible becomes invisible.

Solution

The Firstname and Lastname textfields are wrapped inside a ScrollView. The ScrollView has a props keyboardShouldPersistTaps which can be set to solve this issue.

Fix is to change the below line:

<ScrollView style={styles.flex1} contentContainerStyle={styles.p5}>

to

<ScrollView style={styles.flex1} contentContainerStyle={styles.p5} keyboardShouldPersistTaps='handled'>

Below is the screencast showing the fix:

fix.mp4

@rushatgabhane
Copy link
Member

rushatgabhane commented Jun 6, 2022

Hmm, making the page compatible with Form should solve this issue too.

keyboardShouldPersistTaps="handled"

@marcochavezf can you reproduce this issue on main? I could repro this issue 2 days ago, but now it seems to be fixed!

@jayeshmangwani
Copy link
Contributor

@rushatgabhane I am able to reproduce this issue on main, please try to press on the label of the textinput you will see this issue please check this video for the latest main branch, and this is my proposal

9234.mov

@melvin-bot melvin-bot bot added the Overdue label Jun 8, 2022
@rushatgabhane
Copy link
Member

App is crashing on main for me. I need some time to verify the proposal.
https://expensify.slack.com/archives/C01GTK53T8Q/p1654718949591419

@melvin-bot melvin-bot bot removed the Overdue label Jun 8, 2022
@rushatgabhane
Copy link
Member

rushatgabhane commented Jun 8, 2022

please try to press on the label of the textinput you will see this issue

@jayeshmangwani thanks for helping me reproduce this issue!

This issue is reproducible on all pages that aren't using forms yet. Can you please post an updated proposal to fix this issue for all pages. (a few egs: AddSecondaryLoginPage, NewRoomPage etc)

@liyamahendra
Copy link
Contributor

@rushatgabhane though @jayeshmangwani's solution fixes the issue, it doesn't address the root cause which is the ScrollView. The ScrollView will always cause this issue to happen unless the keyboardShouldPersistTaps prop is used.

Let's say you patch the code using @jayeshmangwani's solution now, and down the line a new custom composite control is used / added. Then this issue will again pop-up.

@rushatgabhane
Copy link
Member

rushatgabhane commented Jun 9, 2022

This issue is reproducible on all pages that aren't using forms yet. Can you please post an updated proposal to fix this issue for all pages. (a few egs: AddSecondaryLoginPage, NewRoomPage etc)

@liyamahendra I'm so sorry. This comment was meant for you. I forgot to tag you.
I def like your proposal better. Please submit an updated proposal which fixes this issue for all such pages

@marcochavezf marcochavezf added the Reviewing Has a PR in review label Jul 11, 2022
@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Aug 4, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 4, 2022

This issue has not been updated in over 15 days. @liyamahendra, @marcochavezf, @kevinksullivan, @rushatgabhane eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@mallenexpensify mallenexpensify self-assigned this Aug 29, 2022
@mallenexpensify
Copy link
Contributor

PR is actively being worked on #9441

@liyamahendra
Copy link
Contributor

I've already received payment for this issue as it got auto approved by Upwork - JFYI.
Is there anything I need to do - asking as this is among the very first issues I've worked with Expensify.

@liyamahendra
Copy link
Contributor

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Monthly KSv2 labels Sep 9, 2022
@melvin-bot melvin-bot bot changed the title [$250] Changing input field on Mobile App closes the keyboard - reported by @Tushu17 [HOLD for payment 2022-09-16] [$250] Changing input field on Mobile App closes the keyboard - reported by @Tushu17 Sep 9, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 9, 2022

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

If no regressions arise, payment will be issued on 2022-09-16. 🎊

@kevinksullivan
Copy link
Contributor

Hm I'm not familiar with the auto approval feature but I do see it was paid. @mallenexpensify are you familiar with that?

@mallenexpensify
Copy link
Contributor

@kevinksullivan , It looks like it might have been because a milestone was created vs hiring for fixed price (I've played with this a bunch lately, it's def a semi-common issue). @liyamahendra 's contract in Upwork was ended too.
image

Looks like @Tushu17 was paid today for reporting, I ended their contract.
image

And... just paid @rushatgabhane and ended contract (to make this easier to manage going forward.

Now, let's hope there are no regressions within the next three days 🤞 . We can close this on 9/16 without taking more actions if no regressions

@liyamahendra
Copy link
Contributor

@mallenexpensify As a part of this issue, the Form refactor was done for which a bounty was agreed here. Just a gentle reminder note about this please.

cc: @luacmartins

@mallenexpensify
Copy link
Contributor

@liyamahendra does that mean you're due another $250? If so, @luacmartins or @rushatgabhane can you confirm and I'll add the $250 to the job?

@rushatgabhane
Copy link
Member

rushatgabhane commented Sep 13, 2022

@mallenexpensify i don't think so. No additional payments due

@luacmartins' comment clearly says that all form refactors are $250. And this issue was a form refactor too. Nothing less, nothing more.

@liyamahendra
Copy link
Contributor

@liyamahendra does that mean you're due another $250?

@mallenexpensify Yes, that's what I understood from @luacmartins' comment.

@liyamahendra
Copy link
Contributor

@luacmartins can you please confirm?

@luacmartins
Copy link
Contributor

@liyamahendra @mallenexpensify The bounty for the issue is $250. If a $250 payment was already made, which seems to be the case, no further payments are due anymore.

@liyamahendra
Copy link
Contributor

@luacmartins did you mean bounty as in payment above the price for the actual issue?

@luacmartins
Copy link
Contributor

@liyamahendra No, the bounty is the price we pay for fixing the issue, $250 in this case. Sorry if this was not clear for you.

@liyamahendra
Copy link
Contributor

Well, I can just express my disappointment only then. Extremely disappointed!

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Sep 15, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2022

@liyamahendra, @marcochavezf, @kevinksullivan, @mallenexpensify, @rushatgabhane Whoops! This issue is 2 days overdue. Let's get this updated quick!

@luacmartins
Copy link
Contributor

@kevinksullivan @mallenexpensify are we good to close this issue? I don't think there was any regression.

@mallenexpensify
Copy link
Contributor

Yes, thanks all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

8 participants