Skip to content
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

Include existing height when calculating new one for KeyboardAvoidingView #34749

Closed

Conversation

pfulop
Copy link
Contributor

@pfulop pfulop commented Sep 21, 2022

Summary

Currently, height is sometimes the only valid option for pushing TextInput up in the layout on Android. The problem is when switching keyboards. For instance, switching from ABC to emojis. This will trigger keyboard show events and recalculate the height for the KeyboardAvoidingView. Since the keyboard is still showing, the view has the height that was previously calculated and thus frame represents that. This means the frame.height has adjustments for the keyboard calculated in it, but it is used the same way as if the keyboard was not showing. This results in wrong calculation and the input showing at the incorrect place in the layout (mostly hidden under the keyboard)

This fix simply uses the previous calculation to offset frame.height, resulting in the correct height and smooth switching between keyboards. It's also scoped only to height mode since that's where the problem shows.

Note: I mention android here, but it fixes it for both platforms. It's just that iOS usually works best with different behaviour so it's rarely used there.

Changelog

[General] [Added] - Include this.state.bottom when calculating new keyboard height to fix android keyboard switching

Test Plan

With simple code:

import { StatusBar } from "expo-status-bar";
import React from "react";
import {
  KeyboardAvoidingView,
  StyleSheet,
  Text,
  TextInput,
  View,
} from "react-native";

export default function App() {
  return (
    <KeyboardAvoidingView style={styles.container} behavior="height">
      <Text>Open up App.js to start working on your app!</Text>
      <StatusBar style="auto" />
      <TextInput style={{ backgroundColor: "red", width: "100%" }} />
    </KeyboardAvoidingView>
  );
}

const styles = StyleSheet.create({
  container: {
    padding: 32,
    flex: 1,
    backgroundColor: "#fff",
    alignItems: "center",
    justifyContent: "space-between",
  },
});

Notice the consistency of the TextInput after the changes, while before it would just move around more you switch the keyboards.

Before After
2022-09-21 13-59-09 2022-09-21 14_01_44 2022-09-21 14-03-33 2022-09-21 14_04_30

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Sep 21, 2022
@react-native-bot react-native-bot added the Type: Enhancement A new feature or enhancement of an existing feature. label Sep 21, 2022
@facebook-github-bot
Copy link
Contributor

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @pfulop in f85e2ec.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Sep 26, 2022
@Orange9000
Copy link

Was the merge reverted? This bug popped again in RN 0.70.2.

@dranitski
Copy link

dranitski commented Jan 2, 2023

I can confirm the bug is here again in RN 0.70.5 ⚠️

Android/ios, behavior = height

@bang9
Copy link
Contributor

bang9 commented Jan 13, 2023

@Orange9000 @dranitski Here, you can find the version that includes PR.

image

image

@anatooly
Copy link

It's very important task, thank you.

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…View (facebook#34749)

Summary:
Currently, height is sometimes the only valid option for pushing `TextInput` up in the layout on Android. The problem is when switching keyboards. For instance, switching from ABC to emojis. This will trigger keyboard show events and recalculate the height for the `KeyboardAvoidingView`. Since the keyboard is still showing, the view has the height that was previously calculated and thus `frame` represents that. This means the `frame.height` has adjustments for the keyboard calculated in it, but it is used the same way as if the keyboard was not showing. This results in wrong calculation and the input showing at the incorrect place in the layout (mostly hidden under the keyboard)

This fix simply uses the previous calculation to offset `frame.height`, resulting in the correct height and smooth switching between keyboards. It's also scoped only to height mode since that's where the problem shows.

_Note: I mention android here, but it fixes it for both platforms. It's just that iOS usually works best with different behaviour so it's rarely used there._

## Changelog

[General] [Added] - Include `this.state.bottom` when calculating new keyboard height to fix android keyboard switching

Pull Request resolved: facebook#34749

Test Plan:
With simple code:

```jsx
import { StatusBar } from "expo-status-bar";
import React from "react";
import {
  KeyboardAvoidingView,
  StyleSheet,
  Text,
  TextInput,
  View,
} from "react-native";

export default function App() {
  return (
    <KeyboardAvoidingView style={styles.container} behavior="height">
      <Text>Open up App.js to start working on your app!</Text>
      <StatusBar style="auto" />
      <TextInput style={{ backgroundColor: "red", width: "100%" }} />
    </KeyboardAvoidingView>
  );
}

const styles = StyleSheet.create({
  container: {
    padding: 32,
    flex: 1,
    backgroundColor: "#fff",
    alignItems: "center",
    justifyContent: "space-between",
  },
});
```

Notice the consistency of the TextInput after the changes, while before it would just move around more you switch the keyboards.

|  Before  | After  |
|---|---|
| ![2022-09-21 13-59-09 2022-09-21 14_01_44](https://user-images.githubusercontent.com/3984319/191499509-b41280a0-2969-4fe6-8796-c5695b999f27.gif)  | ![2022-09-21 14-03-33 2022-09-21 14_04_30](https://user-images.githubusercontent.com/3984319/191499628-a5832b88-e511-448d-8081-ac48d3a3690a.gif)  |

Reviewed By: cipolleschi

Differential Revision: D39718812

Pulled By: NickGerleman

fbshipit-source-id: 2550182e846f3f8e719d727fa8e6d87165faebf6
@supoved
Copy link

supoved commented Jun 13, 2023

I can see this bug happening again in 0.71.8

Media_230613_095004

as far as I can see none of callbacks is called when keyboard type is changed, so its probably somewhere around native code integration (tested on Android 13)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants