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

[IOS] Incorrect behavior of hooks and components when focusing on an input with secureTextEntry enabled. #327

Closed
A-Yatsyk opened this issue Jan 12, 2024 · 14 comments · Fixed by #331
Assignees
Labels
🐛 bug Something isn't working 🍎 iOS iOS specific

Comments

@A-Yatsyk
Copy link

Describe the bug

When focusing on a field with secureTextEntry set to true (iOS only), UI misbehaves with KeyboardAvoidingView, causing erratic jumping. In useKeyboardHandler, onStart will be called with progress 1, followed immediately by onEnd with progress 1, and only after these (prior onStart and onEnd) will correct calls of onStart, onMove, and onEnd occur.
This behavior is also observed in KeyboardEvents, where the first listener is wrong, and only after that, do true listener calls follow.

A similar issue was present in React-Native itself, but it was fixed since KeyboardAvoidingView in React-Native now works as expected for iOS (e.g., facebook/react-native#39411).

Code snippet

<KeyboardAvoidingView
      behavior="padding"
      style={{ flex: 1 }}
      contentContainerStyle={{ flexGrow: 1 }}
>
    <SafeArea style={{ flex: 1 }}>
        <ScrollView
            bounces={false}
            style={{ flex: 1 }}
            keyboardShouldPersistTaps="handled"
            contentContainerStyle={{ flexGrow: 1, paddingHorizontal: 25 }}
        >
            <View style={{ flex: 2 }} />

            <TextInput
                placeholder="Placeholder"
                secureTextEntry
                style={{
                    height: 45,
                    borderWidth: 1,
                    borderColor: 'gray',
                    borderRadius: 10,
                    paddingHorizontal: 10,
                }}
            />

            <View style={{ flex: 1 }} />

            <View style={{ alignItems: 'center' }}>
                <Text>Footer</Text>
          </View>
        </ScrollView>
    </SafeArea>
</KeyboardAvoidingView>

To Reproduce
Steps to reproduce the behavior:

  1. Add secureTextEntry with a value of true to the field.
  2. Wrap the screen using KeyboardAvoidingView.
  3. Focus on the field with a closed keyboard.

OR

  1. Add secureTextEntry with a value of true to the field.
  2. Add useKeyboardHandler with handlers for onStart, onMove, onEnd using console.log.
  3. Focus on the field with a closed keyboard (the first onStart and onEnd won't reflect reality)

Expected behavior

Expected behavior should be the same as when the secureTextEntry is set to false

Screenshots

Screen.Recording.2024-01-12.at.14.48.41.mov

Smartphone:

  • Desktop OS: MacOS 14.2.1
  • Device: iPhone15
  • OS: iOS 17.2
  • RN version: 0.70.13
  • RN architecture: paper (old)
  • JS engine: Hermes
  • Library version: 1.10.1
@wanted-o
Copy link

Have the same issue.

@oleksandr-dziuban
Copy link

Same issue

@kirillzyusko kirillzyusko added 🐛 bug Something isn't working 🍎 iOS iOS specific labels Jan 12, 2024
@kirillzyusko
Copy link
Owner

Interesting, I think this bug was introduced in iOS 17. A similar issue was raised quite long time ago: #243

But I'll have a look on it 👀

@A-Yatsyk
Copy link
Author

Interesting, I think this bug was introduced in iOS 17. A similar issue was raised quite long time ago: #243

But I'll have a look on it 👀

Thank you, as this is a critical issue for working with animations. If we take into account useKeyboardHandler, KeyboardAvoidingView can be replaced with the one from the "react-native" package (for iOS for now).🤞

This library is the only one that truly helps interact with the keyboard for animations. The rest are too buggy. 😅

@kirillzyusko
Copy link
Owner

@A-Yatsyk does this problem happens on 1.10.0? It seems like the problematic code was introduced in #316, am I right?

@kirillzyusko
Copy link
Owner

@A-Yatsyk it should be fixed in #331 can you please check? 👀

@A-Yatsyk
Copy link
Author

@A-Yatsyk it should be fixed in #331 can you please check? 👀

This is better than it was. If you watch the video, you can see that there are still occasional glitches. We still have duplications of onStart/onEnd handler calls.

Screen.Recording.2024-01-13.at.13.33.33.mov

@A-Yatsyk
Copy link
Author

@kirillzyusko at the moment when glitches occur in KeyboardAvoidingView, in the logs for useKeyboardHandler, you can observe the following:

  1. onStart => { height: 291, progress: 1, ... }
  2. onEnd => { height: 291, progress: 1, ... }
  3. onStart => { height: 336, progress: 1, ... }
  4. onMove => { height: 49.28188181016594, progress: 0.14667226729216054, ... }
    ..........
  5. onMove => { height: 336, progress: 1, ... }
  6. onEnd => { height: 336, progress: 1, ... }

@kirillzyusko
Copy link
Owner

kirillzyusko commented Jan 13, 2024

Yes @A-Yatsyk

If you add keyboardWillShow listener to plain react-native module, you'll also see that native side is emitting this additional event:

 LOG  onStart {"duration": 250, "eventName": "onKeyboardMoveStart", "height": 291, "progress": 1, "target": 473}
 LOG  keyboardWillShow 291
 LOG  onStart {"duration": 250, "eventName": "onKeyboardMoveStart", "height": 336, "progress": 1, "target": 473}
 LOG  keyboardWillShow 336

The thing is that I can not filter out bad event from a native code and I have to propagate it to JS 😔 The bug was actually in useKeyboardInterpolation hook. Such case, when onStart is triggered two times with positive height value is similar to what happens when keyboard size is changed on Android. And useKeyboardInterpolation relies that after keyboard resize we'll get new range [291, 336] and keyboard height will be in this range. However it's not the case because real keyboard height will be changing from 0 to 336, and because of that interpolated value will be a negative number.

If I change interpolation mechanism, then we'll see an expected output, because these 2 events are emitted after each other and we'll interpolate to las frame (i. e. from 0 to 336). So in #331 I'm fixing KeyboardAvoidingView integration, but I don't think it's possible to fix useKeyboardHandler hook just because OS is emitting these events on its own and we don't have any way to verify whether it's valid keyboard frame or not. Does it make sense what I'm saying?

This is better than it was. If you watch the video, you can see that there are still occasional glitches. We still have duplications of onStart/onEnd handler calls.

Got you. I will try to experiment more to see why these glitches actually happen (I know it's because of duplicated events, but will try to find a way to ignore them somehow)...

P. S. such glitch happens because in onEnd we update the value, so before the next onStart we'll have fully interpolated value, i. e.:

 LOG  onStart {"duration": 250, "eventName": "onKeyboardMoveStart", "height": 291, "progress": 1, "target": 1377} 1705151792325
 LOG  0 [0, 291]
 LOG  {"bottomHeight": 0}
 LOG  keyboardWillShow 291
 LOG  keyboardWillShow 291
 LOG  onEnd {"duration": 250, "eventName": "onKeyboardMoveEnd", "height": 291, "progress": 1, "target": 1377} 1705151792333
 LOG  291 [0, 291]
 LOG  {"bottomHeight": 139}
 LOG  onStart {"duration": 250, "eventName": "onKeyboardMoveStart", "height": 336, "progress": 1, "target": 1377} 1705151792335
 LOG  291 [0, 336]
 LOG  {"bottomHeight": 159.35714285714286}
 LOG  keyboardWillShow 336
 LOG  keyboardWillShow 336
 LOG  45.18494185101008 [0, 336]
 LOG  {"bottomHeight": 24.744134823172185}

@A-Yatsyk
Copy link
Author

@kirillzyusko yes, thanks, I understand, although it's actually strange that there is an onEnd callback indicating that the keyboard was opened with a height of 291, even though that's not the case, and an update is occurring.

@A-Yatsyk
Copy link
Author

It would be good to find a way to fix this for KeyboardAvoidingView at least.

@kirillzyusko
Copy link
Owner

@A-Yatsyk I pushed f18e223 - can you again check whether it fixes all the issues?

kirillzyusko added a commit that referenced this issue Jan 15, 2024
## 📜 Description

Fixed choppy animation for `KeyboardAvoidingView` if child `TextInputs`
are using `secureTextEntry` property.

## 💡 Motivation and Context

The problem which causes jumpy behavior is hidden in fact, that 2
`keyboardWillShow` + `keyboardWillHide` events are emitted instead of a
single one. If we add logger we'll see:

```bash
onStart height 291
onEnd   height 291
onStart height 336
... // onMove
onEnd   height 336
```

And such events are very similar to what happens on Android when
keyboard gets resized. So basically fix consist from two parts.

### Part 1️⃣ 

In `useKeyboardInterpolation` we are not using Android specific
interpolation and are using "progress"-based animation (interpolation
between current keyboard frame and final keyboard frame). It allows us
to have positive value and make animation smooth. However is some cases
we may see a jump occurring for one frame, and for that we apply second
fix.

### Part 2️⃣ 

In second part we are handling "unnecessary" `onEnd` event. Before we
were simply updating `height`/`progress` values in `onEnd` handler - it
was needed to handle cases when `onMove` handler is not called but we
still need to synchronize the state.

In this PR I slightly re-worked approach and now I'm updating it only if
`duration === 0` (i. e. transition was instant).

Closes
#327

## 📢 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 -->

### JS

- use "progress"-based interpolation for iOS in
`useKeyboardInterpolation`;
- update values in `onEnd` only if `duration` is `0`.

## 🤔 How Has This Been Tested?

Tested on iPhone 15 Pro.

## 📸 Screenshots (if appropriate):

|Before|After|
|-------|-----|
|<video
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/b75f7ca4-fa84-448d-92f8-517b5886b84e">|<video
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/fe3b5cf0-8a61-4af6-ac75-ae3ce64cca72">|

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
@kirillzyusko
Copy link
Owner

@A-Yatsyk I pushed a PR with a fix into main and the issue got closed automatically. I'll publish a new 1.10.2 version in next hour. If you find that something is not working again (or working incorrectly) - feel free to re-open this issue or submit a new one 🙏

Thank you for you help in debugging this issue, active participation in resolution and the creation of such descriptive issue ❤️

@A-Yatsyk
Copy link
Author

Thank you for the prompt resolution of the issue, the problem has been resolved 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🍎 iOS iOS specific
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants