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

[New Lib] react-native-keyboard-controller #35270

Closed
roryabraham opened this issue Jan 26, 2024 · 21 comments
Closed

[New Lib] react-native-keyboard-controller #35270

roryabraham opened this issue Jan 26, 2024 · 21 comments
Assignees
Labels
AutoAssignerAppLibraryReview Auto assign someone to review a new library being added to App Weekly KSv2

Comments

@roryabraham
Copy link
Contributor

roryabraham commented Jan 26, 2024

coming from this PR which Margelo recently dusted off, I wanted to get feedback on the introduction of this new lib

Name of library: react-native-keyboard-controller

Details

Issue: #10632

High level problem description:

React Native's Keyboard APIs are inherently kind of buggy because they depend on asynchronous communication between JS and native. We currently use a mix of native keyboard avoidance and JS keyboard avoidance, which is pretty confusing, but also limiting in our ability to create consistent and interactive keyboard experiences.

We originally set out to use Reanimated's useAnimatedKeyboard hook to solve the problem outlined in the issue, and I asked the contributor why they decided to use this library instead, to which they replied:

with REA hook I'm getting invalid frames sometimes (especially when we need to deal with modals and instant keyboard transitions).
So, when keyboard should appear I was getting frames like 0 -> 291 -> 0 -> 336, while valid sequence had to be 0 -> 291 -> 336.
Also react-native-keyboard-controller supports interactive keyboard dismissal (which can be handy in Expensify app and overall used extensively in many chat-like apps) and already has prebuilt components (such as KeyboardAvoidingView)

I think the most important one here is that it already has prebuilt components specifically for handling keyboard interactions. We've talked to the people at Reanimated about building a KeyboardAvoidingView, and they're open to the idea but it's taken a long time.

  • Number of stars in GH: 1.1k
  • Number of monthly downloads: 24k
  • Number of releases in the last year: 32
  • Level of activity in the repo: low (only 12 open issues and 5 open PRs)
  • Alternatives:

There's a lot:
- Reanimated (which we already use)
- Keyboard API built into React Native
- native iOS and Android keyboard APIs (which are internally leveraged by all these solutions)
- all these other ones: https://kirillzyusko.github.io/react-native-keyboard-controller/docs/api/components/keyboard-aware-scroll-view#comparison

@roryabraham roryabraham added Weekly KSv2 AutoAssignerAppLibraryReview Auto assign someone to review a new library being added to App labels Jan 26, 2024
@roryabraham roryabraham self-assigned this Jan 26, 2024
Copy link

melvin-bot bot commented Jan 26, 2024

Current assignee @roryabraham is eligible for the AutoAssignerAppLibraryReview assigner, not assigning anyone new.

Copy link

melvin-bot bot commented Jan 26, 2024

New Library Review

  • Are all the answers in the main description filled out properly and make sense?
  • Who maintains the library and how well is it maintained?
  • How viable are the alternatives?
  • Should we build it ourselves instead?

Once these questions are answered, start a thread in #engineering-chat, ping the @app_deployers group, and call for a vote to accept the new library. Once the vote is complete, update this issue with the outcome and procede accordingly. Here is a sample post:

Hey @app_deployers,

There is a request to add a new library to App that we need to consider. Please look at this GH and then vote :+1: or :-1: on accepting this new library or not.

GH_LINK

@roryabraham
Copy link
Contributor Author

roryabraham commented Jan 26, 2024

I think maybe someone else is supposed to review this since I created it.

@roryabraham roryabraham removed their assignment Jan 26, 2024
@roryabraham roryabraham added AutoAssignerAppLibraryReview Auto assign someone to review a new library being added to App and removed AutoAssignerAppLibraryReview Auto assign someone to review a new library being added to App labels Jan 26, 2024
Copy link

melvin-bot bot commented Jan 26, 2024

Triggered auto assignment to @Beamanator (AutoAssignerAppLibraryReview), see https://stackoverflowteams.com/c/expensify/questions/17737 for more details.

Copy link

melvin-bot bot commented Jan 26, 2024

New Library Review

  • Are all the answers in the main description filled out properly and make sense?
  • Who maintains the library and how well is it maintained?
  • How viable are the alternatives?
  • Should we build it ourselves instead?

Once these questions are answered, start a thread in #engineering-chat, ping the @app_deployers group, and call for a vote to accept the new library. Once the vote is complete, update this issue with the outcome and procede accordingly. Here is a sample post:

Hey @app_deployers,

There is a request to add a new library to App that we need to consider. Please look at this GH and then vote :+1: or :-1: on accepting this new library or not.

GH_LINK

@roryabraham
Copy link
Contributor Author

I think that the PR itself has a good amount of polish still needed, so this isn't super critical to decide. Mostly I wanted more feedback on whether we're cool with using a new library for keyboard context or if we'd rather invest in improving Reanimated. Not entirely sure if this problem really belongs in the Reanimated space or not, tbh

