android/ui: stop treating settings as data

Updates tailscale/corp#18983

Signed-off-by: Percy Wegmann <percy@tailscale.com>

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 = {

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() }
           }

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(
+fun SettingsView(settingsNav: SettingsNav, viewModel: SettingsViewModel = viewModel()) {
-    settingsNav: SettingsNav,
-    viewModel: SettingsViewModel = viewModel(factory = SettingsViewModelFactory(settingsNav))
-) {
   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
+object Setting {
-fun SettingRow(setting: Setting) {
+  @Composable
-  Box {
+  fun Text(
-    when (setting.type) {
+      titleRes: Int = 0,
-      SettingType.TEXT -> TextRow(setting)
+      title: String? = null,
-      SettingType.NAV -> {
+      destructive: Boolean = false,
-        NavRow(setting)
+      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 {

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)
+                            }
+                          }
+                        })
                   }
                 }
               }

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<Boolean> = MutableStateFlow(true),
-    val isOn: StateFlow<Boolean?>? = 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 {
+class SettingsViewModel() : IpnViewModel() {
-  override fun <T : ViewModel> create(modelClass: Class<T>): T {
-    return SettingsViewModel(navigation) as T
-  }
-}
-
-class SettingsViewModel(val navigation: SettingsNav) : IpnViewModel() {
   // Display name for the logged in user
-  var isAdmin: StateFlow<Boolean> = MutableStateFlow(false)
+  val isAdmin: StateFlow<Boolean> = MutableStateFlow(false)
-
+  val managedByOrganization = MDMSettings.managedByOrganizationName.flow
-  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<Setting?> = MutableStateFlow(null)
-
-  val mdmDebug =
-      Setting(
-          titleRes = R.string.mdm_settings,
-          type = SettingType.NAV,
-          onClick = { navigation.onNavigateToMDMSettings() },
-          enabled = MutableStateFlow(true))
 
   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) }
     }

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<Boolean> = 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