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

Use correct UIManager in findCameraView depending on the architecture #2702

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

j-piasecki
Copy link
Contributor

@j-piasecki j-piasecki commented Mar 29, 2024

What

UIManagerHelper.getUIManager has following signature:

UIManager getUIManager(ReactContext context, @UIManagerType int uiManagerType)

(Here's the source: https://github.com/facebook/react-native/blob/3f8882116782da609664f311427d7a6523ad06cf/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java#L51)

But in findCameraView it's used like this:

UIManagerHelper.getUIManager(
reactApplicationContext,
viewId
)?.resolveView(viewId) as CameraView?

where the view tag is passed instead of the UI manager type. This effectively makes it behave as if DEFAULT was passed and UIManagerModule was returned on the new arch instead of FabricUIManager.

Changes

  • Updates finding the camera view to use correct UIManager depending on the architecture

Tested on

I've only tried on Android emulator

Related issues

Potentially fixes #2613

Copy link

vercel bot commented Mar 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-vision-camera ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 29, 2024 1:36pm

Copy link
Contributor

@WoLewicki WoLewicki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice that you found it 🚀

@mrousavy
Copy link
Owner

mrousavy commented Apr 2, 2024

Wow, nice catch man - thank you so much! LGTM 💪

@mrousavy mrousavy merged commit 5223f5b into mrousavy:main Apr 2, 2024
5 checks passed
@@ -55,10 +56,11 @@ class CameraViewModule(reactContext: ReactApplicationContext) : ReactContextBase
suspendCoroutine { continuation ->
UiThreadUtil.runOnUiThread {
Log.d(TAG, "Finding view $viewId...")
val uiManagerType = if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) UIManagerType.FABRIC else UIManagerType.DEFAULT

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 👍

isaaccolson pushed a commit to isaaccolson/deliveries-mobile that referenced this pull request Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants