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

fix: do not substitute delegate back if it's nil #642

Merged

Conversation

kirillzyusko
Copy link
Owner

@kirillzyusko kirillzyusko commented Oct 16, 2024

📜 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

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
before.mp4
after.mp4

📝 Checklist

  • CI successfully passed
  • I added new mocks and corresponding unit-tests if library API was changed

@kirillzyusko kirillzyusko added 🐛 bug Something isn't working 🍎 iOS iOS specific focused input 📝 Anything about focused input functionality labels Oct 16, 2024
@kirillzyusko kirillzyusko self-assigned this Oct 16, 2024
Copy link
Contributor

📊 Package size report

Current size Target Size Difference
157531 bytes 157518 bytes 13 bytes 📈

@kirillzyusko kirillzyusko merged commit 4056513 into main Oct 17, 2024
16 checks passed
@kirillzyusko kirillzyusko deleted the fix/do-not-substitute-delegate-back-if-it-is-already-null branch October 17, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working focused input 📝 Anything about focused input functionality 🍎 iOS iOS specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS only] [v1.12+] onFocus event is only called on first focus on TextInput
1 participant