-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor showInAppMessage to properly handle suspend function [SC-167747] #28
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
Conversation
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.
Pull Request Overview
This PR refactors the showInAppMessage method in the Android SDK to properly handle suspend functions by using coroutines and lifecycle-aware components. The refactor addresses proper coroutine handling for asynchronous operations while adding error handling.
- Wrapped the
MIClient.showInAppBrowsercall in a coroutine scope to handle suspend functions properly - Added lifecycle-aware coroutine launching using
lifecycleScope - Enhanced error handling with try-catch block and proper UI thread callback execution
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Version bump from 2.1.0 to 2.1.1 |
| RNMovableInkModule.kt | Refactored showInAppMessage to use coroutines with lifecycle scope and added error handling |
| build.gradle | Added androidx.lifecycle:lifecycle-runtime-ktx dependency for lifecycle-aware coroutines |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| val activity = currentActivity | ||
| if (activity is androidx.lifecycle.LifecycleOwner) { |
Copilot
AI
Sep 3, 2025
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.
Missing else branch to handle the case when activity is not a LifecycleOwner. The callback should be invoked with an error message to prevent the caller from waiting indefinitely.
| callback.invoke(value) | ||
| @ReactMethod | ||
| fun showInAppMessage(url: String, callback: Callback) { | ||
| val activity = currentActivity |
Copilot
AI
Sep 3, 2025
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.
Missing null check for currentActivity. If currentActivity is null, the callback should be invoked with an error message to handle this edge case properly.
| val activity = currentActivity | |
| val activity = currentActivity | |
| if (activity == null) { | |
| callback.invoke("Error: currentActivity is null") | |
| return | |
| } |
No description provided.