fix: fire onScroll from props in KeyboardAwareScrollView
#339
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
🐛 bug
📊 Package size report
|
2 tasks
kirillzyusko
added a commit
that referenced
this pull request
Apr 5, 2024
## 📜 Description Fixed firing of `onScroll` in `KeyboardAwareScrollView` ## 💡 Motivation and Context Initially it was fixed in #339 and I'm sure it was working 😅 However it looks like in current setup (RN 0.73, REA 3.8.0) such way is not working and `onScroll` property still gets ignored 😔 So in this PR I followed a different path: initially I wanted to fire this handler via `runOnJS`, but we can not follow a full signature of the method, since REA has only `nativeEvent` property (and indeed in this case some properties would be simply missing). The different path was to update a shared value from JS thread and call a callback as usually - it fixes a problem in old code and in a new, so for now this approach looks decent 👍 Let's see how it works in a wild life 👀 Also in this PR I'm changing content of `tea.yaml` - it should be done in separate PR, but I don't want to open another PR just to fix quotes, so decided to merge everything in a single one 🙈 Closes #337 ## 📢 Changelog ### JS - fire `onScroll` in callback; - react on scroll position change in JS thread. ### FS (file system) - added single quotes `'` for `tea.yaml`; ## 🤔 How Has This Been Tested? Tested manually on iPhone 15 Pro. ## 📸 Screenshots (if appropriate): <img width="474" alt="image" src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/11896adc-3fdd-4ab3-acf9-9f32f27e787c"> ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
kirillzyusko
added
the
KeyboardAwareScrollView 📜
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
Fire
onScrollfrom props inKeyboardAwareScrollView.💡 Motivation and Context
Initially I wanted to achieve that as
runOnJS(props.onScroll)({ nativeEvent: e }). But in this case I'm getting:And it's really incorrect, because several properties will be missing and js-based handler (not based on
Animated.event) may not read expected properties (such astargetetc.).So I wanted to pass
onScrollhandler from reanimated asonScrollReanimatedproperty. In this caseonScrollwill be passed fromrestprops and will be fired correctly.Closes #337
📢 Changelog
JS
onScroll(asrest) toScrollView.🤔 How Has This Been Tested?
Tested manually on iPhone 15 Pro.
📝 Checklist