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

network: show rationale for permissions (fixes #2089) #2156

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Okuro3499
Copy link
Contributor

fixes #2089

Screen_recording_20241111_194922.mp4

@xyb994
Copy link
Member

xyb994 commented Nov 19, 2024

I wonder if jumping to location permission (if not granted already) is the optimal flow. It sort of reminded me of myPlanet jumping to "Usage access" directly without warning upon opening. Would a pop up message asking if the user would like to be taking to location permission settings for the app to scan for wifi SSID be better?

Aside from that, it seem to crash when I try to scan -> get to the SSID list -> click on cancel -> use the wifi icon to scan again

FATAL EXCEPTION: main
Process: io.treehouses.remote, PID: 21886
java.lang.RuntimeException: Error receiving broadcast Intent { act=android.net.wifi.SCAN_RESULTS flg=0x4000010 pkg=io.treehouses.remote (has extras) } in io.treehouses.remote.fragments.dialogfragments.WifiDialogFragment$wifiBroadcastReceiver$1@8832dbf
	at android.app.LoadedApk$ReceiverDispatcher$Args.lambda$getRunnable$0$LoadedApk$ReceiverDispatcher$Args(LoadedApk.java:1581)
	at android.app.-$$Lambda$LoadedApk$ReceiverDispatcher$Args$_BumDX2UKsnxLVrE6UJsJZkotuA.run(Unknown Source:2)
	at android.os.Handler.handleCallback(Handler.java:938)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:233)
	at android.app.ActivityThread.main(ActivityThread.java:8068)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:631)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:978)
Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String android.content.Context.getPackageName()' on a null object reference
	at android.widget.Toast.<init>(Toast.java:167)
	at android.widget.Toast.makeText(Toast.java:492)
	at android.widget.Toast.makeText(Toast.java:480)
	at io.treehouses.remote.fragments.dialogfragments.WifiDialogFragment.scanFailure(WifiDialogFragment.kt:127)
	at io.treehouses.remote.fragments.dialogfragments.WifiDialogFragment.access$scanFailure(WifiDialogFragment.kt:21)
	at io.treehouses.remote.fragments.dialogfragments.WifiDialogFragment$wifiBroadcastReceiver$1.onReceive(WifiDialogFragment.kt:85)
	at android.app.LoadedApk$ReceiverDispatcher$Args.lambda$getRunnable$0$LoadedApk$ReceiverDispatcher$Args(LoadedApk.java:1566)
	at android.app.-$$Lambda$LoadedApk$ReceiverDispatcher$Args$_BumDX2UKsnxLVrE6UJsJZkotuA.run(Unknown Source:2) 
	at android.os.Handler.handleCallback(Handler.java:938) 
	at android.os.Handler.dispatchMessage(Handler.java:99) 
	at android.os.Looper.loop(Looper.java:233) 
	at android.app.ActivityThread.main(ActivityThread.java:8068) 
	at java.lang.reflect.Method.invoke(Native Method) 
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:631) 
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:978) 

@Okuro3499
Copy link
Contributor Author

I wonder if jumping to location permission (if not granted already) is the optimal flow. It sort of reminded me of myPlanet jumping to "Usage access" directly without warning upon opening. Would a pop up message asking if the user would like to be taking to location permission settings for the app to scan for wifi SSID be better?

Aside from that, it seem to crash when I try to scan -> get to the SSID list -> click on cancel -> use the wifi icon to scan again

FATAL EXCEPTION: main
Process: io.treehouses.remote, PID: 21886
java.lang.RuntimeException: Error receiving broadcast Intent { act=android.net.wifi.SCAN_RESULTS flg=0x4000010 pkg=io.treehouses.remote (has extras) } in io.treehouses.remote.fragments.dialogfragments.WifiDialogFragment$wifiBroadcastReceiver$1@8832dbf
	at android.app.LoadedApk$ReceiverDispatcher$Args.lambda$getRunnable$0$LoadedApk$ReceiverDispatcher$Args(LoadedApk.java:1581)
	at android.app.-$$Lambda$LoadedApk$ReceiverDispatcher$Args$_BumDX2UKsnxLVrE6UJsJZkotuA.run(Unknown Source:2)
	at android.os.Handler.handleCallback(Handler.java:938)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:233)
	at android.app.ActivityThread.main(ActivityThread.java:8068)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:631)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:978)
Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String android.content.Context.getPackageName()' on a null object reference
	at android.widget.Toast.<init>(Toast.java:167)
	at android.widget.Toast.makeText(Toast.java:492)
	at android.widget.Toast.makeText(Toast.java:480)
	at io.treehouses.remote.fragments.dialogfragments.WifiDialogFragment.scanFailure(WifiDialogFragment.kt:127)
	at io.treehouses.remote.fragments.dialogfragments.WifiDialogFragment.access$scanFailure(WifiDialogFragment.kt:21)
	at io.treehouses.remote.fragments.dialogfragments.WifiDialogFragment$wifiBroadcastReceiver$1.onReceive(WifiDialogFragment.kt:85)
	at android.app.LoadedApk$ReceiverDispatcher$Args.lambda$getRunnable$0$LoadedApk$ReceiverDispatcher$Args(LoadedApk.java:1566)
	at android.app.-$$Lambda$LoadedApk$ReceiverDispatcher$Args$_BumDX2UKsnxLVrE6UJsJZkotuA.run(Unknown Source:2) 
	at android.os.Handler.handleCallback(Handler.java:938) 
	at android.os.Handler.dispatchMessage(Handler.java:99) 
	at android.os.Looper.loop(Looper.java:233) 
	at android.app.ActivityThread.main(ActivityThread.java:8068) 
	at java.lang.reflect.Method.invoke(Native Method) 
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:631) 
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:978) 

the scanning crash could be fixed in another issue. for the pop up message we already have that as shown in the video the rationale is shown for gps

Screen_recording_20241125_211729.mp4

@xyb994
Copy link
Member

xyb994 commented Dec 3, 2024

@Okuro3499 With debugger, it looks like my Android 11 Phone jumped onto the else condition directly so I could not see the pop up presented in the video's 3s mark.

https://github.com/treehouses/remote/pull/2156/files#diff-4d86c6f8f9c725cb808ad5617afeb068cdba36a4338bdb6ba403ad0f29e5523cR78

@Okuro3499
Copy link
Contributor Author

@Okuro3499 With debugger, it looks like my Android 11 Phone jumped onto the else condition directly so I could not see the pop up presented in the video's 3s mark.

https://github.com/treehouses/remote/pull/2156/files#diff-4d86c6f8f9c725cb808ad5617afeb068cdba36a4338bdb6ba403ad0f29e5523cR78

it is only shown of you have not allowed location permissions. this pr is for the GPS where you are shown with the rationale

@xyb994
Copy link
Member

xyb994 commented Dec 10, 2024

Can you confirm what's in this video is the expected behavior:

  1. First time clicking on the Wi-Fi scan button (currently without location permission)
  2. direct Location permission popup by the system
  3. deny location
  4. 2nd time clicking on the Wi-Fi scan button (Location currently not allowed)
  5. "Location Permission Needed" rationale

I was trying to say wouldn't it also make sense to explain the permission before showing the direct Location permission popup by the system in the above scenario's step 2?


Another issue is if I keep denying the permission, on the 3rd attempt when you expect the "Location Permission Needed" rationale, it would just flash the screen and not showing anything.


Also I just noticed the corner border of the dialog is not darkened like the rest of the screen Screenshot_20241210_164524

@Okuro3499
Copy link
Contributor Author

@xyb994

Screen_recording_20241220_193933.mp4

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.

network: better error message for wifi scan missing location permission
2 participants