Repository navigation
Conversation
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.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces location geotagging support, including a new optional location permission onboarding flow and a "Save Location" toggle in the settings screen. The implementation uses a DataStore-backed repository to persist permission request states and includes comprehensive unit and UI tests. The feedback focuses on adhering to the repository's style guide regarding test tag naming conventions, adding KDoc to internal composables, removing a debug log, and optimizing the permission filtering logic to better handle composite permissions.
- Document PermissionTemplate. - Evaluate each PermissionEnum once in getRequestablePermissions while preserving the requested order.
Reverts an unintended edit to a line that is unrelated to this change, so the file matches main outside of the location additions.
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.
…missions-and-settings-ui
Rename the constants so they follow the element_purpose naming used by the other test tags (BTN_..., DIALOG_...). Tag string values are unchanged.
davidjiagoogle
requested changes
Oct 5, 2026
…missions-and-settings-ui
- Only optional permissions are recorded as dismissed. A mandatory permission that is not granted is requested again. - The permission screen records an optional permission as dismissed when its request ends without a grant, instead of relying on the rationale state changing. Clicking the request button again relaunches the request. - The location section of app settings reports its own grants to the view model through a merge update, so the toggle does not depend on the screen-level permission state refreshing. - If a location request from settings ends without a grant and no rationale was available before or after it, the rationale dialog with the link to app settings is shown on the first toggle.
…r idle before asserting the merged permission state
… for permissions and location settings
davidjiagoogle
approved these changes
Oct 6, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Adds an optional location permission request step to the onboarding flow and introduces a "Save Location" toggle switch in the in-app Settings screen.
This builds on the location settings persistence and decoupled
LocationProvidercontract introduced in #575.Key Changes
:feature:permissions:PermissionEnum.LOCATIONas an optional permission card.PermissionsRepositoryto track dismissed and requested permissions across sessions.PermissionsScreenComponentsfor location permission rationale alert dialog and app-settings deep-linking.SettingsRepository.updateLocationEnabled(true)when granted during onboarding.:feature:settings:LocationSettingComponent("Save Location" toggle) to the Settings screen.LocationUiState(hidden whenLocationProvideris absent,Enabled.On/Offwhen present).SettingsViewModelthroughupdateGrantedPermissions, which updates only the given permissions.:app:DataStorePermissionsRepositoryviaPermissionsModule.onOpenAppSettingsfromMainActivity/JcaApptoSettingsScreen.LocationProvider, so the onboarding location page and the Settings row stay hidden in the app until then.PermissionsScreenTest,PermissionsViewModelTest,SettingsScreenTest,CameraAppSettingsViewModelTest).