-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: events in bridgeless mode #3240
fix: events in bridgeless mode #3240
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - thanks for this PR, really helpful stuff! 🙏
Just one question
package/android/src/main/java/com/mrousavy/camera/react/Events.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WoLewicki Can we please rename these events to topCameraInitialized
(so including Camera) etc?
All good, already went ahead and updated the names. @mrousavy good for merge! |
@hannojg are you sure it works? I remember having some problems with such renaming, I think it is due to JS callbacks not having |
I tested it with the renaming and it was working in the example app, so should be good 😊 If you have the time feel free to double check ofc 🫶 |
package/android/src/main/java/com/mrousavy/camera/react/CameraViewManager.kt
Outdated
Show resolved
Hide resolved
package/android/src/main/java/com/mrousavy/camera/react/Events.kt
Outdated
Show resolved
Hide resolved
package/android/src/main/java/com/mrousavy/camera/react/CameraViewManager.kt
Outdated
Show resolved
Hide resolved
I committed the changes to fix the wrong names - @hannojg if you already have the test setup running could you maybe test if everything still works? Especially onPreviewStarted and onPreviewStopped |
Sure, let me test, one second |
Awesome - LGTM then - thanks @WoLewicki & @hannojg !!! ❤️ |
Hey guys, did you test it in bridgeless mode? It's where the issue exists, as mentioned in the PR title. It's much stricter than bridge mode. cc @hannojg |
Will do those tests in a moment, yes! We'll handle everything in a separate PR for new arch 😊 ! |
PR concerning New Architecture support in the library 🎉
We at Software Mansion have been working on improving support for the new architecture for quite a while now. If you need help with anything related to New Architecture, like:
or you just want to ask any questions, hit us up on [email protected]
What
PR fixing events in bridgeless mode on Android, due to their stricter convention of naming there.
Changes
Tested on
Related issues