[Tracking] [Performance] Investigate creating our own KeyboardAvoidingView

[roryabraham]

Coming from https://expensify.slack.com/archives/C01GTK53T8Q/p1659722402266779?thread_ts=1659347778.519049&cid=C01GTK53T8Q

Outstanding

• [HOLD for payment 2023-03-02] [$3000] mWeb/safari - Create Group button is hidden by keyboard when selecting participants #11463

Fixed

• Software keyboard covers part of the submit button on login page #11701
• • [HOLD for payment 2023-01-11] IOS - Workspace -Units under Reimbursement-hidden by keyboard after tap on it (small phones) #11391
• [HOLD for payment 2022-12-09] [$1000] [Bug] mWeb - If you close the keyboard using native back button then it will keep reopening while selecting the contacts - reported by @thesahindia #11094
• [HOLD for BZ checklist] [$2000] Android - Add Phone number UI not consistent - reported by @gadhiyamanan #11087
• [Bug] iOS - Chat - The keyboard opens and covers the compose box when have message with a draft #11118
• mWeb/Safari - Security - Close account button hidden by keyboard #7572
• [Bug] [$250] Android - Invite members - Invite message text box disappears when select emoji picker #10609
• [Bug] [$1000] Android - Split/Send/Request money - The transition is not smooth when requesting/split amount #10610
• [$250] BUG: Pay button is not visible above the keyboard on the IOU confirmation page #11722
• [$250] Vertical keyboard padding is shown beneath input when opening chat #12114
• [$250] mWeb/Safari - Workspace - Page jumps up and hides the search field when I focus in "Add a personal message" field #11776
• [$1000] [Bug] IOS - Chat - Password - Save button bounces when moving from one line to another #11057
• [$250] Android - Compose box shifts to the top when the keyboard opens #12622
• [HOLD for payment 2022-11-18] [$250] Attachments - Preview is broken for Protected PDF file (iOS and Android) #12480
• [HOLD for payment 2022-11-16] [$1000] iOS - LHN - The green + button moves #10905
• [$500] New chat - Composed box is displayed in the middle of page when open a user that don't have any messages with #11858
• [HOLD for payment 2022-12-23] [$2000] [Bug] Keyboard being open/closed changes position of ToS on certain mobile platforms - reported by @gadhiyamanan #11277
• [HOLD for payment 2023-02-07] [HOLD #11463] [$2000] [Bug] mWeb/iOS - Request money/Split bill - confirmation button is overlapped by the keyboard #10670

Problem

First a few observations:

• Dealing with KeyboardAvoidingViews (different behaviors) is always kind of confusing imo
• Furthermore, we've got our own wrapper component for KeyboardAvoidingView that isn't used anywhere in the codebase
• We've also got this LoginKeyboardAvoidingView component. The native implementation is just the KeyboardAvoidingView from React Native, and the web/desktop implementation seems kind of funky to me. It just passes a style and contentContainerStyle to the style prop of the React Native implementation, which seems unnecessary since the RN component has both of those props.
• We also use this abandoned KeyboardSpacer component that is not very different from KeyboardAvoidingView AFAICT
  • EdIt: We now have our own customer KeyboardSpacer component

All that is to say, it feels like our usage of keyboard-avoiding stuff is a bit inconsistent/all-over-the-place, and maybe it's a good time to step back and see if we can consolidate some of this and make it a bit more consistent.

Moreover, we've noticed issues such as this one where the keyboard-avoiding stuff isn't necessarily working as expected. There have been other similar bugs in the past. For example, we have some screens where we use timeouts to wait for navigation transitions to finish before opening the keyboard, etc... Furthermore, many of these bugs have been sort of inconsistent (maybe suggesting that they are caused by race conditions). Lastly, I've noticed that the keyboard is just really sluggish and slow to open on iOS.

Solution

Now this part is more of a hunch, but I have a theory that many of these bugs are caused because Reanimated (the animation library used by react-navigation) and the React Native KeyboardAvoidingView/KeyboardSpacer components aren't playing nice together. Why? Because Reanimated runs animations synchronously on the UI thread, while the KeyboardAvoidingView components will update asynchronously between the UI thread and the layout/render thread. This makes it hard to keep them in sync / behaving as expected.

I did a bit of research and noticed that Reanimated 3 is set to introduce a useAnimatedKeyboard hook. This would allow us to create a simple KeyboardAvoidingView of our own that is animated synchronously on the UI thread. I think this would be better for performance (which I have noticed is lackluster on iOS staging+production), and hopefully make opening/closing the keyboard much snappier. Furthermore, since (all?) our animations would be running in Reanimated, I have a hunch that many of these inconsistent bugs might go away.

So my proposal (or at minimum, an idea that I think is worth looking into) is that we:

• upgrade to Reanimated 3 (after this PR is merged)
• create our own KeyboardAvoidingView component using the useAnimatedKeyboard hook from Reanimated
• Use our component everywhere. No more use of the RN built-in KeyboardAvoidingView or the KeyboardSpacer library.

[roryabraham]

HOLD on #9841

[roryabraham]

Taking this off HOLD

[roryabraham]

not prioritized yet

[roryabraham]

cc @Szymon20000

[Julesssss]

YES

[marcaaron]

Thanks Rory, the solution part of this is really fascinating and it sounds like maybe there are some big performance gains hiding away in Reanimated 3.

I am having a little trouble with the "problem" section of this and think it needs refining. Or at a minimum break concerns into separate issues to tackle one at a time. I'm admittedly having trouble believing that we aren't just holding something wrong (as that's usually the case).

Dealing with KeyboardAvoidingViews (different behaviors) is always kind of confusing imo

Can we unlock this somehow? There must be some basic cases where the behaviors should be used and it seems possible to articulate what they do and develop some best practices and guidance?

Furthermore, we've got our own wrapper component for KeyboardAvoidingView that isn't used anywhere in the codebase

Could we figure out if we can delete this and replace it with a regular KeyboardAvoidingView ? That seems like a good cleanup task.

We've also got this LoginKeyboardAvoidingView component. The native implementation is just the KeyboardAvoidingView from React Native, and the web/desktop implementation seems kind of funky to me. It just passes a style and contentContainerStyle to the style prop of the React Native implementation, which seems unnecessary since the RN component has both of those props.

This too?

We also use this abandoned KeyboardSpacer component that is not very different from KeyboardAvoidingView AFAICT
EdIt: We now have our own customer KeyboardSpacer component

And this?

maybe it's a good time to step back and see if we can consolidate some of this and make it a bit more consistent

Agree, but what does it have to do with rolling our own KeyboardAvoidingView? Is it possible to articulate what it is exactly about the component that isn't working for us? I think a lot of the changes you are mentioning might have been implemented when many of us were very naively approaching react-native and trying to get things done quickly.

For example, we have some screens where we use timeouts to wait for navigation transitions to finish before opening the keyboard

What's an example of this? Is it something we have or can create issues to address/improve?

Furthermore, many of these bugs have been sort of inconsistent (maybe suggesting that they are caused by race conditions)

Could use an example here as well.

I've noticed that the keyboard is just really sluggish and slow to open on iOS.

And it's because of the KeyboardAvoidingView ? Do we have an open issue for this?

---

One thing learned from today is that barely anyone has a proper understanding of how this component works. 😂 So I'd love to keep exploring KeyboardAvoidingView and see if it is the cause of these woes or if we can make it work for us or not.

[melvin-bot]

⚠️ 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.

[melvin-bot]

⚠️ 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.

[roryabraham]

No update here yet, but looking forward to working on this. Fortunately useAnimatedKeyboard was released in 2.10 so upgrading to a 3.0 beta release is no longer a prerequisite of doing this

[melvin-bot]

⚠️ 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.

[melvin-bot]

⚠️ 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.

[JmillsExpensify]

PR for the last non-held issue is on staging and should hit production tomorrow!

[JmillsExpensify]

Working through a regression on the last outstanding issue.

[JmillsExpensify]

Last issue is getting close! Still working through that regression, though it's likely to be merged this week.

[janicduplessis]

What is the status on this? I ended up investigating using useAnimatedKeyboard while working on enabling edge to edge display on android. However it seems to cause an issue where the status bar is no longer transparent as soon as we use the hook. So it would probably require a fix in reanimated. Except for that it seems to work really well on Android and adds a nice animation so the content moves with the keyboard opening.

I will leave the code here for later use if we can have a fix in reanimated.

/*
 * The KeyboardAvoidingView is only used on ios and android
 */
import React from 'react';
import Animated, {
    useAnimatedKeyboard,
    useAnimatedStyle,
} from 'react-native-reanimated';

const KeyboardAvoidingView = (props) => {
    const keyboard = useAnimatedKeyboard({isStatusBarTranslucentAndroid: true});
    const animatedStyle = useAnimatedStyle(() => ({
        paddingBottom: keyboard.height.value,
    }));
    return (
        // eslint-disable-next-line react/jsx-props-no-spreading, react/prop-types
        <Animated.View {...props} style={[props.style, animatedStyle]} />
    );
};

KeyboardAvoidingView.displayName = 'KeyboardAvoidingView';

export default KeyboardAvoidingView;

[JmillsExpensify]

In terms of this issue, all linked issues above are merged and nothing is being actively worked on for KeyboardAvoidingView. We're essentially just working through a regression/payment step in the one held issue mentioned above.

[roryabraham]

@tomekzaw Want to take a look at @janicduplessis's bug report here?

Edit: Looks like this was probably fixed in software-mansion/react-native-reanimated#3958 – just needs to included in a new release

Edit again: JK, that PR was released in 3.0.0, but we're still on 3.0.0-rc.10, just need to upgrade.

[tomekzaw]

@roryabraham Sure, I will be more than happy to help here.

You're right, software-mansion/react-native-reanimated#3958 is already merged and released so we'll just need to upgrade to 3.0.2 first.

As for the useAnimatedKeyboard hook, we're still investigating two minor issues:

1. It looks like there is 1 or 2 frames of delay between keyboard animation start and shared value update.
2. After some configuration of opening/closing the keyboard, the shared value gets updated only 3-4 times during the animation (see [v3][iOS] useAnimatedKeyboard smooth animation breaks with keyboardDismissMode="on-drag" software-mansion/react-native-reanimated#4194)

Edit: #2 should be fixed with software-mansion/react-native-reanimated#4266

[terrysahaidak]

@tomekzaw @roryabraham I have KeyboardAvoidingView for iOS implemented using useAnimatedKeyboard in my PR #16356 since my task relies heavily on the keyboard being animated in the same frame as everything else, I wanted to animate things based on its height.

[JmillsExpensify]

As for the this tracking issue, I kind of think we should close this issue since all the related issues are now done. Maybe we should create a separate issue for upgrading Reanimated?

[terrysahaidak]

As part of this issue and my #16356, we (cc @roryabraham ) decided to contribute the implementation of KeyboardAvoidingView I did as part of the PR to Reanimated. This way they will be able to track and fix all the bugs related to useAnimatedKeyboard hook. I'll add PR later today.

Another thing I did - reported a bug that stops us from upgrading Reanimated version which fixes some other bugs:
software-mansion/react-native-reanimated#4324

[tomekzaw]

Another thing I did - reported a bug that stops us from upgrading Reanimated version which fixes some other bugs:
software-mansion/react-native-reanimated#4324

Fixed with software-mansion/react-native-reanimated#4329

[JmillsExpensify]

Alright, I think we did it and have officially fixed all issues linked to this tracking initiative. We also already have the regression tests, so I think it's to close this one out.

[JmillsExpensify]

Someone please re-open and correct me if I'm wrong!

[terrysahaidak]

Here is the PR that implements KeyboardAvoidingView in Reanimated:
software-mansion/react-native-reanimated#4339

[terrysahaidak]

The implementation of KeyboardAvoidingView is pretty much ready, finishing the example used for testing, it mimics our usage I'm working at #16356 .

Also, we're currently testing this KeyboardAvoidingView implementation on Android on another project.