[$1000] Android - Keyboard is not automatically displayed when editing a comment

[isagoico]

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 a conversation
2. Edit a comment

Expected Result:

Keyboard should automatically display so the user can start editing right away

Actual Result:

Keyboard doesn't come up and user has to tap the text box again.

Workaround:

Tapping the text box makes the keyboard appear.

Platform:

Where is this issue occurring?

• Android

Version Number: 1.1.4-0

Reproducible in staging?: yes
Reproducible in production?: yes

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

Notes/Photos/Videos: Any additional supporting documentation
Issue is ONLY reproducible on Android. iOS and mWeb are working as expected.

editcomment.mp4

Expensify/Expensify Issue URL:

Issue reported by: @Santhosh-Sellavel
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1633369460395200

View all open jobs on GitHub

[MelvinBot]

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

[MelvinBot]

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

[parasharrajat]

I think it we disabled the keyboard earlier. This is supposed to be intended functionality but I don't know there has been a new discussion about it recently.

When we edit a comment, edit box will scroll down towards the composer and we decided not to open the keyboard due to this. But if keyboard is still opening on other devices then yes this is a real issue.

[laurenreidexpensify]

https://www.upwork.com/jobs/~0124c6af758e2ebe94

[MelvinBot]

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

[nitin145]

(External)

You have to request focus on the edit input text field by calling the requestFocus{} function of edit text class.

If this do not works, you will try the inputMethodManager class function to open the keyboard. Below is the code for the same -

val imm =
            getSystemService(Context.INPUT_METHOD_SERVICE) as InputMethodManager
        imm.showSoftInput(v, 0)

[Santhosh-Sellavel]

@nitin145 this is a react native project. Not an android native project!

[hiennguyen92]

I think, maybe it's because some element has requested focus. so the keyboard was closed. and we can't show the keyboard due to loss of focus.

[ddikodroid]

autoFocus props executed only on the first mount

[kadiealexander]

Please refer to this post for updated information on the n6 hold, we've raised the bonus to $250 for all issues where a PR is created before the N6 hold is lifted.

[nikdev15]

The autoFocus prop does not work if there were any animations running on the screen. The below code will shift the focus after the interactions are run, to the <TextInput> field.

const inputRef = useRef(null);

useEffect(() => {
// Must run after animations for keyboard to automatically open
InteractionManager.runAfterInteractions(() => {
if (inputRef?.current) {
inputRef.current.focus();
}
});
}, [inputRef])

[NikkiWines]

When we edit a comment, edit box will scroll down towards the composer and we decided not to open the keyboard due to this. But if keyboard is still opening on other devices then yes this is a real issue.

Interestingly, I can reproduce the issue on my iOS sim and Android sim (keyboard does not show) but not on my physical iOS device (keyboard does show)

I asked @sonialiap to try on her Android device and she confirmed that the keyboard does not show after clicking Edit comment.

@parasharrajat, do you know why we wouldn't want the keyboard to show / when we would've changed that? Looking back a couple of months to this PR it seems like we did have the keyboard showing for Android devices.

@ddikodroid and @hiennguyen92, can you elaborate a bit on your respective proposed solutions for this if you're interested in taking on this issue.

[parasharrajat]

Hmm. Indeed, it was added by that issue. I may have confused it with something else but I remember that we used to hide the composer also when the Edit comment is clicked. Now this is not working which is another issue (https://expensify.slack.com/archives/C01GTK53T8Q/p1633793260061400)

[NikkiWines]

Ah ok - yeah I think we should definitely standardize behavior for the composer when editing (both for beginning and ending an edit)

Ideally, I think we'd want to show the composer when Edit is pressed, otherwise, we just require another action from the user before they can do anything. Then, once an edit is completed, the composer should automatically hide.

[parasharrajat]

@laurenreidexpensify It looks like you are commenting on the wrong issue. Anyways, #6138 does not solve this issue. I am still figuring out the root cause here.

[NikkiWines]

I think @laurenreidexpensify brought this up it because it looks like your draft PR #6138 mentions this issue.

[sidferreira]

@NikkiWines @laurenreidexpensify

I did some tests and it seems to me that the problem is somehow related to the following line:

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Lines 105 to 109 in dc67437

	if (closePopover) {
	// Hide popover, then call editAction
	hideContextMenu(false, editAction);
	return;
	}

I believe there's a conflict happening between the React Native's Modal (base for react-native-modal/Popover/ContextMeny) and the keyboard being visible.

According my tests, skipping 1 frame is enough to fix this:

hideContextMenu(false, () => {
    setTimeout(editAction, 17);
});

This change can be applied at the line above or at

App/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js

Lines 200 to 202 in dc67437

	if (_.isFunction(onHideActionCallback)) {
	this.onPopoverHideActionCallback = onHideActionCallback;
	}

The 2nd option would cause all hideContextMenu callbacks to skip a frame.

[NikkiWines]

@sidferreira, while this would likely solve our issue, we typically try to avoid solutions that are a bit on the "hackier" side of things since they often come with detriments in the long run and don't lend themselves to clean code.

[sidferreira]

@NikkiWines I was thinking about trying requestAnimationFrame, but I'm not sure if that would be seen as "hacky" as well

[sidferreira]

btw, it seems that there are other issues related to react-native-modal and keyboard: react-native-modal/react-native-modal#516

[sidferreira]

@tgolen I did some research and, except if we do some updates on React Native it self, there's no workaround.

react-native-modal's onModalHide is dispatched based on setState callbacks, and not based on the native view behaviour.
https://github.com/react-native-modal/react-native-modal/blob/master/src/modal.tsx#L675-L686

Android does have an onDismiss for modals ( https://developer.android.com/reference/android/app/Dialog#setOnDismissListener(android.content.DialogInterface.OnDismissListener) ) but it isn't implemented on React Native. Per documentation, only iOS has onDismiss ( https://reactnative.dev/docs/modal#ondismiss-ios )

With that said, I don't see any other options other than "skip a frame".

[parasharrajat]

It was working before...

[sidferreira]

It was working before...

@parasharrajat I'm checking it...

[sidferreira]

@parasharrajat So, I've checked the following commits and it wasn't working on any of those:

• df3c6e9 (creation)
• 0b6263b (merge to master)
• 28ea30f (1.0.82-0)

Can you mention any specific commit/version where it is working as expected?

[mallenexpensify]

We are placing all non-critical issues on hold while we're on a company offsite. The hold is expected to be lifted on 11/19 (but could go longer). For any PRs that are submitted before or during the hold, we will add a $250 bonus payment.

[kadiealexander]

Increased job price to $1000 on behalf of the team!

[NikkiWines]

Asked QA just to be sure this issue is still occurring, will update when they report back.

From @sidferreira's link here it does look like this is a known bug with react-native-modal and not something introduced in our own code.

@marcaaron not sure if we have a process for regressions that aren't introduced by us, but by external libs. Maybe you have some insight here?

[sidferreira]

@NikkiWines just a reminder that, under the hood, react-native-modal uses {modal} = 'react-native'

[mallenexpensify]

@NikkiWines if you don't get the below answered, want to raise in #contributor-management for more 👀? (fwiw, I don't know of a process)

not sure if we have a process for regressions that aren't introduced by us, but by external libs. Maybe you have some insight here?

[sidferreira]

@NikkiWines @tgolen I decided to do some extra tests here, even creating a sandbox project to test it out ( https://github.com/sidferreira/testInputModal ) and even in that simple project the issue happens the same way.

Actually, in that sample I even had to disable autoFocus on input to make it work, and still needed the timeout to make it work.

So, I'm still propose #5637 (comment) (may try to use requestAnimationFrame as may have a shorter response time).

I agree it is ugly, but, it seems to be a platform issue ( #5637 (comment) )

[tgolen]

OK, this is sort of where I am landing on this issue... I'd love to get your thoughts on this.

I am not sure that having a workaround in our code is better than just leaving the issue the way that it is. I don't think we have a clear understanding of just how bad it makes the UX for users, and if we put the workaround in the code, it definitely leaves the temptation for other developers to copy it, regardless if it's an exception to our styles or not.

I propose that we close this issue for now. Then:

1. If we find out that customers and users are complaining about this behavior, we reopen
2. If the regression in the third-party lib is fixed, we reopen

I think we should still pay something for contributor time and investigation.

[NikkiWines]

I agree, closing sounds good for now and I'm also in support of paying some amount for the time + effort spent investigating this issue. @laurenreidexpensify thoughts?

[laurenreidexpensify]

Okay great - @sidferreira @parasharrajat I'll be hiring you both in Upwork to issue a bonus payment here for investigation and time on this issue, and closing it out.

[laurenreidexpensify]

oh and to @Santhosh-Sellavel for reporting this issue too

[laurenreidexpensify]

Sid and Rajat have been paid, and as soon as Santhosh accepts the job I"ll pay him and close out.

[marcaaron]

not sure if we have a process for regressions that aren't introduced by us, but by external libs

Late to the party @NikkiWines but I think as a general rule if we merge a PR that bumps a library that introduces a regression then whoever created the PR to bump that library effectively introduced that regression and should try to do something about it.