-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
feat: add possibility to listen to react model open/request close #45534
base: main
Are you sure you want to change the base?
feat: add possibility to listen to react model open/request close #45534
Conversation
Hi @maciekstosio! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
## 📜 Description Fixed a problem of keyboard movement not being detected in Modal window on Android. ## 💡 Motivation and Context The most challenging in this PR is getting an access to `dialog`. Initially I thought to create an additional view (`ModalKeyboardProvider`) and then through this view get an access to the `dialog`, but after some experiments I realized, that we'll get an access to `DialogRootViewGroup` and this class is not holding a reference to `dialog`, so this idea failed. Another approach was to listen to all events in `eventDispatcher` and when we detect `onShow` (`topShow`) event, then using `uiManager` we can resolve the view and get `ReactModalHostView`. This approach work, but it has one downside - we listen to all events. It doesn't hit the performance, but this approach looks like a workaround (it has too many transitive dependencies). A PR facebook/react-native#45534 introduces a new API for detection show/hide modals. I hope it'll be merged at some point of time and we can start to use new API. In a meantime I'll use the current approach and will incapsulate all work in `ModalAttachedWatcher` - in this case we have our own interface for dealing with modals, and we can change internals without affecting other files 😎 Closes #369 Potentially helps to fix #387 ## 📢 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 --> ### E2E - added new `modal` test; ### Android - added `ModalAttachedWatcher` class; - `KeyboardAnimationCallback` now accepts config to reduce amount of passed params (`KeyboardAnimationCallbackConfig`); - pass `eventPropagationView` to `KeyboardAnimationCallback` and `FocusedInputObserver`; - return `WindowInsetsCompat.CONSUMED` from `onApplyWindowInsets` instead of `insets`; - add `syncKeyboardPosition` to `KeyboardAnimationCallback`; - dispatch the same events through cycle instead of code duplication. ## 🤔 How Has This Been Tested? Tested manually on: - Xiaomi Redmi Note 5 Pro (Android 9); - Pixel 7 Pro (Android 14); - Pixel 3A (Android 13, emulator); - Pixel 2 (AOSP, Android 9, e2e tests); - Pixel 2 (Android 11, emulator, e2e tests). ## 📸 Screenshots (if appropriate): |Before|After| |-------|-----| |<img width="358" alt="Screenshot 2024-07-24 at 16 43 20" src="https://github.com/user-attachments/assets/bb3f5a2a-799a-4e46-af4f-4616d163ebbe">|<img width="360" alt="Screenshot 2024-07-24 at 16 41 39" src="https://github.com/user-attachments/assets/c87fffce-b8ef-4bf3-a4f3-0985e5a030ff">| ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
Summary:
At Reanimated we currently facing an issue that we're not able to listing to keyboard height changes in modal. This is due to the fact that android dialogs are attached next to the view tree, so we need to start observing insets on them separately, thus we need to listen to dialog open. To my knowledge, currently, there is no way to detect/listen when modal (dialog) is opened in a way that we can have access to its window (which we need to observe insets). The only hack that I found is listening to all events and act on “showModal”, which seems like hacky approach. In this PR I added observer that would be notified by ReactModalHostManager and would allow to attach listeners in other places. With this change in other packages we could get an instance and listen for those events. That should allow us to fix software-mansion/react-native-reanimated#5916 and may help with kirillzyusko/react-native-keyboard-controller#369.
Changelog:
[ANDROID] [ADDED] - Allow to listen to RN Modal events in native code.
Test Plan:
Tested on clean project from
npx @react-native-community/cli@next init MyApp --version next --skip-install
, which installs RN: 0.75.0-rc.5.Changes in RN Core as in the PR.
Part added to MainApplication.kt onCreate:
App.ts
Screen.Recording.2024-07-18.at.15.47.57.mov