@roryabraham
Copy link
Contributor Author

I'd encourage a look at the docs before passing judgement, as it does seem like this might be the best keyboard library in the React Native space today

@kirillzyusko
Copy link
Contributor

Looks like 565kb unpacked

Small correction - the impact on JS bundle will be ~60kb.

Below you can see a detailed breakdown of size for each folder. react-native projects will consume code only from src folder, and lib (transformed code from src) folder will be ignored.

image

@Beamanator
Copy link
Contributor

We've talked to the people at Reanimated about building a KeyboardAvoidingView, and they're open to the idea but it's taken a long time.

@roryabraham Quick question, what do you mean here? It's "in progress" but taking too long? Or just the discussion is taking a while?

@Beamanator
Copy link
Contributor

Who maintains the library and how well is it maintained?

I don't quite see Who maintains the library answered in the OP, is it just Kirill Zyusko?

@Beamanator
Copy link
Contributor

  • How viable are the alternatives?
  • Should we build it ourselves instead?

You answered what the alternatives are & that "We currently use a mix of native keyboard avoidance and JS keyboard avoidance", and it looks like this link (https://kirillzyusko.github.io/react-native-keyboard-controller/docs/api/components/keyboard-aware-scroll-view#comparison) basically advocates for this lib, which is great!

So maybe you can just clarify the status / progress of "building it ourselves" and then we'll be ready to start this discussion in slack?

@roryabraham
Copy link
Contributor Author

@roryabraham Quick question, what do you mean here? It's "in progress" but taking too long? Or just the discussion is taking a while?

Just taken a really long time / still not done.

I don't quite see Who maintains the library answered in the OP, is it just Kirill Zyusko?

Yes, I think @kirillzyusko, who works for Margelo. So has support of Margelo team presumably (maybe would make Expensify team feel better if this repo is transferred to Margelo org, but I don't think that should be a requirement).

@roryabraham
Copy link
Contributor Author

roryabraham commented Feb 3, 2024

So maybe you can just clarify the status / progress of "building it ourselves" and then we'll be ready to start this discussion in slack?

There's really no plans to "build it ourselves". There was some discussion with Reanimated maintainers about building it in Reanimated, but I haven't seen any movement on that front in a long time. This library feels like a better bet to me in that it's focused specifically on handling keyboards as best as possible in React Native. It's also maintained by someone we have a close partnership with, and is compatible with Reanimated.

@kirillzyusko can you confirm if this library is New Arch compatible? edit: confirmed that this is New Arch compatible

@roryabraham
Copy link
Contributor Author

roryabraham commented Feb 3, 2024

Just had another scenario crop up that is making this library look better and better. I would love to see us simplify and unify the keyboard handling experience across platforms.

@kirillzyusko BTW, how much have you tested this library on mobile web platforms?

@roryabraham
Copy link
Contributor Author

I don't see any reference to web support in the docs, which I think would probably be a requirement for us.

@kirillzyusko
Copy link
Contributor

@roryabraham this library is simply mocked on web right now (i. e. app will not crash as soon as you added a library, and all hooks will return default values).

I created a discussion a long time ago - kirillzyusko/react-native-keyboard-controller#43 but it got too little attention so I didn't prioritize it.

As far as I know browsers on mobile devices automatically avoid keyboard on browser level, and on desktop there is no sense to avoid keyboard because users are using physical keyboard. Is my understanding correct? Or Expensify has a code that handles keyboard appearance on web in specific manner? 👀

@hannojg
Copy link
Contributor

hannojg commented Feb 8, 2024

Would be interesting to also explore web implementation for expensify in the future!

We are recommending all our clients RNKC for keyboard handling (depends on the use case ofc, but we didn't had any use case yet that the library can't cover), and I think its a good fit for the expensify app as well.
The library is becoming the standard for keyboard handling in react native imo.

@Beamanator
Copy link
Contributor

I def like the plans to start using this lib! I just started the discussion in slack here (https://expensify.slack.com/archives/C01GTK53T8Q/p1707467001447909) so please feel free to add input / follow along! 👍

@melvin-bot melvin-bot bot removed the Overdue label Feb 9, 2024
@melvin-bot melvin-bot bot added the Overdue label Feb 19, 2024
@Beamanator
Copy link
Contributor

still discussing in slack

@melvin-bot melvin-bot bot removed the Overdue label Feb 20, 2024
@Beamanator
Copy link
Contributor

Seems we got consensus here in slack! Let's move forward with this lib 👍

@roryabraham
Copy link
Contributor Author

moving forward in #37203

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoAssignerAppLibraryReview Auto assign someone to review a new library being added to App Weekly KSv2
Projects
None yet
Development

No branches or pull requests

4 participants