-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[$250] react-native-keyboard-controller
migration plan
#37203
Comments
Triggered auto assignment to @JmillsExpensify ( |
this is kind of halfway between a new feature and a bug, but I'm going to treat it as a bug for the sake of accounting (no CAP SW project it fits with) |
Marking this as |
Job added to Upwork: https://www.upwork.com/jobs/~01360efa253e7a2c60 |
Triggered auto assignment to Contributor Plus for review of internal employee PR - @cubuspl42 ( |
welcome @cubuspl42! I encourage you to familiarize yourself with the context on this issue and decide whether you want to take it on or pass it off. Excited to have you help if you're interested! |
moving this to weekly |
@roryabraham I was already assigned #16356. |
Fair enough. This is a broader migration than just that issue/PR though, basically we'll be refactoring keyboard handling across the whole app. |
probably wouldn't hurt to get more than 1 C+ involved here either. |
@roryabraham Can you assign me here also? |
Sounds interesting; I can get on board. This will be implemented by an expert agency, and C+ reviewed, right? |
@cubuspl42 Please have a look at the #16356. |
@shubham1206agra I took a look. It's a PR with a 1-year-long post history. Let me know what exact aspect of that PR you'dl like me to focus on! |
Take a look at the top description and go through code once. |
correct. 2 C+ in this case |
This issue has not been updated in over 15 days. @JmillsExpensify, @rojiphil, @kirillzyusko, @roryabraham, @shubham1206agra eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
@JmillsExpensify, @rojiphil, @kirillzyusko, @roryabraham, @shubham1206agra, this Monthly task hasn't been acted upon in 6 weeks; closing. If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead. |
@kirillzyusko what's the plan here? Should we keep this open or revisit later?. |
@roryabraham we are actively working on that! @perunt already started to use the functionality of this library in #42630 and now he is actively working on #42143 Meanwhile I'm working on #47096 (this PR is kind of a preparation before #42143) When we start to use more functionality of So I'd like to keep it open since the progress is moving! |
It's in the review phase. Once testers confirm there's no jittering on their devices, we'll be good to go. |
Need someone to hand this off to. @luacmartins not sure who's best, but going to reassign to you. Maybe you can handle or pass off to another Expensify engineer |
@luacmartins yes, review + we need to fix issues that were found 👀 |
Still working on the PR. |
This issue has not been updated in over 15 days. @JmillsExpensify, @rojiphil, @luacmartins, @kirillzyusko, @roryabraham, @shubham1206agra eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
@MelvinBot we are actively working on this issue. For example today #52392 was merged (where we started to use keyboard-tracking functionality on Android). So slowly but decently we are moving 💪 |
Problem statement
In Expensify app keyboard handling is not consistent across iOS/Android platforms:
adjustResize
(it works great, but lacks of synchronous animations - which means the screen will be instantly resized, and there will be no animation at all. For chat-like apps this is quite important to have for good UX)KeyboardAvoidingView
, subscriptions to keyboard events)since iOS keyboard handling is done in JS code, on Android we use native keyboard avoidance provided by OS and we are writing cross-platform code -> we have a lot of Platform dependent code (like enabling
KeyboardAvoidingView
only on iOS), which eventually leads to UI inconsistencyDid
events)With
react-native-keyboard-controller
:Will
events which were not available prior to RN API)multiline
TextInput can not grow)Migration plan
I see next potential way for improving keyboard handling (aka migration plan):
implement inline auto suggestions (Inline auto suggestion #42630)
start to use it on a chat screen on iOS only (fix: messages content overlap when bottom sheet is shown #42143)
start to use this library on chat screen on Android (this library also will give a smooth transition on chat screen when keyboard appears/disappears, so UI will look better & more pleasant for end user)
search screen:
KeyboardStickyFooter
component)See it in action
See it in action
on Chat screen we can add "interactive keyboard dismissal" (when keyboard can be closed via gesture)
@roryabraham wanted to replace
useKeyboardContext
with this new libraryIn Profile->Address screen: auto scroll is waiting for
keyboardDidShow
event and we have kind of two-fold animation -> potentially can be fixed by KeyboardAwareScrollView with fully synchronized animationsAdditional resources
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @kirillzyuskoThe text was updated successfully, but these errors were encountered: