Skip to content

Enable ScreenSet WebView debugging in Debug mode #85

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

MewanWA
Copy link
Contributor

@MewanWA MewanWA commented Nov 14, 2024

Aims to resolve the issue (#84) where we are not able to interact with the Gigya ScreenSet webview as the debuggable property is not set. This issue is faced if we are using Appium with the Appium Flutter driver. Within the NATIVE_APP context we can access the webview, but not in FLUTTER context.

Implemented Fix
Accept a isDebug value in showScreenSet SDK function. If this is true, we will set GigyaLogger to debug in iOS and in Android we will call WebView.setWebContentsDebuggingEnabled(true).

Alternate Implementation
Instead of adding a new isDebug bool value, we can also use BuildConfig.DEBUG (Android) or #if DEBUG (iOS). Although this would make it a bit restricted for the users as those values are tied to the build rather than what the Flutter app passes via a function. But in most cases, this would work fine.

@MewanWA
Copy link
Contributor Author

MewanWA commented Nov 15, 2024

SAP/gigya-android-sdk#79

I have created a PR on the Android SDK so both iOS and Android can use the same solution on the gigya-flutter-plugin.

@mcg95
Copy link

mcg95 commented Dec 3, 2024

@navaronbracke @tal-mi Sorry to disturb, but if you have time, can this one be given a quick review?

@@ -723,6 +725,10 @@ class GigyaSDKWrapper<T : GigyaAccount>(application: Application, accountObj: Cl
arguments["parameters"] as MutableMap<String, Any>?
?: mutableMapOf()

(arguments["isDebug"] as? Boolean)?.let { isDebug ->
if (isDebug) WebView.setWebContentsDebuggingEnabled(true).also { Log.d("SCREENSET_TAG", "Screenset Debuggable") }
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need the Log.d() here?

Copy link

Choose a reason for hiding this comment

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

I have removed all the log functions from the PR


PODFILE CHECKSUM: cc1f88378b4bfcf93a6ce00d2c587857c6008d3b

COCOAPODS: 1.13.0
COCOAPODS: 1.15.2
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC Cocoapods currently recommends 1.16.0 ?

Copy link

Choose a reason for hiding this comment

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

Update my Cocoapods to the latest and now it references 1.16.2

@@ -244,7 +245,12 @@ class _HomePageState extends State<HomePage> with WidgetsBindingObserver {
const String screenSet = 'Default-RegistrationLogin1';

try {
screenSetSubscription = widget.sdk.showScreenSet(screenSet).listen(
screenSetSubscription = widget.sdk
Copy link
Contributor

Choose a reason for hiding this comment

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

The formatting is a bit off, due to the last comma?

Suggested change
screenSetSubscription = widget.sdk
screenSetSubscription = widget.sdk.showScreenSet(screenSet, isDebug: kDebugMode)

Copy link

Choose a reason for hiding this comment

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

Fixed this as well

guard let screenSet = arguments["screenSet"] as? String
if let isDebug = arguments["isDebug"] as? Bool, isDebug {
GigyaLogger.setDebugMode(to: isDebug)
print("Screenset Debuggable")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the print here?

@@ -5,6 +5,8 @@ import android.app.Application
import android.content.pm.PackageInfo
import android.content.pm.PackageManager
import android.os.Build
import android.util.Log
Copy link
Contributor

Choose a reason for hiding this comment

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

The Log import can be removed.

Suggested change
import android.util.Log

Copy link

Choose a reason for hiding this comment

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

My bad. I missed that. Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants