Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the “Settings” list item selection in tablet/two-pane mode so it remains selected when navigating to nested Settings sub-screens.
Changes:
- Update selection logic in
MyAccountScreenWrapperto use a predicate-based match for the selected navigation item. - Extend
MyAccountSettingAction.Navigationwith anisSelectedpredicate and adjust theSettingsentry to match nested settings destinations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/MyAccountScreenWrapper.kt | Uses the new isSelected(...) predicate to resolve the currently selected settings navigation entry. |
| app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/MyAccountScreen.kt | Adds an isSelected predicate to navigation actions and updates Settings to be selected across nested settings destinations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val destination: SettingsOptionScreens, | ||
| val isSelected: (SettingsOptionScreens?) -> Boolean, | ||
| ) : MyAccountSettingAction(null) { | ||
| data object Settings : Navigation(SettingsOptionScreens.SETTINGS, { it is SettingsOptionScreens }) |
There was a problem hiding this comment.
Navigation.Settings uses isSelected = { it is SettingsOptionScreens }, but the predicate parameter is already typed as SettingsOptionScreens?, so this check is effectively just it != null. Using an explicit non-null check (or a clearly named helper like isInSettingsSubtree) would make the selection rule easier to understand and maintain.
| data object Settings : Navigation(SettingsOptionScreens.SETTINGS, { it is SettingsOptionScreens }) | |
| data object Settings : Navigation(SettingsOptionScreens.SETTINGS, { it != null }) |
|
The tablet selection did not recognize nested navigations as being part of the "Settings" item