Add device setting to control privacy permissions #423
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
jakub-gonet
reviewed
Jul 5, 2024
| @@ -395,3 +412,18 @@ function convertToSimctlSize(size: DeviceSettings["contentSize"]): string { | |||
| return "extra-extra-extra-large"; | |||
| } | |||
| } | |||
|
|
|||
| function convertToSimctlPermission(permissionType: AppPermissionType): string { | |||
There was a problem hiding this comment.
Do we expect more to come? Right this function is doing nothing as far as I can see.
There was a problem hiding this comment.
I updated it such that I have a separate list of accepted service names. The idea was that we levarage typescript to catch the case when we add new item in the future and it isn't mapping directly to the name that we can use with simctl
packages/vscode-extension/src/webview/components/DeviceSettingsDropdown.tsx
Show resolved
Hide resolved
Comment on lines
+254
to
+256
| if (build.platform !== Platform.IOS) { | ||
| throw new Error("Invalid platform"); | ||
| } |
There was a problem hiding this comment.
to narrow down the type of build from BuildResult to IosBuildResult which has some fields that we need
jakub-gonet
approved these changes
Jul 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a device setting that allows for resetting privacy permissions.
The new setting helps with scenarios where you want to test flows that rely on asking users for different types of permissions.
The current implementations supports resetting the following permissions on iOS:
On Android due to the limitations of the method we currently use, only resetting all permissions is supported.
On top of the above, the app may behave differently depending on the system. On Android, the app process is always terminated when we reset permission if at least one permission was granted prior to reset. On iOS the app never restarts.
An important consideration here is that we may want to restart the app regardless of the operating system, as the behavior where the app keeps running may lead to some unpredictable behavior. This may also end up getting the app in a state that is not expected by the developers. Typically for end-users on Android and iOS it is not possible to reset the privacy settings – you can change the setting to disallow or grant the permission later on, but you can never get it back to the state when it's unknown unless you uninstall the app. So this new option we are adding here may get the app into a state that will never be possible to replicate in real-life and therefore there is no need to handle such state for the developer.
The biggest downside of the current implementation is that we use the restart flow for the case when the permission is reset on Android. This leads to a longer than necessary time to interact with the app, as the current restart flow will also reboot the emulator, potentially trigger build and also restart metro. I plan to address this in a follow up PR, where we will add a new quicker flow for handling restarts that doesn't reset the emulator and metro.
Here's how the new dropdown menu look for iOS:
Test plan:
We're adding new location screen to expo-router example and extending that example to ask for location permission.
This has been tested on this new example with two scenarios: 1) get location info, allow, reset, get again 2) get location info, disallow, reset permission, get again. In both scenarios it is expected to get the permission popup. On Android the app will always restart when resetting the permission.