From 3d422fafd4576475a5a86666e18bc1ceec16262e Mon Sep 17 00:00:00 2001 From: Percy Wegmann Date: Fri, 5 Apr 2024 11:06:27 -0500 Subject: [PATCH] android/ui: stop treating settings as data Updates tailscale/corp#18983 Signed-off-by: Percy Wegmann --- .../tailscale/ipn/ui/view/DNSSettingsView.kt | 2 +- .../tailscale/ipn/ui/view/ExitNodePicker.kt | 2 +- .../com/tailscale/ipn/ui/view/SettingsView.kt | 96 ++++++----------- .../tailscale/ipn/ui/view/UserSwitcherView.kt | 23 +++- .../ipn/ui/viewModel/SettingsViewModel.kt | 101 +----------------- .../ipn/ui/viewModel/UserSwitcherViewModel.kt | 29 ----- 6 files changed, 60 insertions(+), 193 deletions(-) diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/DNSSettingsView.kt b/android/src/main/java/com/tailscale/ipn/ui/view/DNSSettingsView.kt index 7333f0c..1276517 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/view/DNSSettingsView.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/view/DNSSettingsView.kt @@ -65,7 +65,7 @@ fun DNSSettingsView( supportingContent = { Text(stringResource(state.caption)) }) Lists.ItemDivider() - SettingsRow.Switch( + Setting.Switch( R.string.use_ts_dns, isOn = useCorpDNS, onToggle = { diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/ExitNodePicker.kt b/android/src/main/java/com/tailscale/ipn/ui/view/ExitNodePicker.kt index 27bc156..8217dd4 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/view/ExitNodePicker.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/view/ExitNodePicker.kt @@ -74,7 +74,7 @@ fun ExitNodePicker( item(key = "allowLANAccess") { Lists.SectionDivider() - SettingsRow.Switch(R.string.allow_lan_access, isOn = allowLANAccess) { + Setting.Switch(R.string.allow_lan_access, isOn = allowLANAccess) { LoadingIndicator.start() model.toggleAllowLANAccess { LoadingIndicator.stop() } } diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/SettingsView.kt b/android/src/main/java/com/tailscale/ipn/ui/view/SettingsView.kt index 0c69b0b..7e569d4 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/view/SettingsView.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/view/SettingsView.kt @@ -4,7 +4,6 @@ package com.tailscale.ipn.ui.view import androidx.compose.foundation.clickable -import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.padding import androidx.compose.foundation.text.ClickableText @@ -29,21 +28,15 @@ import com.tailscale.ipn.ui.Links import com.tailscale.ipn.ui.theme.link import com.tailscale.ipn.ui.theme.listItem import com.tailscale.ipn.ui.util.Lists -import com.tailscale.ipn.ui.viewModel.Setting -import com.tailscale.ipn.ui.viewModel.SettingType import com.tailscale.ipn.ui.viewModel.SettingsNav import com.tailscale.ipn.ui.viewModel.SettingsViewModel -import com.tailscale.ipn.ui.viewModel.SettingsViewModelFactory @Composable -fun SettingsView( - settingsNav: SettingsNav, - viewModel: SettingsViewModel = viewModel(factory = SettingsViewModelFactory(settingsNav)) -) { +fun SettingsView(settingsNav: SettingsNav, viewModel: SettingsViewModel = viewModel()) { val handler = LocalUriHandler.current val user = viewModel.loggedInUser.collectAsState().value val isAdmin = viewModel.isAdmin.collectAsState().value - val managedBy = viewModel.managedBy.collectAsState().value + val managedByOrganization = viewModel.managedByOrganization.collectAsState().value Scaffold( topBar = { @@ -53,73 +46,68 @@ fun SettingsView( UserView( profile = user, actionState = UserActionState.NAV, - onClick = viewModel.navigation.onNavigateToUserSwitcher) + onClick = settingsNav.onNavigateToUserSwitcher) if (isAdmin) { AdminTextView { handler.openUri(Links.ADMIN_URL) } } Lists.SectionDivider() - SettingRow(viewModel.dns) + Setting.Text(R.string.dns_settings, onClick = settingsNav.onNavigateToDNSSettings) Lists.ItemDivider() - SettingRow(viewModel.tailnetLock) + Setting.Text(R.string.tailnet_lock, onClick = settingsNav.onNavigateToTailnetLock) Lists.ItemDivider() - SettingRow(viewModel.permissions) + Setting.Text(R.string.permissions, onClick = settingsNav.onNavigateToPermissions) - managedBy?.let { + managedByOrganization?.let { Lists.ItemDivider() - SettingRow(it) + Setting.Text( + title = stringResource(R.string.managed_by_orgName, it), + onClick = settingsNav.onNavigateToManagedBy) } Lists.SectionDivider() - SettingRow(viewModel.bugReport) + Setting.Text(R.string.bug_report, onClick = settingsNav.onNavigateToBugReport) Lists.ItemDivider() - SettingRow(viewModel.about) + Setting.Text(R.string.about_tailscale, onClick = settingsNav.onNavigateToAbout) // TODO: put a heading for the debug section if (BuildConfig.DEBUG) { Lists.SectionDivider() - SettingRow(viewModel.mdmDebug) + Setting.Text(R.string.mdm_settings, onClick = settingsNav.onNavigateToMDMSettings) } } } } -@Composable -fun SettingRow(setting: Setting) { - Box { - when (setting.type) { - SettingType.TEXT -> TextRow(setting) - SettingType.NAV -> { - NavRow(setting) - } +object Setting { + @Composable + fun Text( + titleRes: Int = 0, + title: String? = null, + destructive: Boolean = false, + enabled: Boolean = true, + onClick: (() -> Unit)? = null + ) { + var modifier: Modifier = Modifier + if (enabled) { + onClick?.let { modifier = modifier.clickable(onClick = it) } } + ListItem( + modifier = modifier, + colors = MaterialTheme.colorScheme.listItem, + headlineContent = { + Text( + title ?: stringResource(titleRes), + style = MaterialTheme.typography.bodyMedium, + color = if (destructive) MaterialTheme.colorScheme.error else Color.Unspecified) + }, + ) } -} - -@Composable -private fun TextRow(setting: Setting) { - val enabled = setting.enabled.collectAsState().value - var modifier: Modifier = Modifier - if (enabled) { - setting.onClick?.let { modifier = modifier.clickable(onClick = it) } - } - ListItem( - modifier = modifier, - colors = MaterialTheme.colorScheme.listItem, - headlineContent = { - Text( - setting.title ?: stringResource(setting.titleRes), - style = MaterialTheme.typography.bodyMedium, - color = if (setting.destructive) MaterialTheme.colorScheme.error else Color.Unspecified) - }, - ) -} -object SettingsRow { @Composable fun Switch( titleRes: Int = 0, @@ -142,20 +130,6 @@ object SettingsRow { } } -@Composable -private fun NavRow(setting: Setting) { - var modifier: Modifier = Modifier - setting.onClick?.let { modifier = modifier.clickable(onClick = it) } - ListItem( - modifier = modifier, - colors = MaterialTheme.colorScheme.listItem, - headlineContent = { - Text( - setting.title ?: stringResource(setting.titleRes), - style = MaterialTheme.typography.bodyMedium) - }) -} - @Composable fun AdminTextView(onNavigateToAdminConsole: () -> Unit) { val adminStr = buildAnnotatedString { diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/UserSwitcherView.kt b/android/src/main/java/com/tailscale/ipn/ui/view/UserSwitcherView.kt index 95877b3..02b91cc 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/view/UserSwitcherView.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/view/UserSwitcherView.kt @@ -111,12 +111,29 @@ fun UserSwitcherView( item { Lists.SectionDivider() - SettingRow(viewModel.addProfileSetting) + Setting.Text(R.string.add_account) { + viewModel.addProfile { + if (it.isFailure) { + viewModel.errorDialog.set(ErrorDialogType.ADD_PROFILE_FAILED) + } + } + } + Lists.ItemDivider() - SettingRow(viewModel.loginSetting) + Setting.Text(R.string.reauthenticate) { viewModel.login {} } + if (currentUser != null) { Lists.ItemDivider() - SettingRow(viewModel.logoutSetting) + Setting.Text( + R.string.log_out, + destructive = true, + onClick = { + viewModel.logout { + if (it.isFailure) { + viewModel.errorDialog.set(ErrorDialogType.LOGOUT_FAILED) + } + } + }) } } } diff --git a/android/src/main/java/com/tailscale/ipn/ui/viewModel/SettingsViewModel.kt b/android/src/main/java/com/tailscale/ipn/ui/viewModel/SettingsViewModel.kt index 6e69bec..afc59ed 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/viewModel/SettingsViewModel.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/viewModel/SettingsViewModel.kt @@ -3,10 +3,7 @@ package com.tailscale.ipn.ui.viewModel -import androidx.lifecycle.ViewModel -import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewModelScope -import com.tailscale.ipn.R import com.tailscale.ipn.mdm.MDMSettings import com.tailscale.ipn.ui.notifier.Notifier import com.tailscale.ipn.ui.util.set @@ -14,35 +11,6 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.launch -enum class SettingType { - NAV, - TEXT -} - -// Represents a UI setting. -// title: The title of the setting -// type: The type of setting -// enabled: Whether the setting is enabled -// value: The value of the setting for textual settings -// isOn: The value of the setting for switch settings -// onClick: The action to take when the setting is clicked (typically for navigation) -// onToggle: The action to take when the setting is toggled (typically for switches) -// icon: An optional Composable that draws a trailing icon to display with nav settings -// -// Behavior is undefined if you mix the types here. Switch settings should supply an -// isOn and onToggle, while navigation settings should supply an onClick and an optional -// value -data class Setting( - val titleRes: Int = 0, - val title: String? = null, - val type: SettingType, - val destructive: Boolean = false, - val enabled: StateFlow = MutableStateFlow(true), - val isOn: StateFlow? = null, - val onClick: (() -> Unit)? = null, - val onToggle: (Boolean) -> Unit = {} -) - data class SettingsNav( val onNavigateToBugReport: () -> Unit, val onNavigateToAbout: () -> Unit, @@ -55,75 +23,12 @@ data class SettingsNav( val onBackPressed: () -> Unit, ) -class SettingsViewModelFactory(private val navigation: SettingsNav) : ViewModelProvider.Factory { - override fun create(modelClass: Class): T { - return SettingsViewModel(navigation) as T - } -} - -class SettingsViewModel(val navigation: SettingsNav) : IpnViewModel() { +class SettingsViewModel() : IpnViewModel() { // Display name for the logged in user - var isAdmin: StateFlow = MutableStateFlow(false) - - val dns = - Setting( - titleRes = R.string.dns_settings, - type = SettingType.NAV, - onClick = { navigation.onNavigateToDNSSettings() }, - enabled = MutableStateFlow(true)) - - val tailnetLock = - Setting( - titleRes = R.string.tailnet_lock, - type = SettingType.NAV, - onClick = { navigation.onNavigateToTailnetLock() }, - enabled = MutableStateFlow(true)) - - val permissions = - Setting( - titleRes = R.string.permissions, - type = SettingType.NAV, - onClick = { navigation.onNavigateToPermissions() }, - enabled = MutableStateFlow(true)) - - val about = - Setting( - titleRes = R.string.about_tailscale, - type = SettingType.NAV, - onClick = { navigation.onNavigateToAbout() }, - enabled = MutableStateFlow(true)) - - val bugReport = - Setting( - titleRes = R.string.bug_report, - type = SettingType.NAV, - onClick = { navigation.onNavigateToBugReport() }, - enabled = MutableStateFlow(true)) - - val managedBy: StateFlow = MutableStateFlow(null) - - val mdmDebug = - Setting( - titleRes = R.string.mdm_settings, - type = SettingType.NAV, - onClick = { navigation.onNavigateToMDMSettings() }, - enabled = MutableStateFlow(true)) + val isAdmin: StateFlow = MutableStateFlow(false) + val managedByOrganization = MDMSettings.managedByOrganizationName.flow init { - viewModelScope.launch { - MDMSettings.managedByOrganizationName.flow.collect { managedByOrganization -> - managedBy.set( - managedByOrganization?.let { - Setting( - R.string.managed_by_orgName, - it, - SettingType.NAV, - onClick = { navigation.onNavigateToManagedBy() }, - enabled = MutableStateFlow(true)) - }) - } - } - viewModelScope.launch { Notifier.netmap.collect { netmap -> isAdmin.set(netmap?.SelfNode?.isAdmin ?: false) } } diff --git a/android/src/main/java/com/tailscale/ipn/ui/viewModel/UserSwitcherViewModel.kt b/android/src/main/java/com/tailscale/ipn/ui/viewModel/UserSwitcherViewModel.kt index bb2ac9c..d468fe3 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/viewModel/UserSwitcherViewModel.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/viewModel/UserSwitcherViewModel.kt @@ -4,7 +4,6 @@ package com.tailscale.ipn.ui.viewModel import androidx.lifecycle.viewModelScope -import com.tailscale.ipn.R import com.tailscale.ipn.ui.localapi.Client import com.tailscale.ipn.ui.model.Ipn import com.tailscale.ipn.ui.notifier.Notifier @@ -21,34 +20,6 @@ class UserSwitcherViewModel : IpnViewModel() { // True if we should render the kebab menu val showHeaderMenu: StateFlow = MutableStateFlow(false) - val loginSetting = - Setting(titleRes = R.string.reauthenticate, type = SettingType.NAV, onClick = { login {} }) - - val logoutSetting = - Setting( - titleRes = R.string.log_out, - destructive = true, - type = SettingType.TEXT, - onClick = { - logout { - if (it.isFailure) { - errorDialog.set(ErrorDialogType.LOGOUT_FAILED) - } - } - }) - - val addProfileSetting = - Setting( - titleRes = R.string.add_account, - type = SettingType.NAV, - onClick = { - addProfile { - if (it.isFailure) { - errorDialog.set(ErrorDialogType.ADD_PROFILE_FAILED) - } - } - }) - // Sets the custom control URL and immediatly invokes the login flow fun setControlURL(urlStr: String) { // Some basic checks that the entered URL is "reasonable". The underlying