-
-
Notifications
You must be signed in to change notification settings - Fork 87
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 only] [v1.12+] onFocus
event is only called on first focus on TextInput
#456
Comments
Can you post a minimal reproduction code, please? I've used this code: export default function AwareScrollView({ navigation }: Props) {
return (
<>
<ScrollView style={{flex: 1,}} contentContainerStyle={{paddingLeft: 100, flex: 1, justifyContent: "center", alignItems: "center", backgroundColor: "red"}}>
<TextInput
key={1}
placeholder={`TextInput#${1}`}
keyboardType={1 % 2 === 0 ? "numeric" : "default"}
multiline={false}
onFocus={() => console.log("onFocus")}
onBlur={() => console.log("onBlur")}
style={{borderWidth: 2, borderColor: "black"}}
/>
</ScrollView>
</>
);
} But it always calls LOG onFocus
LOG onBlur
LOG onFocus
LOG onBlur
LOG onFocus
LOG onBlur
LOG onFocus
LOG onBlur I tested in RN 0.74 - do you think it makes a difference? |
I also tested RN 0.73.4 (old architecture) - still everything works okay 🤷♂️ Guess something wrong with my code that I'm using 🤔 |
@christophemenager I've also tested on iPhone 6s (iOS 15.8) and I can not reproduce this problem in example app (I used Toolbar screen as example). May I ask you to test my example app and maybe perform some code modifications to show how it can be reproduced? The issue seems to be quite critical, so I want to fix it ASAP, but I can not reproduce it 😔 Please, don't ignore me 🙏 |
Sorry I am a bit busy this week, can I get back to you next Monday? I will try to give you a minimal repro code :) |
Yeah, sure, take your time 👍
Thank you ❤️ |
Hi there! Sorry for the delay. I tried to reproduce in an isolated react-native environment but I could not. I guess it must be very specific to my setup, so I will close this issue and reopen it if I find a way to reproduce it properly. |
@christophemenager yes, feel free to re-open this 👍 In 1.12 version I started to inject my own delegate to intercept some events that are available only on delegate level. So I guess in some scenarios delegate substitution doesn't work properly, so it would be good if you could figure out and provide a repro 🙏 |
I'm also experiencing this issue, but only when As mentioned above reverting to |
@bikowalczyk do you think it would be possible to create a simple reproduction repository? I think it's a very critical issue but I really can't reproduce it in my example project 🤷♂️ Staying on So if you can help me and prepare a reproduction example I would highly appreciate you ❤️ |
Just for info, I had to stay on The issue is still here in latest version. |
@christophemenager yeah, the algorithm for substituting the delegate hasn't been changed since Maybe you are using specific props for |
@kirillzyusko I had a bit more time to dig into this issue and here is what I found:
I used react-native 0.75.3 for this investigation, and an iphone 13 So I guess you can easily reproduce this issue on your side following those steps:
I tried to repro on expo snacks but I could not find a way to select an ios version so it's not useful in this case ! Please let me know if I could help, but I guess the important thing here is that it's only on "old" versions of iOS, that's why it has never been reported a lot |
onFocus
event is only called on first focus on TextInputonFocus
event is only called on first focus on TextInput
@christophemenager I think I did everything properly: Screen.Recording.2024-10-08.at.22.37.29.movCan you show a video how it looks in your case? The code is: import "react-native-gesture-handler";
import * as React from "react";
import { ActivityIndicator, StatusBar, StyleSheet, TextInput } from "react-native";
import { GestureHandlerRootView } from "react-native-gesture-handler";
import { KeyboardProvider } from "react-native-keyboard-controller";
import {
SafeAreaProvider,
initialWindowMetrics,
} from "react-native-safe-area-context";
const styles = StyleSheet.create({
root: {
flex: 1,
},
});
export default function App() {
return (
<SafeAreaProvider initialMetrics={initialWindowMetrics}>
<GestureHandlerRootView style={styles.root}>
<KeyboardProvider statusBarTranslucent>
<TextInput onFocus={() => console.log("onFocus")} style={{ width: 200, height: 50, backgroundColor: "yellow", marginTop: 50 }} />
</KeyboardProvider>
</GestureHandlerRootView>
</SafeAreaProvider>
);
} Setup that I used:
Are you sure it was introduced in Any ideas what is the difference between our setups and why it doesn't work on your machine, but works on mine? The only difference that I see is that you are using Expo and I'm using pure RN project. Do you think it can be a key difference? |
@kirillzyusko thanks for your quick reply. My mistake, this issue was introduced in 1.12.0, I edited my last comment. I will keep digging and send you more info later today. |
@christophemenager thank you! Hope we eventually can figure out what's exactly happening there 👀 By the way - I also have a physical device with iOS 15.8 will try to test today and see if I can reproduce the problem there! |
It seems I managed to repro it on a simple expo example! 🎉 CleanShot.2024-10-09.at.09.55.10.mp4
Extra infos:
|
@christophemenager would you mind to publish reproduction example? And maybe you have a discord or any other messenger so that we can chat with you (I think it'll be more productive)?
So it's visible only if you trigger a keyboard via CMD+K, right? Can you say when exactly you press these keys, i. e. set focus, keyboard is not shown and then press CMD+K (according to the video)? Have you tried to reproduce it on a real device? |
I could reproduce it only one time - was super happy, but when I modified code the issue is not reproducible anymore 🤯 My brain is 🤯 |
Yes and yes, but not this week, I will get back to you next week :)
On the video record, If I remember well, I am not pressing CMD+K, I only click on the input. I made other tests in other conditions and I noticed that I could re-focus without any issue while the keyboard do not open. As soon as it opens, the onFocus is not called again. Maybe it's a coincidence, but I thought it could be a hint for you to know where to start :) |
Well, will try to do more testing, but at the moment it doesn't give me any hints 🙃 |
@kirillzyusko I will try to push a repro repo today :) |
@kirillzyusko well I tried to reproduce again this morning and... I failed 🤦 Trying around 50 times I managed to reproduce only one time, but I could not find how :/ I tried to tackle this issue from the other way: trying to simplify my project (where I can systematically reproduce) until I solve the issue so I could find what is exactly is causing the issue in my current project setup. My feeling is that it could come from an other dependency installed in my project that messes up with yours, but I am really not sure. The only thing I know for sure is that in my project, I reproduced on iOS 15.2 but not on 16,17 and 18. As
I suggest we close the issue for now and wait for more inputs if others have the same problem. |
@christophemenager would you mind sharing your |
Sure, here it is:
|
@christophemenager okay, I don't see anything suspicious 🙈 If you can create an empty project, add all these dependencies and the issue will be reproducible - let me know and I'll have a look on a reproduction example. As you suggested - I'll close the issue, but if you (or anyone else) find anything new to share, feel free to do it here and I'll be happy to re-open that issue! |
@christophemenager I don't give up. Just a random thought came to my mind - maybe RN substitute delegate on its own level? In this case when I try to substitute delegate back most likely the reference (to the delegate that I hold) becomes Can you try to use this code in your project: private func substituteDelegateBack(_ input: UIResponder?) {
if let textField = input as? UITextField {
if (textField.delegate is KCTextInputCompositeDelegate) {
textField.delegate = delegate.activeDelegate as? UITextFieldDelegate
}
} else if let textView = input as? UITextView {
if (textView.delegate is KCTextInputCompositeDelegate) {
(textView as? RCTUITextView)?.setForceDelegate(delegate.activeDelegate as? UITextViewDelegate)
}
}
} And test the scenario that 100% reproducible in your project? |
@kirillzyusko ahah such a warrior fighting again this bug, I admire that ;) Sure, can you just give me the path of the file where I should add this piece of code? |
@christophemenager sure, it's located here: react-native-keyboard-controller/ios/observers/FocusedInputObserver.swift Lines 206 to 212 in 56cd863
You need to replace this function with a code that I provided 🙂 |
@kirillzyusko good news, it fixes the issue !! 🎉
We can't see it but on the "before" video I am pressing multiple times on the first inputs but they are not getting focused. I made sure to revert the change and I reproduced the issue again so I confirm this piece of codes you gave me fixes the issue ! |
Amazing @christophemenager ! 🎉 I'll submit a PR with a fix today then and will publish a fix under May I also ask you to test this variant of the code? private func substituteDelegateBack(_ input: UIResponder?) {
if let textField = input as? UITextField, let oldDelegate = delegate.activeDelegate as? UITextFieldDelegate {
textField.delegate = oldDelegate
} else if let textView = input as? UITextView, let oldDelegate = delegate.activeDelegate as? UITextViewDelegate {
(textView as? RCTUITextView)?.setForceDelegate(oldDelegate)
}
} Just want to understand better what's going wrong internally to apply a better suitable fix 🙃 |
It fixes the issue too :) |
## 📜 Description Fixes a problem when all events such as `onFocus`/`onBlur`/etc. will stop working after keyboard dismissal. ## 💡 Motivation and Context It looks like a delegate in original `TextInput` can be somehow substituted and we are loosing a pointer to original delegate. If on keyboard-hide event we'll substitute delegate with `nil` - we will make all events, such as `onFocus`, `onBlur` etc. not coming to JS. So the most obvious decision is to check, if we actually have a delegate and do a substitution only in this case. Of course a rhetorical question arises - if we had original delegate, then injected our delegate, then it was substituted with a new one -> turns out that our events will not come, right? Right. But: - I don't know how it's possible - I checked RN sources and I don't see a way how delegate can be injected after component creation; - for now it's still safer to miss events in `keyboard-controller` rather than disable all events at framework level; - I can not reliably reproduce this in empty project and the issue is reproducible on relatively old devices (iO 15.2). So yeah, the problem is somewhere deeper, but I don't have an access to the code so can't debug what exactly happens there. So for now it'll be safer not to ruin the RN framework and merge this fix 🙃 Closes #456 ## 📢 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 --> ### iOS - check previous delegate presence when substitute delegate back; ## 🤔 How Has This Been Tested? Tested manually on iOS 15.2 ## 📸 Screenshots (if appropriate): |Before|After| |---|---| |<video src="https://github.com/user-attachments/assets/a12dcf98-b3ea-4d6f-9c02-c0d132c4e3ac">|<video src="https://github.com/user-attachments/assets/06b1ed57-aa11-4e13-b11d-8233aa5a0bdf">| ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
Describe the bug
onFocus
is only called once, and not if text input is re-focusedonBlur
eventTo Reproduce
Steps to reproduce the behavior:
Expected behavior
onFocus and onBlur events should be called everytime the input is focused or blurred
Smartphone (please complete the following information):
Additional context
This regression has been introduced in 1.12, 1.11 is working fine
The text was updated successfully, but these errors were encountered: