fix android keyboard issue #13106

0xmiros · 2022-11-28T21:58:03Z

Details

remove KeyboardAvoidingView in android

Fixed Issues

$ #11087
$ #13111
PROPOSAL: #11087 (comment)

Tests

Scenario 1:

Login with any account
Go to Setting -> Profile > Add phone number
(optional) Click on Phone number field if not auto focused (auto-focus should always happen on native platforms)
Click on Password field
Verify that Send validation button floats above keyboard and is not hidden

Scenario 2:

Login with any account
Go to any chat room
Click on composer and start typing
Verify that composer is not hidden by keyboard

Verify that no errors appear in the JS console

Offline tests

Scenario 1:

Login with any account
Disable network
Go to Setting -> Profile > Add phone number
(optional) Click on Phone number field if not auto focused (auto-focus should always happen on native platforms)
Click on Password field
Verify that Send validation button floats above keyboard and is not hidden

Scenario 2:

Login with any account
Go to any chat room
Disable network
Click on composer and start typing
Verify that composer is not hidden by keyboard

QA Steps

Scenario 1:

Login with any account
Go to Setting -> Profile > Add phone number
(optional) Click on Phone number field if not auto focused (auto-focus should always happen on native platforms)
Click on Password field
Verify that Send validation button floats above keyboard and is not hidden

Scenario 2:

Login with any account
Go to any chat room
Click on composer and start typing
Verify that composer is not hidden by keyboard

Verify that no errors appear in the JS console

PR Author Checklist

PR Reviewer Checklist

The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

msafari.mp4

Desktop

desktop.mov

iOS

ios.mp4

Android

android.mp4

melvin-bot · 2022-11-28T21:58:09Z

@pecanoro @Santhosh-Sellavel One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

roryabraham · 2022-11-29T05:31:33Z

@tgolen didn't we get rid of our own KeyboardAvoidingView in another recent PR?

Edit: I see there's more context on this PR in slack here

github-actions · 2022-11-29T12:02:40Z

⚠️ ⚠️ Heads up! This pull request has the CP Staging label ⚠️ ⚠️
If you applied the CP Staging label before the PR was merged, the PR will be be immediately deployed to staging even if the open StagingDeployCash deploy checklist is locked.
However if you applied the CP Staging after the PR was merged it's possible it won't be CP'ed automatically. If you need it to be CP'ed to staging, tag a member of @Expensify/mobile-deployers to CP it manually, otherwise you can wait for it to go out with the next deploy.

mountiny · 2022-11-29T12:02:51Z

Adding CP Staging label since this PR should fix a deploy blocker

Julesssss · 2022-11-29T12:08:37Z

src/components/KeyboardAvoidingView/index.ios.js

@@ -0,0 +1,26 @@
+/*
+ * This is a KeyboardAvoidingView only enabled for ios && disabled for all other platforms


This is a KeyboardAvoidingView only enabled for ios && disabled for all other platforms

Could we make this a bit more readable (similar to this):

The KeyboardAvoidingView is only used on ios

Julesssss

These PR tests look good, next step is to test against all the listed PRs.

0xmiros · 2022-11-29T12:19:45Z

I suggest to link all fixing issues in PR description.
For now, I only linked $ #11087

Julesssss

I tested all the linked issues. With the exception of #10483 I can confirm this has resolved the problems. Great work here

src/components/KeyboardAvoidingView/index.js

mountiny · 2022-11-29T12:57:55Z

@0xmiroslav you can link all the issues which are:

not linked as help Wanted - we need to make sure the proposal flow has been followed - unless issue is on HOLD
you add the testing and QA steps necessary to confirm those issues have been fixed

Julesssss · 2022-11-29T13:01:51Z

src/pages/settings/AddSecondaryLoginPage.js

@@ -107,7 +107,7 @@ class AddSecondaryLoginPage extends Component {
                    onBackButtonPress={() => Navigation.navigate(ROUTES.SETTINGS_PROFILE)}
                    onCloseButtonPress={() => Navigation.dismissModal()}
                />
-                <ScrollView style={styles.flex1} contentContainerStyle={styles.p5}>
+                <ScrollView style={styles.flex1} contentContainerStyle={styles.p5} keyboardShouldPersistTaps="handled">


Why is this necessary?

+1 it would be nice to leave a comment for this

I believe I've seen this before when the intention is to allow someone to tap outside of an input so that the input will blur and the keyboard will dismiss. This was the case for the old Login form before I changed it in this PR: #12848

This is to prevent keyboard randomly hidden when switching focus on phone/password fields

comment will be added in new commit

Julesssss

Just to confirm what we're doing here, we're removing the android specific ScreenWrapper component which is now unnecessary?

Julesssss · 2022-11-29T13:57:39Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- iOS / native
- Android / native
- iOS / Safari
- Android / Chrome
- MacOS / Chrome
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product was added in all src/languages/* files
- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

// Due to cache, the site is in light mode

Desktop

iOS

![Simulator Screen Shot - iPhone 14 Pro - 2022-11-29 at 13 30 10](https://user-images.githubusercontent.com/10736861/204548520-fba020e7-4a59-4b1e-b635-47fca721dbef.png)

Android

![AndroidPhoneNumber](https://user-images.githubusercontent.com/10736861/204548427-e517b7e7-32f8-48b3-9715-d2cdef96f881.png)

Julesssss · 2022-11-29T14:03:32Z

Please review as this resolves a deploy blocker @pecanoro @luacmartins @Santhosh-Sellavel

mountiny

LGTM, testing on Android

mountiny · 2022-11-29T14:19:41Z

src/pages/settings/AddSecondaryLoginPage.js

@@ -107,7 +107,7 @@ class AddSecondaryLoginPage extends Component {
                    onBackButtonPress={() => Navigation.navigate(ROUTES.SETTINGS_PROFILE)}
                    onCloseButtonPress={() => Navigation.dismissModal()}
                />
-                <ScrollView style={styles.flex1} contentContainerStyle={styles.p5}>
+                <ScrollView style={styles.flex1} contentContainerStyle={styles.p5} keyboardShouldPersistTaps="handled">


+1 it would be nice to leave a comment for this

mountiny

Test 1

Chat report keyboard

The emoji picker also seems to work but I assume this is very android version specific

We can ask Applause to retest all of those on staging once deployed

mountiny · 2022-11-29T14:40:42Z

@tgolen tagging you for a review as you expressed such desire in Slack

src/components/KeyboardAvoidingView/index.js

Santhosh-Sellavel · 2022-11-29T19:21:23Z

@0xmiroslav Please add test steps for this issue #13111 also.

0xmiros · 2022-11-29T19:48:31Z

@0xmiroslav Please add test steps for this issue #13111 also.

done

Santhosh-Sellavel · 2022-11-29T20:00:29Z

All looks good tests well!

Issues noticed while testing

Failing for mWeb Safari

Verify that Send validation button floats above the keyboard and is not hidden

This is failing for mWeb - Safari - but this is not a blocker for this issue. ✅

Keyboard glitch on Android

Also found a weird glitch in the keyboard while opening & closing add phone number page on android . This is not introduced here

Screen.Recording.2022-11-30.at.1.27.05.AM.mov

Autofill email in add phone number screen

Email & password is auto-filled on Add phone number page. This is reported already by marc on slack ✅

What should we do about the keyboard glitch?
cc: @luacmartins @Julesssss @tgolen

Santhosh-Sellavel

Looks good and tested well for me!

comments addressed

luacmartins · 2022-11-29T20:16:23Z

@Santhosh-Sellavel has this keyboard glitch been reported anywhere? If not, let's report it and tackle it in a separate issue.

luacmartins · 2022-11-29T20:17:06Z

I dismissed @tgolen review since we talked 1:1 and his only concern was omitting the KAV props, which we addressed. Gonna merge to continue with the deploy.

fix android keyboard issue (cherry picked from commit d5a293d)

…-13106 🍒 Cherry pick PR #13106 to staging 🍒

pecanoro · 2022-11-29T20:26:57Z

@luacmartins You could have waited a few minutes for my final review though so at least I could get that contribution after reviewing the proposal and originally discussing this issue 😢

OSBotify · 2022-11-29T21:28:34Z

🚀 Cherry-picked to staging by @luacmartins in version: 1.2.33-4 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

OSBotify · 2022-11-30T04:01:31Z

🚀 Deployed to production by @luacmartins in version: 1.2.33-7 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2022-11-30T04:26:04Z

🚀 Deployed to production by @luacmartins in version: 1.2.33-7 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-06-15T01:40:04Z

🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 1.3.28-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

OSBotify · 2023-06-16T18:57:49Z

🚀 Deployed to production by https://github.com/AndrewGable in version: 1.3.28-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

fix android keyboard issue

b9da553

0xmiros requested a review from a team as a code owner November 28, 2022 21:58

melvin-bot bot requested review from pecanoro and Santhosh-Sellavel and removed request for a team November 28, 2022 21:58

This was referenced Nov 28, 2022

[$250] Android narrow/low pixel tablets - Chat - The FAB button disappears or is covered by the keyboard in horizontal #12592

Closed

[HOLD for payment 2022-12-08] Android -Chat - The compose field is covered by keyboard #13111

Closed

luacmartins self-requested a review November 29, 2022 03:24

Julesssss assigned 0xmiros Nov 29, 2022

mountiny added the CP Staging label Nov 29, 2022

Julesssss reviewed Nov 29, 2022

View reviewed changes

update KeyboardAvoidingView ios component description

694125a

Julesssss mentioned this pull request Nov 29, 2022

[$250] Android - Compose box shifts to the top when the keyboard opens #12622

Closed

Julesssss previously approved these changes Nov 29, 2022

View reviewed changes

Julesssss reviewed Nov 29, 2022

View reviewed changes

src/components/KeyboardAvoidingView/index.js Outdated Show resolved Hide resolved

Julesssss reviewed Nov 29, 2022

View reviewed changes

mountiny reviewed Nov 29, 2022

View reviewed changes

mountiny previously approved these changes Nov 29, 2022

View reviewed changes

mountiny requested a review from tgolen November 29, 2022 14:40

omit behavior prop in normal view

eb05c69

0xmiros dismissed luacmartins’s stale review via eb05c69 November 29, 2022 19:00

luacmartins reviewed Nov 29, 2022

View reviewed changes

src/components/KeyboardAvoidingView/index.js Outdated Show resolved Hide resolved

omit KAV props from View props

6a16784

Santhosh-Sellavel approved these changes Nov 29, 2022

View reviewed changes

luacmartins approved these changes Nov 29, 2022

View reviewed changes

luacmartins merged commit d5a293d into Expensify:main Nov 29, 2022

OSBotify pushed a commit that referenced this pull request Nov 29, 2022

Merge pull request #13106 from 0xmiroslav/issue-11087

f318342

fix android keyboard issue (cherry picked from commit d5a293d)

OSBotify mentioned this pull request Nov 29, 2022

🍒 Cherry pick PR #13106 to staging 🍒 #13157

Merged

OSBotify added a commit that referenced this pull request Nov 29, 2022

Merge pull request #13157 from Expensify/OSBotify-cherry-pick-staging…

906cc18

…-13106 🍒 Cherry pick PR #13106 to staging 🍒

OSBotify mentioned this pull request Nov 29, 2022

Deploy Checklist: New Expensify 2022-11-28 #13099

Closed

26 tasks

OSBotify mentioned this pull request Nov 30, 2022

Deploy Checklist: New Expensify 2022-11-30 #13180

Closed

32 tasks

pecanoro mentioned this pull request Dec 1, 2022

[HOLD for BZ checklist] [$2000] Android - Add Phone number UI not consistent - reported by @gadhiyamanan #11087

Closed

This was referenced Dec 4, 2022

Android - New Room -The keyboard overlays the "New Room" button #13152

Closed

[HOLD for payment] [$500] Android - Composer is hidden by native emoji picker - Reported by @rushatgabhane #10483

Closed

situchan mentioned this pull request Mar 14, 2023

dismiss keyboard on scroll on manage members page #15903

Merged

55 tasks

OSBotify mentioned this pull request May 2, 2023

Deploy Checklist: New Expensify 2023-05-02 #18315

Closed

OSBotify mentioned this pull request Jun 15, 2023

Deploy Checklist: New Expensify 2023-06-14 #20782

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix android keyboard issue #13106

fix android keyboard issue #13106

0xmiros commented Nov 28, 2022 •

edited

Loading

melvin-bot bot commented Nov 28, 2022

roryabraham commented Nov 29, 2022 •

edited

Loading

github-actions bot commented Nov 29, 2022

mountiny commented Nov 29, 2022

Julesssss Nov 29, 2022

Julesssss left a comment

0xmiros commented Nov 29, 2022

Julesssss left a comment

mountiny commented Nov 29, 2022

Julesssss Nov 29, 2022

mountiny Nov 29, 2022

tgolen Nov 29, 2022

0xmiros Nov 29, 2022

0xmiros Nov 29, 2022

Julesssss left a comment

Julesssss commented Nov 29, 2022 •

edited

Loading

Julesssss commented Nov 29, 2022

mountiny left a comment

mountiny Nov 29, 2022

mountiny left a comment

mountiny commented Nov 29, 2022

Santhosh-Sellavel commented Nov 29, 2022

0xmiros commented Nov 29, 2022

Santhosh-Sellavel commented Nov 29, 2022 •

edited

Loading

Santhosh-Sellavel left a comment

luacmartins commented Nov 29, 2022

luacmartins commented Nov 29, 2022

pecanoro commented Nov 29, 2022 •

edited

Loading

OSBotify commented Nov 29, 2022

OSBotify commented Nov 30, 2022

OSBotify commented Nov 30, 2022

OSBotify commented Jun 15, 2023

OSBotify commented Jun 16, 2023

		@@ -0,0 +1,26 @@
		/*
		* This is a KeyboardAvoidingView only enabled for ios && disabled for all other platforms

fix android keyboard issue #13106

fix android keyboard issue #13106

Conversation

0xmiros commented Nov 28, 2022 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

PR Reviewer Checklist

Screenshots/Videos

melvin-bot bot commented Nov 28, 2022

roryabraham commented Nov 29, 2022 • edited Loading

github-actions bot commented Nov 29, 2022

mountiny commented Nov 29, 2022

Julesssss Nov 29, 2022

Choose a reason for hiding this comment

Julesssss left a comment

Choose a reason for hiding this comment

0xmiros commented Nov 29, 2022

Julesssss left a comment

Choose a reason for hiding this comment

mountiny commented Nov 29, 2022

Julesssss Nov 29, 2022

Choose a reason for hiding this comment

mountiny Nov 29, 2022

Choose a reason for hiding this comment

tgolen Nov 29, 2022

Choose a reason for hiding this comment

0xmiros Nov 29, 2022

Choose a reason for hiding this comment

0xmiros Nov 29, 2022

Choose a reason for hiding this comment

Julesssss left a comment

Choose a reason for hiding this comment

Julesssss commented Nov 29, 2022 • edited Loading

Reviewer Checklist

Screenshots/Videos

Julesssss commented Nov 29, 2022

mountiny left a comment

Choose a reason for hiding this comment

mountiny Nov 29, 2022

Choose a reason for hiding this comment

mountiny left a comment

Choose a reason for hiding this comment

mountiny commented Nov 29, 2022

Santhosh-Sellavel commented Nov 29, 2022

0xmiros commented Nov 29, 2022

Santhosh-Sellavel commented Nov 29, 2022 • edited Loading

Failing for mWeb Safari

Keyboard glitch on Android

Autofill email in add phone number screen

Santhosh-Sellavel left a comment

Choose a reason for hiding this comment

luacmartins commented Nov 29, 2022

luacmartins commented Nov 29, 2022

pecanoro commented Nov 29, 2022 • edited Loading

OSBotify commented Nov 29, 2022

OSBotify commented Nov 30, 2022

OSBotify commented Nov 30, 2022

OSBotify commented Jun 15, 2023

OSBotify commented Jun 16, 2023

0xmiros commented Nov 28, 2022 •

edited

Loading

roryabraham commented Nov 29, 2022 •

edited

Loading

Julesssss commented Nov 29, 2022 •

edited

Loading

Santhosh-Sellavel commented Nov 29, 2022 •

edited

Loading

pecanoro commented Nov 29, 2022 •

edited

Loading