feat: focused input #257
Merged
feat: focused input #257
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kirillzyusko
added
enhancement
📊 Package size report
|
|
kirillzyusko
force-pushed
the
feat/focused-input
branch
from
6910099 to
1771308
Compare
1 task
kirillzyusko
added a commit
that referenced
this pull request
Jan 12, 2024
## 📜 Description Removed view wrapper. Attached that view as last child + kept padding for this view. ## 💡 Motivation and Context The approach with additional view was added in #257 I don't remember exact reason, but for me it seems like it was added, because TExtInputs were not able to grow. However such approach is causing additional issues: - breaks styling in some cases - stickyHeaderIndices={[0]} produces a crash (because RN will try to attach `Animated`-based style to REA view) So in this PR I'm removing this view wrapper and apply additional padding in mostly the same way as it was before #257 (additional child-view in the end of ScrollView, but instead of `height` I'm animating `paddingBottom`). I've tested and it still works (i. e. inputs are growing). E2E tests also passing (so there seems to be 1px difference, which can be neglected, but i had to update assets to assure E2E tests consistency). > [!IMPORTANT] > This fix will introduce software-mansion/react-native-reanimated#5567. I've added that issue as "known issue" in docs page. I think it's better to have properly working paper architecture and Fabric can be fixed later (because it's still in early adoption and is not widely used). Closes #325 ## 📢 Changelog <!-- High level overview of important changes --> <!-- For example: fixed status bar manipulation; added new types declarations; --> <!-- If your changes don't affect one of platform/language below - then remove this platform/language --> ### E2E - added `detox-clean` command (useful when you updated XCode version and haven't run detox tests yet) ### JS - removed wrapper and moved view as last child ## 🤔 How Has This Been Tested? Tested on: - e2e (Android, iOS - paper) - Pixel 7 Pro (Android 14, paper) ## 📸 Screenshots (if appropriate): There is no visual difference 🙂 ## 📝 Checklist - [x] CI successfully passed
2 tasks
kirillzyusko
added a commit
that referenced
this pull request
Jan 13, 2024
…ScrollView` (#332) ## 📜 Description Synchronized bottom padding with keyboard frame in `KeyboardAwareScrollView`. ## 💡 Motivation and Context Original regression was introduced in #257 ([these](https://github.com/kirillzyusko/react-native-keyboard-controller/pull/257/files#diff-b4493213c8dc470b5ba251af896d133130cc8996cee1118525e4eba1f3aa993bL87-L88) code lines). I decided not animated bottom padding to potentially fix the issue described in #255 (comment) However such approach is causing unsynchronized animations in certain conditions and I'm not sure, that it actually fixes the problem described in comment above. So in this PR I'm reworking it again and bringing back animated transitions 👀 Closes #329 ## 📢 Changelog ### JS - added `currentKeyboardFrameHeight` variable; - update `currentKeyboardFrameHeight` variable in `onMove` handler; - use `currentKeyboardFrameHeight` for bottom padding in KASV. ## 🤔 How Has This Been Tested? Tested manually on iPhone 15 Pro + e2e tests. ## 📸 Screenshots (if appropriate): |Before|After| |-------|-----| |<video src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/a7e5e3d3-8ef0-4b69-8c21-2414b1899e91">|<video src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/bdd34dee-25bd-4491-8612-6048c88760bb">| ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📜 Description
Added new
useReanimatedFocusedInputhook that returns an information aboutTextInputthat currently is in focus.💡 Motivation and Context
Before we could use a
targetproperty to measureTextInput. However such approach had several disadvantages:ShadowNodeand it adds more complexity to the code (and you need to usefindNodeHandlewhich is deprecated in StrictMode);useKeyboardHandleris not reporting such cases (and obviously it shouldn't);With the introduction of this hook all these disadvantages are gone 😎
In this hook can be enhanced to cover more cases (for example add method
updatewhich can synchronously update layout before reading it, or update layout when certain lifecycle events occurs (for example screen rotation)).Closes #249 #222
📢 Changelog
Docs
Inputin API section;useReanimatedFocusedInputhook;package.json.JS
TextInputs;iOS
KVOto focused input;Android
listenerspackage;🤔 How Has This Been Tested?
Tested manually on (both paper and fabric):
📸 Screenshots (if appropriate):
Screen.Recording.2023-10-20.at.00.08.29.mov
📝 Checklist