From f5a562ed1e630779e69217c9aec4e0bdafc870ee Mon Sep 17 00:00:00 2001 From: temcguir Date: Wed, 30 Sep 2026 22:52:44 +0000 Subject: [PATCH 1/4] Add location permission UX to onboarding and settings Adds an optional location permission request step to the onboarding flow and introduces a "Save Location" toggle switch in the in-app Settings screen. Key changes: - In `:feature:permissions`: - Adds `PermissionEnum.LOCATION` as an optional permission card. - Implements `PermissionsRepository` to track dismissed/requested permissions across sessions. - Adds `PermissionsScreenComponents` for location permission rationale dialog and settings deep-linking. - When granted during onboarding, updates `SettingsRepository.updateLocationEnabled(true)` automatically. - In `:feature:settings`: - Adds `LocationSettingComponent` ("Save Location" toggle) to the Settings screen. - Adds `LocationUiState` (Hidden when no LocationProvider is available, Enabled.On/Off when present). - Handles runtime permission requesting when toggled on, and displays a rationale dialog directing users to system app settings if permanently denied. - In `:app`: - Binds `PermissionsRepositoryImpl` via `PermissionsModule`. - Passes `onOpenAppSettings` from `MainActivity` / `JcaApp` to `SettingsScreen`. - Adds unit and Compose UI tests for onboarding and settings screens and ViewModels. --- .../jetpackcamera/di/PermissionsModule.kt | 33 +++ .../com/google/jetpackcamera/ui/JcaApp.kt | 3 + feature/permissions/build.gradle.kts | 16 ++ .../permissions/PermissionsEnums.kt | 46 +++- .../permissions/PermissionsScreen.kt | 21 +- .../permissions/PermissionsViewModel.kt | 87 +++++- .../permissions/data/PermissionsRepository.kt | 72 +++++ .../ui/PermissionsScreenComponents.kt | 63 ++++- .../jetpackcamera/permissions/ui/TestTags.kt | 1 + .../src/main/res/drawable/ic_location_on.xml | 29 ++ .../src/main/res/values/strings.xml | 3 + .../permissions/PermissionsScreenTest.kt | 224 ++++++++++++++++ .../permissions/PermissionsTestFakes.kt | 57 ++++ .../permissions/PermissionsViewModelTest.kt | 253 ++++++++++++++++++ feature/settings/build.gradle.kts | 3 + .../jetpackcamera/settings/SettingsScreen.kt | 154 ++++++++++- .../jetpackcamera/settings/SettingsUiState.kt | 38 +++ .../settings/SettingsViewModel.kt | 46 +++- .../settings/ui/SettingsComponents.kt | 51 ++++ .../jetpackcamera/settings/ui/TestTags.kt | 5 + .../settings/src/main/res/values/strings.xml | 11 + .../CameraAppSettingsViewModelTest.kt | 118 +++++++- .../settings/SettingsScreenTest.kt | 210 +++++++++++++++ 23 files changed, 1504 insertions(+), 40 deletions(-) create mode 100644 app/src/main/java/com/google/jetpackcamera/di/PermissionsModule.kt create mode 100644 feature/permissions/src/main/java/com/google/jetpackcamera/permissions/data/PermissionsRepository.kt create mode 100644 feature/permissions/src/main/res/drawable/ic_location_on.xml create mode 100644 feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsScreenTest.kt create mode 100644 feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsTestFakes.kt create mode 100644 feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsViewModelTest.kt create mode 100644 feature/settings/src/test/java/com/google/jetpackcamera/settings/SettingsScreenTest.kt diff --git a/app/src/main/java/com/google/jetpackcamera/di/PermissionsModule.kt b/app/src/main/java/com/google/jetpackcamera/di/PermissionsModule.kt new file mode 100644 index 0000000000..36767e638a --- /dev/null +++ b/app/src/main/java/com/google/jetpackcamera/di/PermissionsModule.kt @@ -0,0 +1,33 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.jetpackcamera.di + +import com.google.jetpackcamera.permissions.data.DataStorePermissionsRepository +import com.google.jetpackcamera.permissions.data.PermissionsRepository +import dagger.Binds +import dagger.Module +import dagger.hilt.InstallIn +import dagger.hilt.components.SingletonComponent + +/** + * Dagger [Module] for permission onboarding dependencies. + */ +@Module +@InstallIn(SingletonComponent::class) +internal interface PermissionsModule { + @Binds + fun bindPermissionsRepository(impl: DataStorePermissionsRepository): PermissionsRepository +} diff --git a/app/src/main/java/com/google/jetpackcamera/ui/JcaApp.kt b/app/src/main/java/com/google/jetpackcamera/ui/JcaApp.kt index f0e62abc8f..44d4218a02 100644 --- a/app/src/main/java/com/google/jetpackcamera/ui/JcaApp.kt +++ b/app/src/main/java/com/google/jetpackcamera/ui/JcaApp.kt @@ -102,6 +102,8 @@ private fun JetpackCameraNavHost( add(android.Manifest.permission.CAMERA) if (externalCaptureMode == ExternalCaptureMode.Standard) { add(android.Manifest.permission.RECORD_AUDIO) + add(android.Manifest.permission.ACCESS_FINE_LOCATION) + add(android.Manifest.permission.ACCESS_COARSE_LOCATION) if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { add(android.Manifest.permission.WRITE_EXTERNAL_STORAGE) } @@ -152,6 +154,7 @@ private fun JetpackCameraNavHost( buildType = BuildConfig.BUILD_TYPE ), onNavigateBack = { navController.popBackStack() }, + onOpenAppSettings = onOpenAppSettings, cameraSettingsSlot = { DefaultCameraSettings( customEffectSlot = { diff --git a/feature/permissions/build.gradle.kts b/feature/permissions/build.gradle.kts index ac9534e10b..7e7ab51678 100644 --- a/feature/permissions/build.gradle.kts +++ b/feature/permissions/build.gradle.kts @@ -31,6 +31,7 @@ android { defaultConfig { minSdk = libs.versions.minSdk.get().toInt() + testOptions.targetSdk = libs.versions.targetSdk.get().toInt() testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" consumerProguardFiles("consumer-rules.pro") @@ -48,6 +49,12 @@ android { buildConfig = true compose = true } + testOptions { + unitTests { + isReturnDefaultValues = true + isIncludeAndroidResources = true + } + } } dependencies { @@ -62,6 +69,7 @@ dependencies { // Compose - Android Studio Preview support implementation(libs.compose.ui.tooling.preview) debugImplementation(libs.compose.ui.tooling) + debugImplementation(libs.compose.test.manifest) // Compose - Integration with ViewModels with Navigation and Hilt implementation(libs.androidx.navigation.compose) @@ -69,6 +77,7 @@ dependencies { // Compose - Testing + testImplementation(libs.compose.junit) androidTestImplementation(libs.compose.junit) // Accompanist - Permissions @@ -81,7 +90,14 @@ dependencies { implementation(libs.androidx.core.ktx) implementation(libs.androidx.appcompat) + implementation(libs.androidx.datastore.preferences) + implementation(project(":data:settings")) + testImplementation(project(":core:settings")) + testImplementation(project(":data:settings:testing")) + testImplementation(libs.kotlinx.coroutines.test) testImplementation(libs.junit) + testImplementation(libs.robolectric) + testImplementation(libs.truth) androidTestImplementation(libs.androidx.junit) androidTestImplementation(libs.androidx.espresso.core) } diff --git a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsEnums.kt b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsEnums.kt index acf3f286c1..e02efdb172 100644 --- a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsEnums.kt +++ b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsEnums.kt @@ -22,6 +22,7 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.graphics.painter.Painter import androidx.compose.ui.res.painterResource import com.google.jetpackcamera.permissions.ui.CAMERA_PERMISSION_BUTTON +import com.google.jetpackcamera.permissions.ui.LOCATION_PERMISSION_BUTTON import com.google.jetpackcamera.permissions.ui.RECORD_AUDIO_PERMISSION_BUTTON import com.google.jetpackcamera.permissions.ui.WRITE_EXTERNAL_STORAGE_PERMISSION_BUTTON @@ -37,6 +38,18 @@ sealed interface PermissionInfoProvider { */ fun getPermission(): String + /** + * Returns the complete list of system runtime permissions associated with this entry. + * + * Supports composite permission requests (e.g., coarse and fine location). + */ + fun getPermissions(): List = listOf(getPermission()) + + /** + * Whether this permission is optional during onboarding. + * + * Optional permissions can be declined or skipped without blocking app navigation. + */ fun isOptional(): Boolean fun getTestTag(): String @@ -122,11 +135,42 @@ enum class PermissionEnum : PermissionInfoProvider { override fun getIconAccessibilityTextResId(): Int = R.string.write_storage_permission_accessibility_text + }, + + /** + * Location permission entry requesting both [Manifest.permission.ACCESS_FINE_LOCATION] + * and [Manifest.permission.ACCESS_COARSE_LOCATION]. + * + * Marked as optional so users can opt out during initial onboarding. + */ + LOCATION { + override fun getPermission(): String = Manifest.permission.ACCESS_FINE_LOCATION + + override fun getPermissions(): List = listOf( + Manifest.permission.ACCESS_FINE_LOCATION, + Manifest.permission.ACCESS_COARSE_LOCATION + ) + + override fun isOptional(): Boolean = true + + override fun getTestTag(): String = LOCATION_PERMISSION_BUTTON + + override fun getDrawableResId(): Int = R.drawable.ic_location_on + + override fun getPermissionTitleResId(): Int = R.string.location_permission_screen_title + + override fun getPermissionBodyTextResId(): Int = + R.string.location_permission_required_rationale + + override fun getRationaleBodyTextResId(): Int? = null + + override fun getIconAccessibilityTextResId(): Int = + R.string.location_permission_accessibility_text }; companion object { fun fromString(permission: String): PermissionEnum = - entries.firstOrNull { it.getPermission() == permission } + entries.firstOrNull { it.getPermissions().contains(permission) } ?: throw IllegalArgumentException("Unknown permission: $permission") } } diff --git a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsScreen.kt b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsScreen.kt index 6cc0ab023d..712535089f 100644 --- a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsScreen.kt +++ b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsScreen.kt @@ -20,7 +20,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue -import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.runtime.key import androidx.compose.ui.Modifier import androidx.hilt.navigation.compose.hiltViewModel import com.google.accompanist.permissions.ExperimentalPermissionsApi @@ -51,7 +51,7 @@ fun PermissionsScreen( } } - LaunchedEffect(permissionStates) { + LaunchedEffect(permissionStates.permissions.map { it.status }) { viewModel.updatePermissionStates(permissionStates) } @@ -59,12 +59,15 @@ fun PermissionsScreen( val permissionEnum = (permissionsUiState as PermissionsUiState.PermissionsNeeded).currentPermission - val currentPermissionStates by rememberUpdatedState(permissionStates) - PermissionTemplate( - modifier = modifier, - permissionEnum = permissionEnum, - onDismissPermission = { viewModel.updatePermissionStates(currentPermissionStates) }, - onOpenAppSettings = onOpenAppSettings - ) + key(permissionEnum) { + PermissionTemplate( + modifier = modifier, + permissionEnum = permissionEnum, + onDismissPermission = { + viewModel.dismissPermission(permissionEnum) + }, + onOpenAppSettings = onOpenAppSettings + ) + } } } diff --git a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsViewModel.kt b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsViewModel.kt index 4ed95b8801..a12794e277 100644 --- a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsViewModel.kt +++ b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsViewModel.kt @@ -22,7 +22,9 @@ import com.google.accompanist.permissions.ExperimentalPermissionsApi import com.google.accompanist.permissions.MultiplePermissionsState import com.google.accompanist.permissions.isGranted import com.google.accompanist.permissions.shouldShowRationale +import com.google.jetpackcamera.permissions.data.PermissionsRepository import com.google.jetpackcamera.permissions.navigation.getRequestablePermissions +import com.google.jetpackcamera.settings.SettingsRepository import dagger.hilt.android.lifecycle.HiltViewModel import javax.inject.Inject import kotlinx.coroutines.flow.MutableStateFlow @@ -31,6 +33,7 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch /** * A [ViewModel] for [PermissionsScreen]] @@ -38,7 +41,9 @@ import kotlinx.coroutines.flow.update @OptIn(ExperimentalPermissionsApi::class) @HiltViewModel() class PermissionsViewModel @Inject constructor( - savedStateHandle: SavedStateHandle + savedStateHandle: SavedStateHandle, + private val settingsRepository: SettingsRepository, + private val permissionsRepository: PermissionsRepository ) : ViewModel() { // Initialize required permissions from savedStateHandle. Assume all permissions are not yet @@ -58,9 +63,62 @@ class PermissionsViewModel @Inject constructor( PermissionsUiState.PermissionsNeeded(first()) } + private var lastLocationGranted: Boolean? = null + private val dismissedPermissions = mutableSetOf() + private var requestedPermissions = emptySet() + private var latestPermissionsState: MultiplePermissionsState? = null + + init { + viewModelScope.launch { + permissionsRepository.requestedPermissions.collect { requested -> + requestedPermissions = requested + latestPermissionsState?.let { state -> + recomputeQueue(state) + } + } + } + } + + /** + * Dismisses [permission] without granting it. + * + * The permission is removed from the current queue, excluded for the rest of this session, + * and recorded as requested in [PermissionsRepository] so it is not shown again. + * + * @param permission The permission the user chose to skip. + */ + internal fun dismissPermission(permission: PermissionEnum) { + dismissedPermissions.add(permission) + viewModelScope.launch { + permissionsRepository.markPermissionRequested(permission.name) + } + permissionQueue.update { queue -> + queue.filter { it != permission } + } + } + fun updatePermissionStates(multiplePermissionsState: MultiplePermissionsState) { + latestPermissionsState = multiplePermissionsState + val isLocationGranted = multiplePermissionsState.permissions.any { + it.permission in PermissionEnum.LOCATION.getPermissions() && it.status.isGranted + } + + if (lastLocationGranted == false && isLocationGranted) { + viewModelScope.launch { + settingsRepository.updateLocationEnabled(true) + } + } + lastLocationGranted = isLocationGranted + + recomputeQueue(multiplePermissionsState) + } + + private fun recomputeQueue(multiplePermissionsState: MultiplePermissionsState) { permissionQueue.update { - getRequestablePermissions(multiplePermissionsState) + getRequestablePermissions( + multiplePermissionsState, + requestedPermissions + ).filter { it !in dismissedPermissions } } } } @@ -73,14 +131,27 @@ class PermissionsViewModel @Inject constructor( * - optional permissions that have not yet been denied by the user */ @OptIn(ExperimentalPermissionsApi::class) -fun getRequestablePermissions(permissionStates: MultiplePermissionsState): List = - buildList { - permissionStates.permissions.forEach { permissionState -> - val permission = PermissionEnum.fromString(permissionState.permission) - if (!permissionState.status.isGranted) { - if (!permission.isOptional() || !permissionState.status.shouldShowRationale) { +fun getRequestablePermissions( + permissionStates: MultiplePermissionsState, + requestedPermissions: Set = emptySet() +): List = buildSet { + for (permissionState in permissionStates.permissions) { + val permission = PermissionEnum.fromString(permissionState.permission) + val componentStates = permission.getPermissions().mapNotNull { permStr -> + permissionStates.permissions.firstOrNull { it.permission == permStr } + } + val isAnyGranted = componentStates.any { it.status.isGranted } + if (!isAnyGranted) { + if (!permission.isOptional()) { + add(permission) + } else { + val wasPreviouslyHandled = + permission.name in requestedPermissions || + componentStates.any { it.status.shouldShowRationale } + if (!wasPreviouslyHandled) { add(permission) } } } } +}.toList() diff --git a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/data/PermissionsRepository.kt b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/data/PermissionsRepository.kt new file mode 100644 index 0000000000..1d31019a15 --- /dev/null +++ b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/data/PermissionsRepository.kt @@ -0,0 +1,72 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.jetpackcamera.permissions.data + +import android.content.Context +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringSetPreferencesKey +import androidx.datastore.preferences.preferencesDataStore +import dagger.hilt.android.qualifiers.ApplicationContext +import javax.inject.Inject +import javax.inject.Singleton +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.map + +/** + * Persists which optional permissions have already been presented to the user, so they are not + * prompted again across app restarts. + */ +interface PermissionsRepository { + /** + * Emits the [com.google.jetpackcamera.permissions.PermissionEnum] names that have been + * presented to the user. + */ + val requestedPermissions: Flow> + + /** + * Records that a permission has been presented to the user. + * + * @param permissionName The [com.google.jetpackcamera.permissions.PermissionEnum.name] to + * record. + */ + suspend fun markPermissionRequested(permissionName: String) +} + +private val Context.permissionsDataStore: DataStore by preferencesDataStore( + name = "jca_permissions" +) + +private val KEY_REQUESTED_PERMISSIONS = stringSetPreferencesKey("requested_permissions") + +/** [PermissionsRepository] backed by a Preferences [DataStore]. */ +@Singleton +class DataStorePermissionsRepository @Inject constructor( + @ApplicationContext context: Context +) : PermissionsRepository { + private val dataStore = context.permissionsDataStore + + override val requestedPermissions: Flow> = + dataStore.data.map { prefs -> prefs[KEY_REQUESTED_PERMISSIONS] ?: emptySet() } + + override suspend fun markPermissionRequested(permissionName: String) { + dataStore.edit { prefs -> + prefs[KEY_REQUESTED_PERMISSIONS] = + (prefs[KEY_REQUESTED_PERMISSIONS] ?: emptySet()) + permissionName + } + } +} diff --git a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/PermissionsScreenComponents.kt b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/PermissionsScreenComponents.kt index e18c241e1f..9e1f74bf73 100644 --- a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/PermissionsScreenComponents.kt +++ b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/PermissionsScreenComponents.kt @@ -34,6 +34,11 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.key +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color @@ -45,8 +50,9 @@ import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import com.google.accompanist.permissions.ExperimentalPermissionsApi +import com.google.accompanist.permissions.MultiplePermissionsState import com.google.accompanist.permissions.isGranted -import com.google.accompanist.permissions.rememberPermissionState +import com.google.accompanist.permissions.rememberMultiplePermissionsState import com.google.accompanist.permissions.shouldShowRationale import com.google.jetpackcamera.permissions.PermissionEnum import com.google.jetpackcamera.permissions.R @@ -64,13 +70,37 @@ fun PermissionTemplate( onDismissPermission: () -> Unit, onOpenAppSettings: () -> Unit ) { - val permissionState = rememberPermissionState(permissionEnum.getPermission()) + key(permissionEnum) { + val permissionStates = rememberMultiplePermissionsState(permissionEnum.getPermissions()) + PermissionTemplate( + modifier = modifier, + permissionEnum = permissionEnum, + permissionStates = permissionStates, + onDismissPermission = onDismissPermission, + onOpenAppSettings = onOpenAppSettings + ) + } +} - // LaunchedEffect will skip permission enum if already granted. - LaunchedEffect(permissionState.status) { - if (permissionState.status.isGranted || - (permissionState.status.shouldShowRationale && permissionEnum.isOptional()) - ) { +@OptIn(ExperimentalPermissionsApi::class) +@Composable +internal fun PermissionTemplate( + permissionEnum: PermissionEnum, + permissionStates: MultiplePermissionsState, + onDismissPermission: () -> Unit, + onOpenAppSettings: () -> Unit, + modifier: Modifier = Modifier +) { + var hasAttemptedRequest by rememberSaveable(permissionEnum) { mutableStateOf(false) } + + val isAnyGranted = permissionStates.permissions.any { it.status.isGranted } + val canShowRationale = + permissionStates.shouldShowRationale || + permissionStates.permissions.any { it.status.shouldShowRationale } + + // LaunchedEffect will skip permission enum if already granted or declined. + LaunchedEffect(isAnyGranted, canShowRationale) { + if (isAnyGranted || (canShowRationale && permissionEnum.isOptional())) { onDismissPermission() } } @@ -79,10 +109,19 @@ fun PermissionTemplate( modifier = modifier, testTag = permissionEnum.getTestTag(), onRequestPermission = { - if (permissionState.status.shouldShowRationale) { - onOpenAppSettings() + if (permissionEnum.isOptional()) { + if (hasAttemptedRequest && !canShowRationale) { + onDismissPermission() + } else { + hasAttemptedRequest = true + permissionStates.launchMultiplePermissionRequest() + } } else { - permissionState.launchPermissionRequest() + if (permissionStates.shouldShowRationale) { + onOpenAppSettings() + } else { + permissionStates.launchMultiplePermissionRequest() + } } }, painter = permissionEnum.getPainter(), @@ -91,13 +130,13 @@ fun PermissionTemplate( // if declined by user, must navigate to system app settings to enable permission bodyText = - if (!permissionState.status.shouldShowRationale || permissionEnum.isOptional()) { + if (!permissionStates.shouldShowRationale || permissionEnum.isOptional()) { stringResource(id = permissionEnum.getPermissionBodyTextResId()) } else { stringResource(id = permissionEnum.getRationaleBodyTextResId()!!) }, requestButtonText = - if (!permissionState.status.shouldShowRationale || permissionEnum.isOptional()) { + if (!permissionStates.shouldShowRationale || permissionEnum.isOptional()) { stringResource(id = R.string.request_permission) } else { stringResource(id = R.string.navigate_to_settings) diff --git a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/TestTags.kt b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/TestTags.kt index 7e8f8bbfe3..3c49634705 100644 --- a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/TestTags.kt +++ b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/TestTags.kt @@ -20,3 +20,4 @@ const val CAMERA_PERMISSION_BUTTON = "CameraPermissionButton" const val RECORD_AUDIO_PERMISSION_BUTTON = "RecordAudioPermissionButton" const val WRITE_EXTERNAL_STORAGE_PERMISSION_BUTTON = "WriteExternalStoragePermissionButton" +const val LOCATION_PERMISSION_BUTTON = "LocationPermissionButton" diff --git a/feature/permissions/src/main/res/drawable/ic_location_on.xml b/feature/permissions/src/main/res/drawable/ic_location_on.xml new file mode 100644 index 0000000000..a0902df751 --- /dev/null +++ b/feature/permissions/src/main/res/drawable/ic_location_on.xml @@ -0,0 +1,29 @@ + + + + + + diff --git a/feature/permissions/src/main/res/values/strings.xml b/feature/permissions/src/main/res/values/strings.xml index add0a42636..fb72937bcc 100644 --- a/feature/permissions/src/main/res/values/strings.xml +++ b/feature/permissions/src/main/res/values/strings.xml @@ -31,5 +31,8 @@ Please provide permission to write to external storage. You will be unable to save captured mages or videos if this permission is denied. An icon representing a file folder + Enable Location + Please provide permission to access location. This allows Jetpack Camera to tag photos and videos with where they were captured. + An icon representing a location pin \ No newline at end of file diff --git a/feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsScreenTest.kt b/feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsScreenTest.kt new file mode 100644 index 0000000000..40aceba690 --- /dev/null +++ b/feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsScreenTest.kt @@ -0,0 +1,224 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.jetpackcamera.permissions + +import android.Manifest +import androidx.compose.runtime.getValue +import androidx.compose.runtime.key +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.ui.test.junit4.v2.createComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.performClick +import com.google.accompanist.permissions.ExperimentalPermissionsApi +import com.google.accompanist.permissions.PermissionStatus +import com.google.common.truth.Truth.assertThat +import com.google.jetpackcamera.permissions.ui.PermissionTemplate +import com.google.jetpackcamera.permissions.ui.REQUEST_PERMISSION_BUTTON +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@OptIn(ExperimentalPermissionsApi::class) +@RunWith(RobolectricTestRunner::class) +// The permission layout needs a phone-sized screen for the request button to be on screen. +@Config(qualifiers = "w411dp-h891dp") +class PermissionsScreenTest { + + @get:Rule + val composeTestRule = createComposeRule() + + @Test + fun whenLocationFirstClick_launchesPermissionRequest() { + var launchCount = 0 + + val fakeFine = FakePermissionState( + permission = Manifest.permission.ACCESS_FINE_LOCATION, + status = PermissionStatus.Denied(shouldShowRationale = false) + ) + val fakeCoarse = FakePermissionState( + permission = Manifest.permission.ACCESS_COARSE_LOCATION, + status = PermissionStatus.Denied(shouldShowRationale = false) + ) + val fakeState = FakeMultiplePermissionsState( + permissions = listOf(fakeFine, fakeCoarse), + onLaunch = { launchCount++ } + ) + + composeTestRule.setContent { + PermissionTemplate( + permissionEnum = PermissionEnum.LOCATION, + permissionStates = fakeState, + onDismissPermission = {}, + onOpenAppSettings = {} + ) + } + + // First click: attempts system permission request + composeTestRule.onNodeWithTag(REQUEST_PERMISSION_BUTTON).performClick() + assertThat(launchCount).isEqualTo(1) + } + + @Test + fun whenLocationPermanentlyDenied_clickingAgain_invokesDismissPermission() { + var dismissCalled = false + var launchCount = 0 + + val fakeFine = FakePermissionState( + permission = Manifest.permission.ACCESS_FINE_LOCATION, + status = PermissionStatus.Denied(shouldShowRationale = false) + ) + val fakeCoarse = FakePermissionState( + permission = Manifest.permission.ACCESS_COARSE_LOCATION, + status = PermissionStatus.Denied(shouldShowRationale = false) + ) + val fakeState = FakeMultiplePermissionsState( + permissions = listOf(fakeFine, fakeCoarse), + onLaunch = { launchCount++ } + ) + + composeTestRule.setContent { + PermissionTemplate( + permissionEnum = PermissionEnum.LOCATION, + permissionStates = fakeState, + onDismissPermission = { dismissCalled = true }, + onOpenAppSettings = {} + ) + } + + // First click: attempts system permission request + composeTestRule.onNodeWithTag(REQUEST_PERMISSION_BUTTON).performClick() + assertThat(launchCount).isEqualTo(1) + assertThat(dismissCalled).isFalse() + + // Second click when permanently silenced: skips/dismisses optional permission cleanly + composeTestRule.onNodeWithTag(REQUEST_PERMISSION_BUTTON).performClick() + assertThat(dismissCalled).isTrue() + } + + @Test + fun whenOptionalPermissionDeniedOnce_launchedEffect_invokesDismissPermission() { + var dismissCalled = false + + val fakeFine = FakePermissionState( + permission = Manifest.permission.ACCESS_FINE_LOCATION, + status = PermissionStatus.Denied(shouldShowRationale = true) + ) + val fakeState = FakeMultiplePermissionsState( + permissions = listOf(fakeFine), + shouldShowRationale = true + ) + + composeTestRule.setContent { + PermissionTemplate( + permissionEnum = PermissionEnum.LOCATION, + permissionStates = fakeState, + onDismissPermission = { dismissCalled = true }, + onOpenAppSettings = {} + ) + } + + composeTestRule.waitForIdle() + assertThat(dismissCalled).isTrue() + } + + @Test + fun whenAudioRequestedThenLocationShown_clickingAllow_launchesLocationRequest() { + var audioLaunchCount = 0 + var locationLaunchCount = 0 + var currentEnum by mutableStateOf(PermissionEnum.RECORD_AUDIO) + + val audioState = FakeMultiplePermissionsState( + permissions = listOf( + FakePermissionState( + permission = Manifest.permission.RECORD_AUDIO, + status = PermissionStatus.Denied(shouldShowRationale = false) + ) + ), + onLaunch = { audioLaunchCount++ } + ) + val locationState = FakeMultiplePermissionsState( + permissions = listOf( + FakePermissionState( + permission = Manifest.permission.ACCESS_FINE_LOCATION, + status = PermissionStatus.Denied(shouldShowRationale = false) + ), + FakePermissionState( + permission = Manifest.permission.ACCESS_COARSE_LOCATION, + status = PermissionStatus.Denied(shouldShowRationale = false) + ) + ), + onLaunch = { locationLaunchCount++ } + ) + + composeTestRule.setContent { + key(currentEnum) { + PermissionTemplate( + permissionEnum = currentEnum, + permissionStates = if (currentEnum == PermissionEnum.RECORD_AUDIO) { + audioState + } else { + locationState + }, + onDismissPermission = {}, + onOpenAppSettings = {} + ) + } + } + + // First click on Audio: launches audio request + composeTestRule.onNodeWithTag(REQUEST_PERMISSION_BUTTON).performClick() + assertThat(audioLaunchCount).isEqualTo(1) + + // Switch to Location screen + currentEnum = PermissionEnum.LOCATION + composeTestRule.waitForIdle() + + // Click on Location: must launch location request and not be skipped + composeTestRule.onNodeWithTag(REQUEST_PERMISSION_BUTTON).performClick() + assertThat(locationLaunchCount).isEqualTo(1) + } + + @Test + fun permissionTemplate_statusMutatedInPlaceToGranted_dismissesPermission() { + var dismissed = false + val fineLocationState = FakePermissionState( + permission = Manifest.permission.ACCESS_FINE_LOCATION, + status = PermissionStatus.Denied(shouldShowRationale = false) + ) + val permissionStates = FakeMultiplePermissionsState( + permissions = listOf(fineLocationState) + ) + + composeTestRule.setContent { + PermissionTemplate( + permissionEnum = PermissionEnum.LOCATION, + permissionStates = permissionStates, + onDismissPermission = { dismissed = true }, + onOpenAppSettings = {} + ) + } + + composeTestRule.waitForIdle() + assertThat(dismissed).isFalse() + + fineLocationState.status = PermissionStatus.Granted + composeTestRule.waitForIdle() + assertThat(dismissed).isTrue() + } +} diff --git a/feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsTestFakes.kt b/feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsTestFakes.kt new file mode 100644 index 0000000000..390820aa4b --- /dev/null +++ b/feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsTestFakes.kt @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.jetpackcamera.permissions + +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import com.google.accompanist.permissions.ExperimentalPermissionsApi +import com.google.accompanist.permissions.MultiplePermissionsState +import com.google.accompanist.permissions.PermissionState +import com.google.accompanist.permissions.PermissionStatus +import com.google.jetpackcamera.permissions.data.PermissionsRepository +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.update + +@OptIn(ExperimentalPermissionsApi::class) +class FakeMultiplePermissionsState( + override val permissions: List = emptyList(), + override val allPermissionsGranted: Boolean = false, + override val revokedPermissions: List = emptyList(), + override val shouldShowRationale: Boolean = false, + private val onLaunch: () -> Unit = {} +) : MultiplePermissionsState { + override fun launchMultiplePermissionRequest() { + onLaunch() + } +} + +@OptIn(ExperimentalPermissionsApi::class) +class FakePermissionState( + override val permission: String, + status: PermissionStatus +) : PermissionState { + override var status: PermissionStatus by mutableStateOf(status) + override fun launchPermissionRequest() {} +} + +class FakePermissionsRepository : PermissionsRepository { + override val requestedPermissions = MutableStateFlow>(emptySet()) + + override suspend fun markPermissionRequested(permissionName: String) { + requestedPermissions.update { it + permissionName } + } +} diff --git a/feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsViewModelTest.kt b/feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsViewModelTest.kt new file mode 100644 index 0000000000..9b2369795b --- /dev/null +++ b/feature/permissions/src/test/java/com/google/jetpackcamera/permissions/PermissionsViewModelTest.kt @@ -0,0 +1,253 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.jetpackcamera.permissions + +import androidx.lifecycle.SavedStateHandle +import com.google.accompanist.permissions.ExperimentalPermissionsApi +import com.google.accompanist.permissions.PermissionStatus +import com.google.common.truth.Truth.assertThat +import com.google.jetpackcamera.settings.testing.FakeSettingsRepository +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(JUnit4::class) +@OptIn(ExperimentalPermissionsApi::class, ExperimentalCoroutinesApi::class) +class PermissionsViewModelTest { + + private lateinit var viewModel: PermissionsViewModel + private lateinit var settingsRepository: FakeSettingsRepository + private lateinit var permissionsRepository: FakePermissionsRepository + private val testDispatcher = StandardTestDispatcher() + + @Before + fun setup() { + Dispatchers.setMain(testDispatcher) + settingsRepository = FakeSettingsRepository() + permissionsRepository = FakePermissionsRepository() + val savedStateHandle = SavedStateHandle() + viewModel = PermissionsViewModel( + savedStateHandle = savedStateHandle, + settingsRepository = settingsRepository, + permissionsRepository = permissionsRepository + ) + } + + @After + fun tearDown() { + Dispatchers.resetMain() + } + + @Test + fun updatePermissionStates_locationTransitionedToGranted_updatesSettingsRepository() = runTest { + // ensure initial is false + settingsRepository.updateLocationEnabled(false) + + // Initial state: location not granted + val initialState = FakeMultiplePermissionsState( + permissions = listOf( + FakePermissionState( + android.Manifest.permission.ACCESS_COARSE_LOCATION, + PermissionStatus.Denied(false) + ) + ) + ) + viewModel.updatePermissionStates(initialState) + advanceUntilIdle() + + var settings = settingsRepository.getCurrentDefaultCameraAppSettings() + assertThat(settings.locationEnabled).isFalse() + + // State 2: location granted + val grantedState = FakeMultiplePermissionsState( + permissions = listOf( + FakePermissionState( + android.Manifest.permission.ACCESS_COARSE_LOCATION, + PermissionStatus.Granted + ) + ) + ) + viewModel.updatePermissionStates(grantedState) + advanceUntilIdle() + + settings = settingsRepository.getCurrentDefaultCameraAppSettings() + assertThat(settings.locationEnabled).isTrue() + } + + @Test + fun updatePermissionStates_initialLocationGranted_doesNotUpdateSettingsRepository() = runTest { + // Initial state: user has location disabled manually via settings (perhaps) + settingsRepository.updateLocationEnabled(false) + + val initialState = FakeMultiplePermissionsState( + permissions = listOf( + FakePermissionState( + android.Manifest.permission.ACCESS_COARSE_LOCATION, + PermissionStatus.Granted + ) + ) + ) + + viewModel.updatePermissionStates(initialState) + advanceUntilIdle() + + val settings = settingsRepository.getCurrentDefaultCameraAppSettings() + assertThat(settings.locationEnabled).isFalse() + } + + @Test + fun updatePermissionStates_optionalPermissionInRequestedPermissions_isSkipped() = runTest { + backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { + viewModel.permissionsUiState.collect() + } + permissionsRepository.requestedPermissions.value = setOf(PermissionEnum.LOCATION.name) + + val state = FakeMultiplePermissionsState( + permissions = listOf( + FakePermissionState( + android.Manifest.permission.CAMERA, + PermissionStatus.Denied(false) + ), + FakePermissionState( + android.Manifest.permission.ACCESS_FINE_LOCATION, + PermissionStatus.Denied(false) + ) + ) + ) + + viewModel.updatePermissionStates(state) + advanceUntilIdle() + + val uiState = viewModel.permissionsUiState.value + assertThat(uiState).isInstanceOf(PermissionsUiState.PermissionsNeeded::class.java) + // Camera is still needed, but Location is excluded because it was already requested + assertThat((uiState as PermissionsUiState.PermissionsNeeded).currentPermission) + .isEqualTo(PermissionEnum.CAMERA) + } + + @Test + fun updatePermissionStates_whenCameraGrantedOnFirstRun_doesNotSkipOptionalPermissions() = + runTest { + backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { + viewModel.permissionsUiState.collect() + } + val state = FakeMultiplePermissionsState( + permissions = listOf( + FakePermissionState( + android.Manifest.permission.CAMERA, + PermissionStatus.Granted + ), + FakePermissionState( + android.Manifest.permission.RECORD_AUDIO, + PermissionStatus.Denied(false) + ), + FakePermissionState( + android.Manifest.permission.ACCESS_FINE_LOCATION, + PermissionStatus.Denied(false) + ) + ) + ) + + viewModel.updatePermissionStates(state) + advanceUntilIdle() + + // When camera is granted on first run, optional permissions (Audio, Location) + // are not yet requested and must NOT be skipped. + val uiState = viewModel.permissionsUiState.value + assertThat(uiState).isInstanceOf(PermissionsUiState.PermissionsNeeded::class.java) + assertThat((uiState as PermissionsUiState.PermissionsNeeded).currentPermission) + .isEqualTo(PermissionEnum.RECORD_AUDIO) + } + + @Test + fun updatePermissionStates_whenCameraGrantedAndOptionalRequested_allGranted() = runTest { + backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { + viewModel.permissionsUiState.collect() + } + permissionsRepository.requestedPermissions.value = setOf( + PermissionEnum.RECORD_AUDIO.name, + PermissionEnum.LOCATION.name + ) + val state = FakeMultiplePermissionsState( + permissions = listOf( + FakePermissionState( + android.Manifest.permission.CAMERA, + PermissionStatus.Granted + ), + FakePermissionState( + android.Manifest.permission.ACCESS_FINE_LOCATION, + PermissionStatus.Denied(false) + ), + FakePermissionState( + android.Manifest.permission.RECORD_AUDIO, + PermissionStatus.Denied(false) + ) + ) + ) + + viewModel.updatePermissionStates(state) + advanceUntilIdle() + + // When optional permissions were already requested, they are skipped + // and AllPermissionsGranted is reached. + val uiState = viewModel.permissionsUiState.value + assertThat(uiState).isEqualTo(PermissionsUiState.AllPermissionsGranted) + } + + @Test + fun getRequestablePermissions_rationaleOnAnyLocationPermission_skipsLocation() { + val state = FakeMultiplePermissionsState( + permissions = listOf( + FakePermissionState( + android.Manifest.permission.CAMERA, + PermissionStatus.Granted + ), + FakePermissionState( + android.Manifest.permission.ACCESS_FINE_LOCATION, + PermissionStatus.Denied(shouldShowRationale = true) + ), + FakePermissionState( + android.Manifest.permission.ACCESS_COARSE_LOCATION, + PermissionStatus.Denied(shouldShowRationale = false) + ) + ) + ) + + assertThat(getRequestablePermissions(state)).isEmpty() + } + + @Test + fun dismissPermission_marksPermissionRequestedInRepository() = runTest { + viewModel.dismissPermission(PermissionEnum.LOCATION) + advanceUntilIdle() + + val requested = permissionsRepository.requestedPermissions.first() + assertThat(requested).contains(PermissionEnum.LOCATION.name) + } +} diff --git a/feature/settings/build.gradle.kts b/feature/settings/build.gradle.kts index 2df58cf970..514296431f 100644 --- a/feature/settings/build.gradle.kts +++ b/feature/settings/build.gradle.kts @@ -110,6 +110,7 @@ dependencies { testImplementation(project(":core:settings:datastore-prefs:testing")) testImplementation(project(":data:settings:testing")) testImplementation(libs.androidx.datastore.preferences) + testImplementation(project(":core:location:testing")) androidTestImplementation(libs.androidx.junit) androidTestImplementation(libs.androidx.espresso.core) androidTestImplementation(libs.truth) @@ -134,6 +135,8 @@ dependencies { implementation(project(":core:model")) implementation(project(":core:camera")) implementation(project(":core:camera:effects:single-stream")) + implementation(project(":core:location")) + implementation(project(":core:location:location-di")) } // Allow references to generated code diff --git a/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsScreen.kt b/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsScreen.kt index 99b3c6656a..176625e1b9 100644 --- a/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsScreen.kt +++ b/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsScreen.kt @@ -16,6 +16,9 @@ package com.google.jetpackcamera.settings import android.Manifest +import android.content.Intent +import android.net.Uri +import android.provider.Settings import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -24,24 +27,35 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.AlertDialog import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBarDefaults import androidx.compose.material3.rememberTopAppBarState import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.input.nestedscroll.nestedScroll +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import com.google.accompanist.permissions.ExperimentalPermissionsApi +import com.google.accompanist.permissions.MultiplePermissionsState +import com.google.accompanist.permissions.isGranted import com.google.accompanist.permissions.rememberMultiplePermissionsState +import com.google.accompanist.permissions.shouldShowRationale import com.google.jetpackcamera.model.AspectRatio import com.google.jetpackcamera.model.ConcurrentCameraMode import com.google.jetpackcamera.model.DarkMode @@ -55,6 +69,10 @@ import com.google.jetpackcamera.settings.ui.ConcurrentCameraSetting import com.google.jetpackcamera.settings.ui.DarkModeSetting import com.google.jetpackcamera.settings.ui.DefaultCameraFacing import com.google.jetpackcamera.settings.ui.FlashModeSetting +import com.google.jetpackcamera.settings.ui.LOCATION_PERMISSION_DIALOG_CANCEL_BTN_TAG +import com.google.jetpackcamera.settings.ui.LOCATION_PERMISSION_DIALOG_CONFIRM_BTN_TAG +import com.google.jetpackcamera.settings.ui.LOCATION_PERMISSION_RATIONALE_DIALOG_TAG +import com.google.jetpackcamera.settings.ui.LocationSetting import com.google.jetpackcamera.settings.ui.LowLightBoostPrioritySetting import com.google.jetpackcamera.settings.ui.MaxVideoDurationSetting import com.google.jetpackcamera.settings.ui.RecordingAudioSetting @@ -74,6 +92,7 @@ private val LOADING_INDICATOR_SIZE = 50.dp * @param versionInfo Holder for application version and build type information. * @param onNavigateBack Callback when the user navigates back from settings. * @param viewModel The [SettingsViewModel] providing the settings state. + * @param onOpenAppSettings Optional callback when user chooses to open system app settings. * @param cameraSettingsSlot Slot for the camera settings section. * @param recordingSettingsSlot Slot for the recording settings section. * @param appSettingsSlot Slot for the general application settings section. @@ -84,12 +103,17 @@ fun SettingsScreen( versionInfo: VersionInfoHolder, onNavigateBack: () -> Unit, viewModel: SettingsViewModel = hiltViewModel(), + onOpenAppSettings: (() -> Unit)? = null, cameraSettingsSlot: @Composable () -> Unit = { DefaultCameraSettings(viewModel = viewModel) }, recordingSettingsSlot: @Composable () -> Unit = { DefaultRecordingSettings(viewModel = viewModel) }, appSettingsSlot: @Composable () -> Unit = { - DefaultAppSettings(versionInfo = versionInfo, viewModel = viewModel) + DefaultAppSettings( + versionInfo = versionInfo, + viewModel = viewModel, + onOpenAppSettings = onOpenAppSettings + ) } ) { val permissionStates = rememberMultiplePermissionsState( @@ -97,7 +121,9 @@ fun SettingsScreen( listOf( Manifest.permission.CAMERA, Manifest.permission.RECORD_AUDIO, - Manifest.permission.READ_EXTERNAL_STORAGE + Manifest.permission.READ_EXTERNAL_STORAGE, + Manifest.permission.ACCESS_FINE_LOCATION, + Manifest.permission.ACCESS_COARSE_LOCATION ) ) @@ -293,20 +319,87 @@ fun DefaultRecordingSettings( * * @param versionInfo The [VersionInfoHolder] containing app version information. * @param viewModel The [SettingsViewModel] providing the settings state. + * @param locationPermissionStates The [MultiplePermissionsState] for location permissions. + * @param onOpenAppSettings Optional callback when user chooses to open system app settings. */ +@OptIn(ExperimentalPermissionsApi::class) @Composable fun DefaultAppSettings( versionInfo: VersionInfoHolder, - viewModel: SettingsViewModel = hiltViewModel() + viewModel: SettingsViewModel = hiltViewModel(), + locationPermissionStates: MultiplePermissionsState = rememberMultiplePermissionsState( + permissions = listOf( + Manifest.permission.ACCESS_FINE_LOCATION, + Manifest.permission.ACCESS_COARSE_LOCATION + ) + ), + onOpenAppSettings: (() -> Unit)? = null ) { val uiState by viewModel.settingsUiState.collectAsState() val enabledState = uiState as? SettingsUiState.Enabled ?: return + val context = LocalContext.current + val openSettingsHandler = onOpenAppSettings ?: { + Intent( + Settings.ACTION_APPLICATION_DETAILS_SETTINGS, + Uri.fromParts("package", context.packageName, null) + ).also(context::startActivity) + } + + var showLocationRationaleDialog by rememberSaveable { mutableStateOf(false) } + var hasAttemptedLocationRequest by rememberSaveable { mutableStateOf(false) } + var pendingLocationEnable by rememberSaveable { mutableStateOf(false) } + + val hasLocationPermission = locationPermissionStates.permissions.any { it.status.isGranted } + + LaunchedEffect(hasLocationPermission) { + if (hasLocationPermission && pendingLocationEnable) { + viewModel.setLocationEnabled(true) + pendingLocationEnable = false + } + } + DefaultAppSettings( versionInfo = versionInfo, enabledState = enabledState, - setDarkMode = viewModel::setDarkMode + setDarkMode = viewModel::setDarkMode, + setLocationEnabled = { enabled -> + if (enabled) { + if (hasLocationPermission) { + viewModel.setLocationEnabled(true) + } else { + pendingLocationEnable = true + val canShowRationale = + locationPermissionStates.permissions.any { it.status.shouldShowRationale } + // After a request has been denied without a rationale, the system no longer + // shows the prompt, so direct the user to app settings instead. + if (canShowRationale || !hasAttemptedLocationRequest) { + hasAttemptedLocationRequest = true + locationPermissionStates.launchMultiplePermissionRequest() + } else { + showLocationRationaleDialog = true + } + } + } else { + pendingLocationEnable = false + viewModel.setLocationEnabled(false) + } + } ) + + if (showLocationRationaleDialog) { + LocationPermissionRationaleDialog( + onConfirm = { + showLocationRationaleDialog = false + pendingLocationEnable = true + openSettingsHandler() + }, + onDismiss = { + showLocationRationaleDialog = false + pendingLocationEnable = false + } + ) + } } /** @@ -315,15 +408,24 @@ fun DefaultAppSettings( * @param versionInfo The [VersionInfoHolder] containing app version information. * @param enabledState The current [SettingsUiState.Enabled] state. * @param setDarkMode Callback to set the dark mode. + * @param setLocationEnabled Callback to set whether location is enabled. */ @Composable fun DefaultAppSettings( versionInfo: VersionInfoHolder, enabledState: SettingsUiState.Enabled, - setDarkMode: (DarkMode) -> Unit + setDarkMode: (DarkMode) -> Unit, + setLocationEnabled: (Boolean) -> Unit = {} ) { SectionHeader(title = stringResource(id = R.string.section_title_app_settings)) + if (enabledState.locationUiState !is LocationUiState.Hidden) { + LocationSetting( + locationUiState = enabledState.locationUiState, + onLocationToggled = setLocationEnabled + ) + } + DarkModeSetting( darkModeUiState = enabledState.darkModeUiState, setDarkMode = setDarkMode @@ -337,4 +439,46 @@ fun DefaultAppSettings( ) } +/** + * Dialog informing the user that location permissions are required for geotagging and + * providing an action to navigate to system app settings. + * + * @param onConfirm Callback when the user confirms navigating to app settings. + * @param onDismiss Callback when the dialog is dismissed or cancelled. + * @param modifier Modifier for the dialog layout. + */ +@Composable +fun LocationPermissionRationaleDialog( + onConfirm: () -> Unit, + onDismiss: () -> Unit, + modifier: Modifier = Modifier +) { + AlertDialog( + modifier = modifier.testTag(LOCATION_PERMISSION_RATIONALE_DIALOG_TAG), + onDismissRequest = onDismiss, + title = { + Text(text = stringResource(R.string.location_permission_dialog_title)) + }, + text = { + Text(text = stringResource(R.string.location_permission_dialog_message)) + }, + confirmButton = { + TextButton( + modifier = Modifier.testTag(LOCATION_PERMISSION_DIALOG_CONFIRM_BTN_TAG), + onClick = onConfirm + ) { + Text(text = stringResource(R.string.location_permission_dialog_open_settings)) + } + }, + dismissButton = { + TextButton( + modifier = Modifier.testTag(LOCATION_PERMISSION_DIALOG_CANCEL_BTN_TAG), + onClick = onDismiss + ) { + Text(text = stringResource(R.string.location_permission_dialog_cancel)) + } + } + ) +} + data class VersionInfoHolder(val versionName: String, val buildType: String) diff --git a/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsUiState.kt b/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsUiState.kt index 2f7bfdca2e..3daa9f3c8b 100644 --- a/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsUiState.kt +++ b/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsUiState.kt @@ -32,6 +32,7 @@ import com.google.jetpackcamera.settings.ui.CONCURRENT_CAMERA_ENABLED_TAG import com.google.jetpackcamera.settings.ui.DEVICE_UNSUPPORTED_TAG import com.google.jetpackcamera.settings.ui.FPS_UNSUPPORTED_TAG import com.google.jetpackcamera.settings.ui.LENS_UNSUPPORTED_TAG +import com.google.jetpackcamera.settings.ui.PERMISSION_LOCATION_NOT_GRANTED_TAG import com.google.jetpackcamera.settings.ui.PERMISSION_RECORD_AUDIO_NOT_GRANTED_TAG import com.google.jetpackcamera.settings.ui.STABILIZATION_UNSUPPORTED_TAG import com.google.jetpackcamera.settings.ui.ULTRA_HDR_ENABLED_TAG @@ -57,6 +58,7 @@ sealed interface SettingsUiState { val maxVideoDurationUiState: MaxVideoDurationUiState.Enabled, val videoQualityUiState: VideoQualityUiState, val audioUiState: AudioUiState, + val locationUiState: LocationUiState, val lowLightBoostPriorityUiState: LowLightBoostPriorityUiState, val concurrentCameraUiState: ConcurrentCameraUiState ) : SettingsUiState @@ -82,6 +84,17 @@ sealed interface DisabledRationale { override val testTag = PERMISSION_RECORD_AUDIO_NOT_GRANTED_TAG } + /** + * Rationale indicating that location geotagging is unavailable because location + * permissions have not been granted. + */ + data class PermissionLocationNotGrantedRationale( + override val affectedSettingNameResId: Int + ) : DisabledRationale { + override val reasonTextResId: Int = R.string.permission_location_unsupported + override val testTag = PERMISSION_LOCATION_NOT_GRANTED_TAG + } + /** * Text will be [affectedSettingNameResId] is [R.string.device_unsupported] */ @@ -219,6 +232,26 @@ sealed interface AudioUiState { data class Disabled(val disabledRationale: DisabledRationale) : AudioUiState } +/** + * Represents the UI state of the location geotagging setting. + */ +sealed interface LocationUiState { + /** The location setting is not displayed. */ + data object Hidden : LocationUiState + + /** The location setting is enabled and interactive. */ + sealed interface Enabled : LocationUiState { + /** Location geotagging is enabled. */ + data object On : Enabled + + /** Location geotagging is disabled. */ + data object Off : Enabled + } + + /** The location setting is disabled due to missing permissions. */ + data class Disabled(val disabledRationale: DisabledRationale) : LocationUiState +} + sealed interface FlashUiState { data class Enabled( val currentFlashMode: FlashMode, @@ -332,6 +365,11 @@ val TYPICAL_SETTINGS_UISTATE = SettingsUiState.Enabled( } else { AudioUiState.Enabled.Mute() }, + locationUiState = LocationUiState.Disabled( + DisabledRationale.PermissionLocationNotGrantedRationale( + R.string.save_location_rationale_prefix + ) + ), flashUiState = FlashUiState.Enabled( currentFlashMode = DEFAULT_CAMERA_APP_SETTINGS.flashMode, diff --git a/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsViewModel.kt b/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsViewModel.kt index 51c1d5ad46..1994345ff0 100644 --- a/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsViewModel.kt +++ b/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsViewModel.kt @@ -22,6 +22,7 @@ import androidx.lifecycle.viewModelScope import com.google.accompanist.permissions.ExperimentalPermissionsApi import com.google.accompanist.permissions.MultiplePermissionsState import com.google.accompanist.permissions.isGranted +import com.google.jetpackcamera.core.location.LocationProvider import com.google.jetpackcamera.model.AspectRatio import com.google.jetpackcamera.model.CameraEffectId import com.google.jetpackcamera.model.ConcurrentCameraMode @@ -51,6 +52,7 @@ import com.google.jetpackcamera.settings.ui.FLASH_LLB_ACTIVE_TAG import com.google.jetpackcamera.settings.ui.HDR_ACTIVE_TAG import com.google.jetpackcamera.settings.ui.STABILIZATION_ACTIVE_TAG import dagger.hilt.android.lifecycle.HiltViewModel +import java.util.Optional import javax.inject.Inject import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted @@ -70,7 +72,8 @@ private val fpsOptions = setOf(TARGET_FPS_15, TARGET_FPS_30, TARGET_FPS_60) @HiltViewModel class SettingsViewModel @Inject constructor( private val settingsRepository: SettingsRepository, - constraintsRepository: ConstraintsRepository + constraintsRepository: ConstraintsRepository, + private val locationProvider: Optional = Optional.empty() ) : ViewModel() { private var grantedPermissions = MutableStateFlow>(emptySet()) @@ -80,7 +83,7 @@ class SettingsViewModel @Inject constructor( constraintsRepository.systemConstraints.filterNotNull(), grantedPermissions ) { updatedSettings, constraints, grantedPerms -> - updatedSettings.videoQuality + val unused = updatedSettings.videoQuality SettingsUiState.Enabled( aspectRatioUiState = AspectRatioUiState.Enabled(updatedSettings.aspectRatio), cameraEffectUiState = getCameraEffectUiState(updatedSettings, constraints), @@ -93,6 +96,15 @@ class SettingsViewModel @Inject constructor( updatedSettings.audioEnabled, grantedPerms.contains(Manifest.permission.RECORD_AUDIO) ), + locationUiState = if (locationProvider.isPresent) { + getLocationUiState( + updatedSettings.locationEnabled, + grantedPerms.contains(Manifest.permission.ACCESS_FINE_LOCATION) || + grantedPerms.contains(Manifest.permission.ACCESS_COARSE_LOCATION) + ) + } else { + LocationUiState.Hidden + }, fpsUiState = getFpsUiState(constraints, updatedSettings), lensFlipUiState = getLensFlipUiState(constraints, updatedSettings), stabilizationUiState = getStabilizationUiState(constraints, updatedSettings), @@ -260,6 +272,24 @@ class SettingsViewModel @Inject constructor( ) } + private fun getLocationUiState( + isLocationEnabled: Boolean, + permissionGranted: Boolean + ): LocationUiState = if (permissionGranted) { + if (isLocationEnabled) { + LocationUiState.Enabled.On + } else { + LocationUiState.Enabled.Off + } + } else { + LocationUiState.Disabled( + DisabledRationale + .PermissionLocationNotGrantedRationale( + R.string.save_location_rationale_prefix + ) + ) + } + @OptIn(ExperimentalPermissionsApi::class) fun setGrantedPermissions(multiplePermissionsState: MultiplePermissionsState) { val permissions = mutableSetOf() @@ -750,6 +780,18 @@ class SettingsViewModel @Inject constructor( } } + /** + * Updates the location setting in the repository. + * + * @param enabled Whether location tagging should be enabled for captured media. + */ + internal fun setLocationEnabled(enabled: Boolean) { + viewModelScope.launch { + settingsRepository.updateLocationEnabled(enabled) + Log.d(TAG, "set save location enabled: $enabled") + } + } + /** * Updates the concurrent camera mode setting in the repository. * diff --git a/feature/settings/src/main/java/com/google/jetpackcamera/settings/ui/SettingsComponents.kt b/feature/settings/src/main/java/com/google/jetpackcamera/settings/ui/SettingsComponents.kt index 08f32b5504..c2b5814647 100644 --- a/feature/settings/src/main/java/com/google/jetpackcamera/settings/ui/SettingsComponents.kt +++ b/feature/settings/src/main/java/com/google/jetpackcamera/settings/ui/SettingsComponents.kt @@ -81,6 +81,7 @@ import com.google.jetpackcamera.settings.FIVE_SECONDS_DURATION import com.google.jetpackcamera.settings.FlashUiState import com.google.jetpackcamera.settings.FlipLensUiState import com.google.jetpackcamera.settings.FpsUiState +import com.google.jetpackcamera.settings.LocationUiState import com.google.jetpackcamera.settings.LowLightBoostPriorityUiState import com.google.jetpackcamera.settings.MaxVideoDurationUiState import com.google.jetpackcamera.settings.R @@ -819,6 +820,51 @@ fun RecordingAudioSetting( ) } +/** + * A settings item that renders a toggle switch for enabling or disabling location geotagging. + * + * @param modifier Modifier to be applied to the setting layout. + * @param locationUiState The current [LocationUiState] controlling title, rationale, and state. + * @param onLocationToggled Callback invoked when the user toggles the switch. + */ +@Composable +fun LocationSetting( + modifier: Modifier = Modifier, + locationUiState: LocationUiState, + onLocationToggled: (Boolean) -> Unit +) { + if (locationUiState is LocationUiState.Hidden) return + + SwitchSettingUI( + modifier = modifier.testTag(BTN_SWITCH_SETTING_LOCATION_TAG), + title = stringResource(id = R.string.location_setting_title), + description = when (locationUiState) { + is LocationUiState.Enabled.On -> { + stringResource(R.string.location_setting_description_on) + } + + is LocationUiState.Enabled.Off -> { + stringResource(R.string.location_setting_description_off) + } + + is LocationUiState.Disabled -> { + disabledRationaleString(disabledRationale = locationUiState.disabledRationale) + } + + LocationUiState.Hidden -> "" + }, + leadingIcon = null, + onSwitchChanged = onLocationToggled, + settingValue = when (locationUiState) { + is LocationUiState.Enabled.On -> true + is LocationUiState.Disabled, + is LocationUiState.Enabled.Off, + LocationUiState.Hidden -> false + }, + enabled = true // always clickable to allow on-demand permission prompts + ) +} + /** * A setting component that allows the user to enable or disable concurrent camera mode. * @@ -1099,6 +1145,11 @@ fun disabledRationaleString(disabledRationale: DisabledRationale): String = stringResource(disabledRationale.affectedSettingNameResId) ) + is DisabledRationale.PermissionLocationNotGrantedRationale -> stringResource( + disabledRationale.reasonTextResId, + stringResource(disabledRationale.affectedSettingNameResId) + ) + is DisabledRationale.ConcurrentCameraDisabledRationale -> stringResource( disabledRationale.reasonTextResId, stringResource(disabledRationale.affectedSettingNameResId) diff --git a/feature/settings/src/main/java/com/google/jetpackcamera/settings/ui/TestTags.kt b/feature/settings/src/main/java/com/google/jetpackcamera/settings/ui/TestTags.kt index 08a677df9b..8f561cbd54 100644 --- a/feature/settings/src/main/java/com/google/jetpackcamera/settings/ui/TestTags.kt +++ b/feature/settings/src/main/java/com/google/jetpackcamera/settings/ui/TestTags.kt @@ -37,6 +37,7 @@ const val LENS_UNSUPPORTED_TAG = "LensUnsupportedTag" const val FPS_UNSUPPORTED_TAG = "FpsUnsupportedTag" const val VIDEO_QUALITY_UNSUPPORTED_TAG = "VideoQualityUnsupportedTag" const val PERMISSION_RECORD_AUDIO_NOT_GRANTED_TAG = "PermissionRecordAudioNotGrantedTag" +const val PERMISSION_LOCATION_NOT_GRANTED_TAG = "PermissionLocationNotGrantedTag" const val CAPTURE_MODE_UNSUPPORTED_TAG = "CaptureModeUnsupportedTag" const val STREAM_CONFIG_UNSUPPORTED_TAG = "StreamConfigUnsupportedTag" const val FLASH_LLB_UNSUPPORTED_TAG = "FlashLlbUnsupportedTag" @@ -52,6 +53,10 @@ const val ULTRA_HDR_ENABLED_TAG = "UltraHdrEnabledTag" // Settings w/ no dialog const val BTN_SWITCH_SETTING_LENS_FACING_TAG = "btn_switch_setting_lens_facing_tag" const val BTN_SWITCH_SETTING_ENABLE_AUDIO_TAG = "btn_switch_setting_enable_audio_tag" +const val BTN_SWITCH_SETTING_LOCATION_TAG = "btn_switch_setting_location_tag" +const val LOCATION_PERMISSION_RATIONALE_DIALOG_TAG = "dialog_location_permission_rationale_tag" +const val LOCATION_PERMISSION_DIALOG_CONFIRM_BTN_TAG = "btn_location_permission_dialog_confirm_tag" +const val LOCATION_PERMISSION_DIALOG_CANCEL_BTN_TAG = "btn_location_permission_dialog_cancel_tag" const val BTN_SWITCH_SETTING_CONCURRENT_CAMERA_TAG = "btn_switch_setting_concurrent_camera_tag" const val TEXT_SETTING_APP_VERSION_TAG = "text_setting_app_version_tag" diff --git a/feature/settings/src/main/res/values/strings.xml b/feature/settings/src/main/res/values/strings.xml index 1e63dd0b02..6206e4f373 100644 --- a/feature/settings/src/main/res/values/strings.xml +++ b/feature/settings/src/main/res/values/strings.xml @@ -64,6 +64,15 @@ Recordings will start with audio enabled Recordings will start muted + + Save Location + Photos and videos will be saved with location coordinates + Photos and videos will be saved without location + Location permission required + To save location with your photos and videos, grant location permissions for Jetpack Camera in system settings. + Open Settings + Cancel + Enable Concurrent Camera Record from multiple cameras concurrently @@ -163,6 +172,7 @@ %1$s is unsupported by the front lens %1$s is unsupported by the current video quality %1$s requires permission to record audio + %1$s requires location permission %1$s is unsupported when Concurrent Camera is active %1$s is unsupported when Ultra HDR is enabled @@ -186,6 +196,7 @@ Fixed frame rate Mute + Save Location Concurrent camera diff --git a/feature/settings/src/test/java/com/google/jetpackcamera/settings/CameraAppSettingsViewModelTest.kt b/feature/settings/src/test/java/com/google/jetpackcamera/settings/CameraAppSettingsViewModelTest.kt index d7f64593d2..dc17ffd45f 100644 --- a/feature/settings/src/test/java/com/google/jetpackcamera/settings/CameraAppSettingsViewModelTest.kt +++ b/feature/settings/src/test/java/com/google/jetpackcamera/settings/CameraAppSettingsViewModelTest.kt @@ -26,6 +26,7 @@ import androidx.datastore.preferences.core.intPreferencesKey import androidx.datastore.preferences.core.stringPreferencesKey import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.common.truth.Truth.assertThat +import com.google.jetpackcamera.core.location.testing.FakeLocationProvider import com.google.jetpackcamera.core.settings.datastoreprefs.PrefsDataStoreSettingsDataSource import com.google.jetpackcamera.core.settings.datastoreprefs.testing.FakeDataStoreModule import com.google.jetpackcamera.model.CaptureMode @@ -41,6 +42,7 @@ import com.google.jetpackcamera.settings.testing.FakeConstraintsRepository import com.google.jetpackcamera.settings.testing.FakeSettingsRepository import com.google.jetpackcamera.settings.ui.BTN_OPEN_DIALOG_SETTING_FLASH_TAG import java.io.File +import java.util.Optional import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -106,6 +108,8 @@ internal class CameraAppSettingsViewModelTest { private lateinit var testDataStore: DataStore private lateinit var datastoreScope: CoroutineScope private lateinit var settingsViewModel: SettingsViewModel + private lateinit var settingsRepository: SettingsRepository + private lateinit var constraintsRepository: FakeConstraintsRepository @Before fun setup() = runTest(StandardTestDispatcher()) { @@ -122,13 +126,14 @@ internal class CameraAppSettingsViewModelTest { dataStore = testDataStore, defaultCaptureModeOverride = CaptureMode.STANDARD ) - val settingsRepository = LocalSettingsRepository( + settingsRepository = LocalSettingsRepository( settingsDataSource = settingsDataSource ) - val constraintsRepository = FakeConstraintsRepository(TYPICAL_SYSTEM_CONSTRAINTS) + constraintsRepository = FakeConstraintsRepository(TYPICAL_SYSTEM_CONSTRAINTS) settingsViewModel = SettingsViewModel( settingsRepository, - constraintsRepository + constraintsRepository, + Optional.of(FakeLocationProvider()) ) advanceUntilIdle() } @@ -138,6 +143,18 @@ internal class CameraAppSettingsViewModelTest { datastoreScope.cancel() } + @Test + fun locationSetting_whenLocationProviderNotPresent_isHidden() = + runTest(StandardTestDispatcher()) { + val vm = SettingsViewModel( + settingsRepository, + constraintsRepository, + Optional.empty() + ) + val uiState = vm.settingsUiState.first { it is SettingsUiState.Enabled } + assertThat(assertIsEnabled(uiState).locationUiState).isEqualTo(LocationUiState.Hidden) + } + @Test fun getSettingsUiState() = runTest(StandardTestDispatcher()) { settingsViewModel.setGrantedPermissions( @@ -218,6 +235,101 @@ internal class CameraAppSettingsViewModelTest { } } + @Test + fun setLocation_permission_granted() = runTest(StandardTestDispatcher()) { + // Wait for first Enabled state + settingsViewModel.setGrantedPermissions( + mutableSetOf(Manifest.permission.ACCESS_FINE_LOCATION) + ) + val initialState = settingsViewModel.settingsUiState.first { + it is SettingsUiState.Enabled + } + + val initialLocationState = assertIsEnabled(initialState).locationUiState + // assert that locationUiState is Enabled.Off + assertThat(initialLocationState).isInstanceOf(LocationUiState.Enabled.Off::class.java) + + val nextLocationUiState = LocationUiState.Enabled.On + settingsViewModel.setLocationEnabled(true) + + advanceUntilIdle() + + assertIsEnabled(settingsViewModel.settingsUiState.value).also { + assertThat(it.locationUiState).isEqualTo(nextLocationUiState) + } + } + + @Test + fun setLocation_permission_not_granted() = runTest(StandardTestDispatcher()) { + // Wait for first Enabled state + val initialState = settingsViewModel.settingsUiState.first { + it is SettingsUiState.Enabled + } + + val initialLocationState = assertIsEnabled(initialState).locationUiState + // assert that locationUiState is disabled + assertThat(initialLocationState).isNotInstanceOf(LocationUiState.Enabled::class.java) + + settingsViewModel.setLocationEnabled(true) + + advanceUntilIdle() + + // ensure still disabled + assertIsEnabled(settingsViewModel.settingsUiState.value).also { + assertThat(it.locationUiState).isNotInstanceOf(LocationUiState.Enabled::class.java) + } + } + + @Test + fun setLocation_coarsePermissionOnly_granted() = runTest(StandardTestDispatcher()) { + settingsViewModel.setGrantedPermissions( + mutableSetOf(Manifest.permission.ACCESS_COARSE_LOCATION) + ) + advanceUntilIdle() + + val initialState = settingsViewModel.settingsUiState.first { + it is SettingsUiState.Enabled + } + + val initialLocationState = assertIsEnabled(initialState).locationUiState + assertThat(initialLocationState).isInstanceOf(LocationUiState.Enabled.Off::class.java) + + settingsViewModel.setLocationEnabled(true) + advanceUntilIdle() + + assertIsEnabled(settingsViewModel.settingsUiState.value).also { + assertThat(it.locationUiState).isEqualTo(LocationUiState.Enabled.On) + } + } + + @Test + fun locationUiState_permissionRevoked_disablesLocationUiState() = + runTest(StandardTestDispatcher()) { + // Start with permission granted and location enabled + settingsViewModel.setGrantedPermissions( + mutableSetOf(Manifest.permission.ACCESS_FINE_LOCATION) + ) + settingsViewModel.setLocationEnabled(true) + advanceUntilIdle() + + val stateWithPerm = assertIsEnabled( + settingsViewModel.settingsUiState.first { it is SettingsUiState.Enabled } + ) + assertThat(stateWithPerm.locationUiState).isEqualTo(LocationUiState.Enabled.On) + + // Revoke location permission + settingsViewModel.setGrantedPermissions(mutableSetOf()) + advanceUntilIdle() + + val stateWithoutPerm = assertIsEnabled(settingsViewModel.settingsUiState.value) + assertThat(stateWithoutPerm.locationUiState) + .isInstanceOf(LocationUiState.Disabled::class.java) + val disabledRationale = + (stateWithoutPerm.locationUiState as LocationUiState.Disabled).disabledRationale + assertThat(disabledRationale) + .isInstanceOf(DisabledRationale.PermissionLocationNotGrantedRationale::class.java) + } + @Test fun setDefaultToFrontCamera() = runTest(StandardTestDispatcher()) { // Wait for first Enabled state diff --git a/feature/settings/src/test/java/com/google/jetpackcamera/settings/SettingsScreenTest.kt b/feature/settings/src/test/java/com/google/jetpackcamera/settings/SettingsScreenTest.kt new file mode 100644 index 0000000000..aecbfe8f52 --- /dev/null +++ b/feature/settings/src/test/java/com/google/jetpackcamera/settings/SettingsScreenTest.kt @@ -0,0 +1,210 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.jetpackcamera.settings + +import android.Manifest +import androidx.compose.foundation.layout.Column +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.v2.createComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.performClick +import com.google.accompanist.permissions.ExperimentalPermissionsApi +import com.google.accompanist.permissions.MultiplePermissionsState +import com.google.accompanist.permissions.PermissionState +import com.google.accompanist.permissions.PermissionStatus +import com.google.common.truth.Truth.assertThat +import com.google.jetpackcamera.core.location.testing.FakeLocationProvider +import com.google.jetpackcamera.core.settings.datastoreprefs.PrefsDataStoreSettingsDataSource +import com.google.jetpackcamera.core.settings.datastoreprefs.testing.FakeDataStoreModule +import com.google.jetpackcamera.model.CaptureMode +import com.google.jetpackcamera.settings.model.TYPICAL_SYSTEM_CONSTRAINTS +import com.google.jetpackcamera.settings.testing.FakeConstraintsRepository +import com.google.jetpackcamera.settings.ui.BTN_SWITCH_SETTING_LOCATION_TAG +import com.google.jetpackcamera.settings.ui.LOCATION_PERMISSION_DIALOG_CANCEL_BTN_TAG +import com.google.jetpackcamera.settings.ui.LOCATION_PERMISSION_DIALOG_CONFIRM_BTN_TAG +import com.google.jetpackcamera.settings.ui.LOCATION_PERMISSION_RATIONALE_DIALOG_TAG +import java.util.Optional +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@OptIn(ExperimentalPermissionsApi::class) +@RunWith(RobolectricTestRunner::class) +class SettingsScreenTest { + + @get:Rule + val composeTestRule = createComposeRule() + + @get:Rule + val tempFolder = TemporaryFolder() + + private fun createSettingsViewModel(): SettingsViewModel { + val testDataStore = FakeDataStoreModule.providePreferenceDataStore( + scope = CoroutineScope(Dispatchers.Unconfined + SupervisorJob()), + file = tempFolder.newFile("test_settings.preferences_pb") + ) + val settingsRepository = LocalSettingsRepository( + settingsDataSource = PrefsDataStoreSettingsDataSource( + dataStore = testDataStore, + defaultCaptureModeOverride = CaptureMode.STANDARD + ) + ) + val constraintsRepository = FakeConstraintsRepository(TYPICAL_SYSTEM_CONSTRAINTS) + return SettingsViewModel( + settingsRepository, + constraintsRepository, + Optional.of(FakeLocationProvider()) + ) + } + + private fun locationPermissionStates( + status: PermissionStatus, + onLaunch: () -> Unit = {} + ): MultiplePermissionsState = FakeMultiplePermissionsState( + permissions = listOf( + FakePermissionState(Manifest.permission.ACCESS_FINE_LOCATION, status), + FakePermissionState(Manifest.permission.ACCESS_COARSE_LOCATION, status) + ), + onLaunch = onLaunch + ) + + private fun setAppSettingsContent( + locationPermissionStates: MultiplePermissionsState, + onOpenAppSettings: () -> Unit = {} + ) { + val settingsViewModel = createSettingsViewModel() + composeTestRule.setContent { + // DefaultAppSettings emits sibling rows, so it needs a layout parent. + Column { + DefaultAppSettings( + versionInfo = VersionInfoHolder("1.0", "debug"), + viewModel = settingsViewModel, + locationPermissionStates = locationPermissionStates, + onOpenAppSettings = onOpenAppSettings + ) + } + } + composeTestRule.waitUntil(5_000) { + settingsViewModel.settingsUiState.value is SettingsUiState.Enabled + } + } + + private fun clickLocationSwitch() { + composeTestRule.onNodeWithTag(BTN_SWITCH_SETTING_LOCATION_TAG).performClick() + } + + @Test + fun locationPermanentlyDenied_secondToggle_showsRationaleDialogThatOpensSettings() { + var launchCount = 0 + var openAppSettingsCalled = false + setAppSettingsContent( + locationPermissionStates = locationPermissionStates( + status = PermissionStatus.Denied(shouldShowRationale = false), + onLaunch = { launchCount++ } + ), + onOpenAppSettings = { openAppSettingsCalled = true } + ) + + // First toggle requests the permission. + clickLocationSwitch() + assertThat(launchCount).isEqualTo(1) + + // The system no longer prompts after a denial, so the second toggle shows the dialog. + clickLocationSwitch() + assertThat(launchCount).isEqualTo(1) + composeTestRule.onNodeWithTag(LOCATION_PERMISSION_RATIONALE_DIALOG_TAG).assertIsDisplayed() + + composeTestRule.onNodeWithTag(LOCATION_PERMISSION_DIALOG_CONFIRM_BTN_TAG).performClick() + assertThat(openAppSettingsCalled).isTrue() + } + + @Test + fun locationRationaleDialog_cancel_dismissesWithoutOpeningSettings() { + var openAppSettingsCalled = false + setAppSettingsContent( + locationPermissionStates = locationPermissionStates( + status = PermissionStatus.Denied(shouldShowRationale = false) + ), + onOpenAppSettings = { openAppSettingsCalled = true } + ) + + clickLocationSwitch() + clickLocationSwitch() + composeTestRule.onNodeWithTag(LOCATION_PERMISSION_DIALOG_CANCEL_BTN_TAG).performClick() + + composeTestRule.onNodeWithTag(LOCATION_PERMISSION_RATIONALE_DIALOG_TAG).assertDoesNotExist() + assertThat(openAppSettingsCalled).isFalse() + } + + @Test + fun locationRationaleAvailable_toggle_requestsPermissionAgain() { + var launchCount = 0 + setAppSettingsContent( + locationPermissionStates = locationPermissionStates( + status = PermissionStatus.Denied(shouldShowRationale = true), + onLaunch = { launchCount++ } + ) + ) + + clickLocationSwitch() + clickLocationSwitch() + + assertThat(launchCount).isEqualTo(2) + composeTestRule.onNodeWithTag(LOCATION_PERMISSION_RATIONALE_DIALOG_TAG).assertDoesNotExist() + } + + @Test + fun locationPermissionGranted_toggle_doesNotRequestOrShowDialog() { + var launchCount = 0 + setAppSettingsContent( + locationPermissionStates = locationPermissionStates( + status = PermissionStatus.Granted, + onLaunch = { launchCount++ } + ) + ) + + clickLocationSwitch() + + assertThat(launchCount).isEqualTo(0) + composeTestRule.onNodeWithTag(LOCATION_PERMISSION_RATIONALE_DIALOG_TAG).assertDoesNotExist() + } +} + +@OptIn(ExperimentalPermissionsApi::class) +private class FakeMultiplePermissionsState( + override val permissions: List, + override val allPermissionsGranted: Boolean = false, + override val revokedPermissions: List = emptyList(), + override val shouldShowRationale: Boolean = false, + private val onLaunch: () -> Unit = {} +) : MultiplePermissionsState { + override fun launchMultiplePermissionRequest() { + onLaunch() + } +} + +@OptIn(ExperimentalPermissionsApi::class) +private class FakePermissionState( + override val permission: String, + override val status: PermissionStatus +) : PermissionState { + override fun launchPermissionRequest() {} +} From ee53c4e7a5ddeb9fe057f31c9de102563ff56cf9 Mon Sep 17 00:00:00 2001 From: temcguir Date: Thu, 1 Oct 2026 00:50:30 +0000 Subject: [PATCH 2/4] Address review feedback on location permission UX - Document PermissionTemplate. - Evaluate each PermissionEnum once in getRequestablePermissions while preserving the requested order. --- .../permissions/PermissionsViewModel.kt | 24 +++++++++---------- .../ui/PermissionsScreenComponents.kt | 16 +++++++++++++ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsViewModel.kt b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsViewModel.kt index a12794e277..c8dd2f0904 100644 --- a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsViewModel.kt +++ b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/PermissionsViewModel.kt @@ -134,24 +134,22 @@ class PermissionsViewModel @Inject constructor( fun getRequestablePermissions( permissionStates: MultiplePermissionsState, requestedPermissions: Set = emptySet() -): List = buildSet { - for (permissionState in permissionStates.permissions) { - val permission = PermissionEnum.fromString(permissionState.permission) - val componentStates = permission.getPermissions().mapNotNull { permStr -> - permissionStates.permissions.firstOrNull { it.permission == permStr } +): List = permissionStates.permissions + .map { PermissionEnum.fromString(it.permission) } + .distinct() + .filter { permission -> + val componentStates = permissionStates.permissions.filter { + it.permission in permission.getPermissions() } val isAnyGranted = componentStates.any { it.status.isGranted } - if (!isAnyGranted) { - if (!permission.isOptional()) { - add(permission) - } else { + when { + isAnyGranted -> false + !permission.isOptional() -> true + else -> { val wasPreviouslyHandled = permission.name in requestedPermissions || componentStates.any { it.status.shouldShowRationale } - if (!wasPreviouslyHandled) { - add(permission) - } + !wasPreviouslyHandled } } } -}.toList() diff --git a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/PermissionsScreenComponents.kt b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/PermissionsScreenComponents.kt index 9e1f74bf73..591432a0ed 100644 --- a/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/PermissionsScreenComponents.kt +++ b/feature/permissions/src/main/java/com/google/jetpackcamera/permissions/ui/PermissionsScreenComponents.kt @@ -82,6 +82,22 @@ fun PermissionTemplate( } } +/** + * Displays the request screen for a single [PermissionEnum] and routes the request button based + * on the permission's current state. + * + * The screen is skipped automatically via [onDismissPermission] once any component permission is + * granted, or when an optional permission has already been declined. For mandatory permissions + * the user previously declined, the button opens the system app settings and the rationale text + * is shown. For optional permissions, a second press after a request that was denied permanently + * dismisses the screen instead of requesting again. + * + * @param permissionEnum The permission being requested. + * @param permissionStates The state of the system permissions that make up [permissionEnum]. + * @param onDismissPermission Called when the screen should advance past this permission. + * @param onOpenAppSettings Called to open the system app settings for a declined permission. + * @param modifier The [Modifier] to be applied to the layout. + */ @OptIn(ExperimentalPermissionsApi::class) @Composable internal fun PermissionTemplate( From e804bf67dfca93dbc86128b16ce5d15d30ce2f95 Mon Sep 17 00:00:00 2001 From: temcguir Date: Thu, 1 Oct 2026 17:39:49 +0000 Subject: [PATCH 3/4] Restore unchanged videoQuality line in SettingsViewModel Reverts an unintended edit to a line that is unrelated to this change, so the file matches main outside of the location additions. --- .../java/com/google/jetpackcamera/settings/SettingsViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsViewModel.kt b/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsViewModel.kt index 1994345ff0..f1988b13fc 100644 --- a/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsViewModel.kt +++ b/feature/settings/src/main/java/com/google/jetpackcamera/settings/SettingsViewModel.kt @@ -83,7 +83,7 @@ class SettingsViewModel @Inject constructor( constraintsRepository.systemConstraints.filterNotNull(), grantedPermissions ) { updatedSettings, constraints, grantedPerms -> - val unused = updatedSettings.videoQuality + updatedSettings.videoQuality SettingsUiState.Enabled( aspectRatioUiState = AspectRatioUiState.Enabled(updatedSettings.aspectRatio), cameraEffectUiState = getCameraEffectUiState(updatedSettings, constraints), From 039ca9a701053087610557aec04804f995eab9b2 Mon Sep 17 00:00:00 2001 From: temcguir Date: Thu, 1 Oct 2026 18:40:01 +0000 Subject: [PATCH 4/4] Defer adding location to the startup permission list The location permission request is added to the app's startup list in the follow-up change that also binds a LocationProvider and declares the location permissions in the manifest. Until then, the onboarding flow does not show the location page. --- app/src/main/java/com/google/jetpackcamera/ui/JcaApp.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/src/main/java/com/google/jetpackcamera/ui/JcaApp.kt b/app/src/main/java/com/google/jetpackcamera/ui/JcaApp.kt index 44d4218a02..5862ddb032 100644 --- a/app/src/main/java/com/google/jetpackcamera/ui/JcaApp.kt +++ b/app/src/main/java/com/google/jetpackcamera/ui/JcaApp.kt @@ -102,8 +102,6 @@ private fun JetpackCameraNavHost( add(android.Manifest.permission.CAMERA) if (externalCaptureMode == ExternalCaptureMode.Standard) { add(android.Manifest.permission.RECORD_AUDIO) - add(android.Manifest.permission.ACCESS_FINE_LOCATION) - add(android.Manifest.permission.ACCESS_COARSE_LOCATION) if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { add(android.Manifest.permission.WRITE_EXTERNAL_STORAGE) }