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: missing unfocus event on iOS Fabric #799

Merged
merged 3 commits into from
Feb 7, 2025

Conversation

kirillzyusko
Copy link
Owner

@kirillzyusko kirillzyusko commented Feb 4, 2025

📜 Description

Fixed a problem when input looses a focus, but FocusedInputObserver doesn't send noFocusedInputEvent to JS.

💡 Motivation and Context

The problem was introduced in this PR #760 before we were relying on keyboard evens, but in #760 I re-worked the approach and started to use blur/focus events. However I couldn't use only input events, because on iOS 15 keyboard event (keyboardWillShow) was arriving earlier than focus event, and for us it's crucial to know the layout of focused input inside onStart (i. e. before keyboard movement starts).

I decided to keep backward compatibility and keep keyboard events in place. However it comes with own set of challenges. For example:

image

This check works well for Paper, but doesn't work for Fabric. And for focus events we can keep two event sources, because logic for focus is simple -> "we got a focus -> save the state and push event to JS". For blur it's different:

  • keyboard can be closed but focus still can be present;
  • when we switch between inputs we also receive blur event but we should ignore it;
  • we may have a random order of events including some missing events.

Initially I don't know how to fix it. But then I realized, that on Android we already have a similar situation. We can looses a focus and keyboard will be focus, we can have a focus and close a keyboard, so overall it's a very similar to what we can have on iOS right now. But why I don't experience any problems on Android? Because I just rely on "focus"/"blur" events there (and actually relying on keyboardWillShow on iOS is needed to have a stable order of events in JS). So at this point of time I thought "what if I remove keyboardWillHide handler?". Yes, it will not work as before (some events may be delayed), but for dismissing keyboard it's not so important at the moment (because on Android we can also have any order of those events).

So after thinking a lot I decided that it can be safe to remove that handler completely and reduce logic complexity presented in this class.

Closes #798

📢 Changelog

iOS

  • rely on blur event only for sending noFocusedInputEvent (remove keyboardWillHide event);
  • check in blur event UIResponder.current presence instead of self.currentResponder.

🤔 How Has This Been Tested?

Tested manually on iPhone 15 Pro iOS 17.5

📸 Screenshots (if appropriate):

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2025-02-07.at.10.41.32.mp4

📝 Checklist

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

Sorry, something went wrong.

@kirillzyusko kirillzyusko added 🍎 iOS iOS specific 🏭 fabric Changes specific to new (fabric/jsi) architecture focused input 📝 Anything about focused input functionality labels Feb 4, 2025
@kirillzyusko kirillzyusko self-assigned this Feb 4, 2025
Copy link
Contributor

github-actions bot commented Feb 4, 2025

📊 Package size report

Current size Target Size Difference
167369 bytes 167503 bytes -134 bytes 📉

@kirillzyusko kirillzyusko changed the title fix: no unfocus event on iOS Fabric fix: missing unfocus event on iOS Fabric Feb 6, 2025
@kirillzyusko kirillzyusko marked this pull request as ready for review February 7, 2025 09:51
@kirillzyusko kirillzyusko merged commit 83bd3c1 into main Feb 7, 2025
15 checks passed
@kirillzyusko kirillzyusko deleted the fix/accident-view-memoization branch February 7, 2025 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏭 fabric Changes specific to new (fabric/jsi) architecture focused input 📝 Anything about focused input functionality 🍎 iOS iOS specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent KeyboardAwareScrollView scrolling up when focused on the same input
1 participant