Skip to content

Commit

Permalink
fix: remove wrapper view from KeyboardAwareScrollView (#321)
Browse files Browse the repository at this point in the history
## 📜 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
  • Loading branch information
kirillzyusko authored Jan 12, 2024
1 parent 6f055c9 commit ec912c0
Show file tree
Hide file tree
Showing 14 changed files with 12 additions and 2 deletions.
4 changes: 4 additions & 0 deletions docs/docs/api/components/keyboard-aware-scroll-view.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,7 @@ const styles = StyleSheet.create({
},
});
```

## Known issues

- [react-native-reanimated#5567](https://github.com/software-mansion/react-native-reanimated/issues/5567): Resizing content inside `ScrollView` prevents multiline `TextInput` from growing in Fabric
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,7 @@ const styles = StyleSheet.create({
},
});
```

## Known issues

- [react-native-reanimated#5567](https://github.com/software-mansion/react-native-reanimated/issues/5567): Resizing content inside `ScrollView` prevents multiline `TextInput` from growing in Fabric
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified e2e/kit/assets/ios/iPhone 13 Pro/AwareScrollViewFirstInputGrown.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified e2e/kit/assets/ios/iPhone 13 Pro/AwareScrollViewInputChanged.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified e2e/kit/assets/ios/iPhone 13 Pro/AwareScrollViewKeyboardClosed.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified e2e/kit/assets/ios/iPhone 13 Pro/AwareScrollViewTextChanged.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 2 additions & 1 deletion e2e/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"test-example:android": "detox test --configuration example.android.emu.release --loglevel verbose --take-screenshots failing --record-videos failing",
"build-example:ios": "detox build --configuration example.ios.sim.release",
"test-example:ios": "detox test --configuration example.ios.sim.release --loglevel verbose --take-screenshots failing --record-videos failing",
"test": "echo \"Error: no test specified\" && exit 1"
"test": "echo \"Error: no test specified\" && exit 1",
"detox-clean": "detox clean-framework-cache && detox build-framework-cache"
},
"repository": {
"type": "git",
Expand Down
3 changes: 2 additions & 1 deletion src/components/KeyboardAwareScrollView/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,8 @@ const KeyboardAwareScrollView: FC<KeyboardAwareScrollViewProps> = ({
onScroll={onScroll}
scrollEventThrottle={16}
>
<Reanimated.View style={view}>{children}</Reanimated.View>
{children}
<Reanimated.View style={view} />
</Reanimated.ScrollView>
);
};
Expand Down

0 comments on commit ec912c0

Please sign in to comment.