fix: remove wrapper view from KeyboardAwareScrollView
#321
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
|
|
For history purposes I'm attaching pixel diff (it seems like it under/over-scrolls 1px, what can be ignored):
|
|
I opened an issue for Fabric problem in REA repository: software-mansion/react-native-reanimated#5567 |
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
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
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:
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
heightI'm animatingpaddingBottom).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
E2E
detox-cleancommand (useful when you updated XCode version and haven't run detox tests yet)JS
🤔 How Has This Been Tested?
Tested on:
📸 Screenshots (if appropriate):
There is no visual difference 🙂
📝 Checklist