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-03-24] $500 Keyboard is hidden after user send message with send button in mobile web - reported by @K4tsuki #7798

Closed
mvtglobally opened this issue Feb 17, 2022 · 29 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@mvtglobally
Copy link

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. Navigate to any room in mobile web
  2. Send a message by pressing send button
  3. Onscreen keyboard disappear / hidden

Expected Result:

Onscreen keyboard stays visible like in native apps

Actual Result:

Onscreen keyboard is hidden/disappears

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Mobile Web

Version Number: 1.1.39-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

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

View all open jobs on GitHub

@mvtglobally mvtglobally added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Feb 17, 2022
@MelvinBot
Copy link

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

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Feb 17, 2022
@puneetlath puneetlath removed their assignment Feb 17, 2022
@puneetlath puneetlath added the External Added to denote the issue can be worked on by a contributor label Feb 17, 2022
@MelvinBot
Copy link

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

@laurenreidexpensify
Copy link
Contributor

@MelvinBot
Copy link

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

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 17, 2022
@MelvinBot
Copy link

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

@roryabraham
Copy link
Contributor

I have a feeling this might be more trouble than it's worth. Dealing with the virtual keyboard's disobedience on mobile safari is always a pain.

@laurenreidexpensify
Copy link
Contributor

@K4tsuki did you have a proposal on how to fix this?

@K4tsuki
Copy link
Contributor

K4tsuki commented Feb 24, 2022

@laurenreidexpensify currently I don't have proposal for this issue.

@laurenreidexpensify laurenreidexpensify changed the title Keyboard is hidden after user send message with send button in mobile web - reported by @K4tsuki $500 Keyboard is hidden after user send message with send button in mobile web - reported by @K4tsuki Mar 4, 2022
@laurenreidexpensify
Copy link
Contributor

Increased pricing to $500

@MelvinBot MelvinBot removed the Overdue label Mar 4, 2022
@mollfpr
Copy link
Contributor

mollfpr commented Mar 9, 2022

Proposal

I notice that the issue is the default behavior of the browser. So what I'm gonna do add this.textInput.focus() to onFocus props of TouchableOpacity.

<TouchableOpacity
style={[
styles.chatItemSubmitButton,
this.state.isCommentEmpty ? styles.buttonDisable : styles.buttonSuccess,
]}
onPress={this.submitForm}
underlayColor={themeColors.componentBG}
disabled={this.state.isCommentEmpty || isBlockedFromConcierge || isArchivedChatRoom}
hitSlop={{
top: 3, right: 3, bottom: 3, left: 3,
}}

+ onFocus={() => this.textInput.focus()}

This will make re-focus the text input after the button is pressed. Also, this will fix text input lost focus in Web.

mWeb Chrome

RPReplay_Final1646819206.MP4

mWeb Safari

RPReplay_Final1646820212.MP4

@parasharrajat
Copy link
Member

parasharrajat commented Mar 9, 2022

Interesting solution @mollfpr. It seems to work well on the mobile Web. But it will cause problem on the web. When a user is focusing on the send button via tab it will move it back to the composer.

@mollfpr
Copy link
Contributor

mollfpr commented Mar 9, 2022

Nice catch @parasharrajat . There’s another approach. Adding ‘this.textInput.focus()’ inside ‘submit’ also work, but there’s some delay for the focus applied to text input.

@parasharrajat
Copy link
Member

Yeah, that delay is bad as it will close the keyboard and reopen it causing layout shifts.

@parasharrajat
Copy link
Member

parasharrajat commented Mar 9, 2022

I do have a proposal too cc: @roryabraham

Proposal.

  1. We can preventDefault mouseDown event to prevent the focus from being lost from Composer. test here

On Submit button add an event

				<TouchableOpacity
                      ....
                      onPress={this.submitForm}
					  onMouseDown={e => e.preventDefault()}
					  ....

then in submitForm reset the selection to 0 so that cursor is reset as well at the last line.

this.setTextInputShouldClear(true);
}

this.textInput.setNativeProps({selection: {start: 0, end: 0}});

@mollfpr
Copy link
Contributor

mollfpr commented Mar 9, 2022

@parasharrajat I think you got this 😉

@roryabraham
Copy link
Contributor

I can't validate this atm because of https://expensify.slack.com/archives/C01GTK53T8Q/p1646964936524219

@roryabraham
Copy link
Contributor

@parasharrajat I managed to test out your solution on a physical device and it worked well 👍

LGTM

@roryabraham roryabraham removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 11, 2022
@roryabraham
Copy link
Contributor

I just removed the Help Wanted label and will wait for a PR. @laurenreidexpensify feel free to hire @parasharrajat for this on upwork.

@roryabraham
Copy link
Contributor

PR merged – as a reminder @laurenreidexpensify since @parasharrajat submitted the PR to fix this issue he's not eligible for the C+ reward for the issue as well, as stated our C+ guidelines:

If C+ wants to fix an issue they’re assigned to they will only be compensated for the fix ($0 for the C+ review) and they won’t have another C+ assigned to review. The CME assigned to the issue will review proposals fairly, not favoring the C+.

@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Mar 17, 2022
@botify botify changed the title $500 Keyboard is hidden after user send message with send button in mobile web - reported by @K4tsuki [HOLD for payment 2022-03-24] $500 Keyboard is hidden after user send message with send button in mobile web - reported by @K4tsuki Mar 17, 2022
@botify
Copy link

botify commented Mar 17, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.43-2 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-03-24. 🎊

@mallenexpensify
Copy link
Contributor

Taking over CM to manage payments. @parasharrajat , can you apply here plz https://www.upwork.com/jobs/~012a764a955a9354d3

@parasharrajat
Copy link
Member

@mallenexpensify Applied with correct terms. Thanks.

@botify botify removed the Weekly KSv2 label Mar 23, 2022
@MelvinBot MelvinBot added the Daily KSv2 label Mar 23, 2022
@mallenexpensify
Copy link
Contributor

Paid @parasharrajat $500 for the fix and @K4tsuki $250 for reporting. Thanks!

@MonilBhavsar
Copy link
Contributor

Since the linked PR caused a small regression, @parasharrajat could you please follow the steps mentioned in the doc here and fill the RCA doc. It will be helpful to learn something and improve our process.
I have added a row in the RCA sheet, feel free to update it.

@parasharrajat
Copy link
Member

parasharrajat commented May 19, 2022

Thanks, @MonilBhavsar but I wasn't C+ on this issue. I solved with a PR as a contributor. So IMO, the C+ assigned on the new issue should proceed with the steps. cc: @mananjadhav. Please let me know otherwise whatever is the process.


On a side note, I tried to solve it but didn't find any solution.

@mallenexpensify
Copy link
Contributor

@parasharrajat from the main C+ doc

Identify the initial/root/core/offending issue and PR that was responsible. Add “Root GH issue link” to the sheet and add the C+ from the root issue as “Person leading”.

Would that not be you?

@parasharrajat
Copy link
Member

Ok, I will do that.

@MonilBhavsar
Copy link
Contributor

Thank you! 🙌

@melvin-bot
Copy link

melvin-bot bot commented Sep 14, 2022

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

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 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests