Overhaul SettingsItem

Close #5021

app/src/main/java/com/topjohnwu/magisk/ui/settings/BaseSettingsItem.kt

@@ -3,7 +3,6 @@ package com.topjohnwu.magisk.ui.settings
 import android.content.Context
 import android.content.res.Resources
 import android.view.View
-import androidx.annotation.CallSuper
 import androidx.databinding.Bindable
 import com.topjohnwu.magisk.BR
 import com.topjohnwu.magisk.R
@@ -20,76 +19,33 @@ sealed class BaseSettingsItem : ObservableRvItem() {
     open val title: TextHolder get() = TextHolder.EMPTY
     @get:Bindable
     open val description: TextHolder get() = TextHolder.EMPTY
-
-    // ---
-
     open val showSwitch get() = false
-
     @get:Bindable
     open val isChecked get() = false
-
-    open fun onToggle(view: View, callback: Callback, checked: Boolean) {}
-
-    // ---
-
     @get:Bindable
     var isEnabled = true
         set(value) = set(value, field, { field = it }, BR.enabled, BR.description)
 
-    open fun onPressed(view: View, callback: Callback) {
-        callback.onItemPressed(view, this)
+    open fun onToggle(view: View, handler: Handler, checked: Boolean) {}
+    open fun onPressed(view: View, handler: Handler) {
+        handler.onItemPressed(view, this)
     }
-
     open fun refresh() {}
 
-    // ---
-
-    interface Callback {
-        fun onItemPressed(view: View, item: BaseSettingsItem, callback: () -> Unit = {})
-        fun onItemChanged(view: View, item: BaseSettingsItem)
+    interface Handler {
+        fun onItemPressed(view: View, item: BaseSettingsItem, andThen: () -> Unit = {})
+        fun onItemAction(view: View, item: BaseSettingsItem)
     }
 
-    // ---
-
     abstract class Value<T> : BaseSettingsItem() {
 
         /**
          * Represents last agreed-upon value by the validation process and the user for current
          * child. Be very aware that this shouldn't be **set** unless both sides agreed that _that_
          * is the new value.
-         *
-         * Annotating [value] as [Bindable] property should raise red flags immediately. If you
-         * need a [Bindable] property create another one. Seriously.
          * */
         abstract var value: T
-            /**
-             * We don't want this to be accessible to be set from outside the instances. It will
-             * introduce unwanted bugs!
-             * */
             protected set
-
-        protected var callbackVars: Pair<View, Callback>? = null
-
-        @CallSuper
-        override fun onPressed(view: View, callback: Callback) {
-            callbackVars = view to callback
-            callback.onItemPressed(view, this) {
-                onPressed(view)
-            }
-        }
-
-        abstract fun onPressed(view: View)
-
-        protected inline fun <reified T> setV(
-            new: T, old: T, setter: (T) -> Unit, afterChanged: (T) -> Unit = {}) {
-            set(new, old, setter, BR.description, BR.checked) {
-                afterChanged(it)
-                callbackVars?.let { (view, callback) ->
-                    callbackVars = null
-                    callback.onItemChanged(view, this)
-                }
-            }
-        }
     }
 
     abstract class Toggle : Value<Boolean>() {
@@ -97,38 +53,45 @@ sealed class BaseSettingsItem : ObservableRvItem() {
         override val showSwitch get() = true
         override val isChecked get() = value
 
-        override fun onToggle(view: View, callback: Callback, checked: Boolean) =
-            set(checked, value, { onPressed(view, callback) }, BR.checked)
+        override fun onToggle(view: View, handler: Handler, checked: Boolean) =
+            set(checked, value, { onPressed(view, handler) })
 
-        override fun onPressed(view: View) {
-            value = !value
+        override fun onPressed(view: View, handler: Handler) {
+            handler.onItemPressed(view, this) {
+                value = !value
+                notifyPropertyChanged(BR.checked)
+                handler.onItemAction(view, this)
+            }
         }
     }
 
     abstract class Input : Value<String>() {
 
-        protected abstract val inputResult: String?
+        @get:Bindable
+        abstract val inputResult: String?
 
-        override fun onPressed(view: View) {
-            MagiskDialog(view.context).apply {
-                setTitle(title.getText(view.resources))
-                setView(getView(view.context))
-                setButton(MagiskDialog.ButtonType.POSITIVE) {
-                    text = android.R.string.ok
-                    onClick {
-                        inputResult?.let { result ->
-                            doNotDismiss = false
-                            value = result
-                            it.dismiss()
-                            return@onClick
+        override fun onPressed(view: View, handler: Handler) {
+            handler.onItemPressed(view, this) {
+                MagiskDialog(view.context).apply {
+                    setTitle(title.getText(view.resources))
+                    setView(getView(view.context))
+                    setButton(MagiskDialog.ButtonType.POSITIVE) {
+                        text = android.R.string.ok
+                        onClick {
+                            inputResult?.let { result ->
+                                doNotDismiss = false
+                                value = result
+                                handler.onItemAction(view, this@Input)
+                                return@onClick
+                            }
+                            doNotDismiss = true
                         }
-                        doNotDismiss = true
                     }
-                }
-                setButton(MagiskDialog.ButtonType.NEGATIVE) {
-                    text = android.R.string.cancel
-                }
-            }.show()
+                    setButton(MagiskDialog.ButtonType.NEGATIVE) {
+                        text = android.R.string.cancel
+                    }
+                }.show()
+            }
         }
 
         abstract fun getView(context: Context): View
@@ -150,18 +113,23 @@ sealed class BaseSettingsItem : ObservableRvItem() {
         private fun Resources.getArrayOrEmpty(id: Int): Array<String> =
             runCatching { getStringArray(id) }.getOrDefault(emptyArray())
 
-        override fun onPressed(view: View) {
-            MagiskDialog(view.context).apply {
-                setTitle(title.getText(view.resources))
-                setButton(MagiskDialog.ButtonType.NEGATIVE) {
-                    text = android.R.string.cancel
-                }
-                setListItems(entries(view.resources)) {
-                    value = it
-                }
-            }.show()
+        override fun onPressed(view: View, handler: Handler) {
+            handler.onItemPressed(view, this) {
+                MagiskDialog(view.context).apply {
+                    setTitle(title.getText(view.resources))
+                    setButton(MagiskDialog.ButtonType.NEGATIVE) {
+                        text = android.R.string.cancel
+                    }
+                    setListItems(entries(view.resources)) {
+                        if (value != it) {
+                            value = it
+                            notifyPropertyChanged(BR.description)
+                            handler.onItemAction(view, this@Selector)
+                        }
+                    }
+                }.show()
+            }
         }
-
     }
 
     abstract class Blank : BaseSettingsItem()

app/src/main/java/com/topjohnwu/magisk/ui/settings/SettingsItems.kt

@@ -37,8 +37,9 @@ object Customization : BaseSettingsItem.Section() {
 
 object Language : BaseSettingsItem.Selector() {
     override var value = -1
-        set(value) = setV(value, field, { field = it }) {
-            Config.locale = entryValues[it]
+        set(value) {
+            field = value
+            Config.locale = entryValues[value]
         }
 
     override val title = R.string.language.asText()
@@ -49,9 +50,9 @@ object Language : BaseSettingsItem.Selector() {
     override fun entries(res: Resources) = entries
     override fun descriptions(res: Resources) = entries
 
-    override fun onPressed(view: View, callback: Callback) {
-        if (entries.isEmpty()) return
-        super.onPressed(view, callback)
+    override fun onPressed(view: View, handler: Handler) {
+        if (entries.isNotEmpty())
+            super.onPressed(view, handler)
     }
 
     suspend fun loadLanguages(scope: CoroutineScope) {
@@ -80,9 +81,7 @@ object AppSettings : BaseSettingsItem.Section() {
 object Hide : BaseSettingsItem.Input() {
     override val title = R.string.settings_hide_app_title.asText()
     override val description = R.string.settings_hide_app_summary.asText()
-
     override var value = ""
-        set(value) = setV(value, field, { field = it })
 
     override val inputResult
         get() = if (isError) null else result
@@ -113,30 +112,31 @@ object AddShortcut : BaseSettingsItem.Blank() {
 }
 
 object DownloadPath : BaseSettingsItem.Input() {
-    override var value = Config.downloadDir
-        set(value) = setV(value, field, { field = it }) { Config.downloadDir = it }
+    override var value
+        get() = Config.downloadDir
+        set(value) {
+            Config.downloadDir = value
+            notifyPropertyChanged(BR.description)
+        }
 
     override val title = R.string.settings_download_path_title.asText()
-    override val description get() = path.asText()
+    override val description get() = MediaStoreUtils.fullPath(value).asText()
 
-    override val inputResult: String get() = result
+    override var inputResult: String = value
+        set(value) = set(value, field, { field = it }, BR.inputResult, BR.path)
 
     @get:Bindable
-    var result = value
-        set(value) = set(value, field, { field = it }, BR.result, BR.path)
-
-    @get:Bindable
-    val path
-        get() = MediaStoreUtils.fullPath(result)
+    val path get() = MediaStoreUtils.fullPath(inputResult)
 
     override fun getView(context: Context) = DialogSettingsDownloadPathBinding
         .inflate(LayoutInflater.from(context)).also { it.data = this }.root
 }
 
 object UpdateChannel : BaseSettingsItem.Selector() {
-    override var value = Config.updateChannel.let { if (it < 0) 0 else it }
-        set(value) = setV(value, field, { field = it }) {
-            Config.updateChannel = it
+    override var value
+        get() = Config.updateChannel
+        set(value) {
+            Config.updateChannel = value
             Info.remote = Info.EMPTY_REMOTE
         }
 
@@ -154,18 +154,17 @@ object UpdateChannel : BaseSettingsItem.Selector() {
 
 object UpdateChannelUrl : BaseSettingsItem.Input() {
     override val title = R.string.settings_update_custom.asText()
-    override var value = Config.customChannelUrl
-        set(value) = setV(value, field, { field = it }) {
-            Config.customChannelUrl = it
-            Info.remote = Info.EMPTY_REMOTE
-        }
     override val description get() = value.asText()
+    override var value
+        get() = Config.customChannelUrl
+        set(value) {
+            Config.customChannelUrl = value
+            Info.remote = Info.EMPTY_REMOTE
+            notifyPropertyChanged(BR.description)
+        }
 
-    override val inputResult get() = result
-
-    @get:Bindable
-    var result = value
-        set(value) = set(value, field, { field = it }, BR.result)
+    override var inputResult: String = value
+        set(value) = set(value, field, { field = it }, BR.inputResult)
 
     override fun refresh() {
         isEnabled = UpdateChannel.value == Config.Value.CUSTOM_CHANNEL
@@ -178,9 +177,10 @@ object UpdateChannelUrl : BaseSettingsItem.Input() {
 object UpdateChecker : BaseSettingsItem.Toggle() {
     override val title = R.string.settings_check_update_title.asText()
     override val description = R.string.settings_check_update_summary.asText()
-    override var value = Config.checkUpdate
-        set(value) = setV(value, field, { field = it }) {
-            Config.checkUpdate = it
+    override var value
+        get() = Config.checkUpdate
+        set(value) {
+            Config.checkUpdate = value
             JobService.schedule(AppContext)
         }
 }
@@ -188,51 +188,14 @@ object UpdateChecker : BaseSettingsItem.Toggle() {
 object DoHToggle : BaseSettingsItem.Toggle() {
     override val title = R.string.settings_doh_title.asText()
     override val description = R.string.settings_doh_description.asText()
-    override var value = Config.doh
-        set(value) = setV(value, field, { field = it }) {
-            Config.doh = it
-        }
+    override var value by Config::doh
 }
 
-// check whether is module already installed beforehand?
 object SystemlessHosts : BaseSettingsItem.Blank() {
     override val title = R.string.settings_hosts_title.asText()
     override val description = R.string.settings_hosts_summary.asText()
 }
 
-object Tapjack : BaseSettingsItem.Toggle() {
-    override val title = R.string.settings_su_tapjack_title.asText()
-    override var description = R.string.settings_su_tapjack_summary.asText()
-    override var value = Config.suTapjack
-        set(value) = setV(value, field, { field = it }) { Config.suTapjack = it }
-}
-
-object Biometrics : BaseSettingsItem.Toggle() {
-    override val title = R.string.settings_su_biometric_title.asText()
-    override var value = Config.suBiometric
-        set(value) = setV(value, field, { field = it }) { Config.suBiometric = it }
-    override var description = R.string.settings_su_biometric_summary.asText()
-
-    override fun refresh() {
-        isEnabled = BiometricHelper.isSupported
-        if (!isEnabled) {
-            value = false
-            description = R.string.no_biometric.asText()
-        }
-    }
-}
-
-object Reauthenticate : BaseSettingsItem.Toggle() {
-    override val title = R.string.settings_su_reauth_title.asText()
-    override val description = R.string.settings_su_reauth_summary.asText()
-    override var value = Config.suReAuth
-        set(value) = setV(value, field, { field = it }) { Config.suReAuth = it }
-
-    override fun refresh() {
-        isEnabled = Build.VERSION.SDK_INT < Build.VERSION_CODES.O && Utils.showSuperUser()
-    }
-}
-
 // --- Magisk
 
 object Magisk : BaseSettingsItem.Section() {
@@ -244,11 +207,13 @@ object Zygisk : BaseSettingsItem.Toggle() {
     override val description get() =
         if (mismatch) R.string.reboot_apply_change.asText()
         else R.string.settings_zygisk_summary.asText()
-    override var value = Config.zygisk
-        set(value) = setV(value, field, { field = it }) {
-            Config.zygisk = it
-            DenyList.isEnabled = it
-            DenyListConfig.isEnabled = it
+    override var value
+        get() = Config.zygisk
+        set(value) {
+            Config.zygisk = value
+            DenyList.isEnabled = value
+            DenyListConfig.isEnabled = value
+            notifyPropertyChanged(BR.description)
             DenyList.notifyPropertyChanged(BR.description)
         }
     val mismatch get() = value != Info.isZygiskEnabled
@@ -267,11 +232,16 @@ object DenyList : BaseSettingsItem.Toggle() {
         }
 
     override var value = Config.denyList
-        set(value) = setV(value, field, { field = it }) {
-            val cmd = if (it) "enable" else "disable"
+        set(value) {
+            field = value
+            val cmd = if (value) "enable" else "disable"
             Shell.su("magisk --denylist $cmd").submit { result ->
-                if (result.isSuccess) Config.denyList = it
-                else field = !it
+                if (result.isSuccess) {
+                    Config.denyList = value
+                } else {
+                    field = !value
+                    notifyPropertyChanged(BR.checked)
+                }
             }
         }
 
@@ -290,6 +260,27 @@ object DenyListConfig : BaseSettingsItem.Blank() {
 
 // --- Superuser
 
+object Tapjack : BaseSettingsItem.Toggle() {
+    override val title = R.string.settings_su_tapjack_title.asText()
+    override val description = R.string.settings_su_tapjack_summary.asText()
+    override var value by Config::suTapjack
+}
+
+object Biometrics : BaseSettingsItem.Toggle() {
+    override val title = R.string.settings_su_biometric_title.asText()
+    override var description = R.string.settings_su_biometric_summary.asText()
+    override var value by Config::suBiometric
+
+    override fun refresh() {
+        isEnabled = BiometricHelper.isSupported
+        if (!isEnabled) {
+            value = false
+            description = R.string.no_biometric.asText()
+            notifyPropertyChanged(BR.checked)
+        }
+    }
+}
+
 object Superuser : BaseSettingsItem.Section() {
     override val title = R.string.superuser.asText()
 }
@@ -297,22 +288,14 @@ object Superuser : BaseSettingsItem.Section() {
 object AccessMode : BaseSettingsItem.Selector() {
     override val title = R.string.superuser_access.asText()
     override val entryRes = R.array.su_access
-
-    override var value = Config.rootMode
-        set(value) = setV(value, field, { field = it }) {
-            Config.rootMode = it
-        }
+    override var value by Config::rootMode
 }
 
 object MultiuserMode : BaseSettingsItem.Selector() {
     override val title = R.string.multiuser_mode.asText()
     override val entryRes = R.array.multiuser_mode
     override val descriptionRes = R.array.multiuser_summary
-
-    override var value = Config.suMultiuserMode
-        set(value) = setV(value, field, { field = it }) {
-            Config.suMultiuserMode = it
-        }
+    override var value by Config::suMultiuserMode
 
     override fun refresh() {
         isEnabled = Const.USER_ID == 0
@@ -323,21 +306,13 @@ object MountNamespaceMode : BaseSettingsItem.Selector() {
     override val title = R.string.mount_namespace_mode.asText()
     override val entryRes = R.array.namespace
     override val descriptionRes = R.array.namespace_summary
-
-    override var value = Config.suMntNamespaceMode
-        set(value) = setV(value, field, { field = it }) {
-            Config.suMntNamespaceMode = it
-        }
+    override var value by Config::suMntNamespaceMode
 }
 
 object AutomaticResponse : BaseSettingsItem.Selector() {
     override val title = R.string.auto_response.asText()
     override val entryRes = R.array.auto_response
-
-    override var value = Config.suAutoResponse
-        set(value) = setV(value, field, { field = it }) {
-            Config.suAutoResponse = it
-        }
+    override var value by Config::suAutoResponse
 }
 
 object RequestTimeout : BaseSettingsItem.Selector() {
@@ -345,21 +320,25 @@ object RequestTimeout : BaseSettingsItem.Selector() {
     override val entryRes = R.array.request_timeout
 
     private val entryValues = listOf(10, 15, 20, 30, 45, 60)
-    override var value = selected
-        set(value) = setV(value, field, { field = it }) {
-            Config.suDefaultTimeout = entryValues[it]
+    override var value = entryValues.indexOfFirst { it == Config.suDefaultTimeout }
+        set(value) {
+            field = value
+            Config.suDefaultTimeout = entryValues[value]
         }
-
-    private val selected: Int
-        get() = entryValues.indexOfFirst { it == Config.suDefaultTimeout }
 }
 
 object SUNotification : BaseSettingsItem.Selector() {
     override val title = R.string.superuser_notification.asText()
     override val entryRes = R.array.su_notification
-
-    override var value = Config.suNotification
-        set(value) = setV(value, field, { field = it }) {
-            Config.suNotification = it
-        }
+    override var value by Config::suNotification
+}
+
+object Reauthenticate : BaseSettingsItem.Toggle() {
+    override val title = R.string.settings_su_reauth_title.asText()
+    override val description = R.string.settings_su_reauth_summary.asText()
+    override var value by Config::suReAuth
+
+    override fun refresh() {
+        isEnabled = Build.VERSION.SDK_INT < Build.VERSION_CODES.O && Utils.showSuperUser()
+    }
 }

app/src/main/java/com/topjohnwu/magisk/ui/settings/SettingsViewModel.kt

@@ -26,10 +26,10 @@ import com.topjohnwu.magisk.utils.Utils
 import com.topjohnwu.superuser.Shell
 import kotlinx.coroutines.launch
 
-class SettingsViewModel : BaseViewModel(), BaseSettingsItem.Callback {
+class SettingsViewModel : BaseViewModel(), BaseSettingsItem.Handler {
 
     val adapter = adapterOf<BaseSettingsItem>()
-    val itemBinding = itemBindingOf<BaseSettingsItem> { it.bindExtra(BR.callback, this) }
+    val itemBinding = itemBindingOf<BaseSettingsItem> { it.bindExtra(BR.handler, this) }
     val items = createItems()
 
     init {
@@ -96,27 +96,25 @@ class SettingsViewModel : BaseViewModel(), BaseSettingsItem.Callback {
         return list
     }
 
-    override fun onItemPressed(view: View, item: BaseSettingsItem, callback: () -> Unit) {
+    override fun onItemPressed(view: View, item: BaseSettingsItem, andThen: () -> Unit) {
         when (item) {
-            is DownloadPath -> withExternalRW(callback)
-            is Biometrics -> authenticate(callback)
-            is Theme ->
-                SettingsFragmentDirections.actionSettingsFragmentToThemeFragment().navigate()
-            is DenyListConfig ->
-                SettingsFragmentDirections.actionSettingsFragmentToDenyFragment().navigate()
-            is SystemlessHosts -> createHosts()
-            is Restore -> RestoreAppDialog().publish()
-            is AddShortcut -> AddHomeIconEvent().publish()
-            else -> callback()
+            DownloadPath -> withExternalRW(andThen)
+            Biometrics -> authenticate(andThen)
+            Theme -> SettingsFragmentDirections.actionSettingsFragmentToThemeFragment().navigate()
+            DenyListConfig -> SettingsFragmentDirections.actionSettingsFragmentToDenyFragment().navigate()
+            SystemlessHosts -> createHosts()
+            Restore -> RestoreAppDialog().publish()
+            AddShortcut -> AddHomeIconEvent().publish()
+            else -> andThen()
         }
     }
 
-    override fun onItemChanged(view: View, item: BaseSettingsItem) {
+    override fun onItemAction(view: View, item: BaseSettingsItem) {
         when (item) {
-            is Language -> RecreateEvent().publish()
-            is UpdateChannel -> openUrlIfNecessary(view)
+            Language -> RecreateEvent().publish()
+            UpdateChannel -> openUrlIfNecessary(view)
             is Hide -> viewModelScope.launch { HideAPK.hide(view.activity, item.value) }
-            is Zygisk -> if (Zygisk.mismatch) SnackbarEvent(R.string.reboot_apply_change).publish()
+            Zygisk -> if (Zygisk.mismatch) SnackbarEvent(R.string.reboot_apply_change).publish()
             else -> Unit
         }
     }

app/src/main/res/layout/dialog_settings_download_path.xml

@@ -42,7 +42,7 @@
                 android:layout_width="match_parent"
                 android:layout_height="wrap_content"
                 android:inputType="textUri"
-                android:text="@={data.result}"
+                android:text="@={data.inputResult}"
                 android:textAppearance="@style/AppearanceFoundation.Body"
                 android:textColor="?colorOnSurface"
                 tools:text="@tools:sample/lorem" />

app/src/main/res/layout/dialog_settings_update_channel.xml

@@ -34,7 +34,7 @@
                 android:layout_width="match_parent"
                 android:layout_height="wrap_content"
                 android:inputType="textUri"
-                android:text="@={data.result}"
+                android:text="@={data.inputResult}"
                 android:textAppearance="@style/AppearanceFoundation.Body"
                 android:textColor="?colorOnSurface"
                 tools:text="@tools:sample/lorem" />

app/src/main/res/layout/item_settings.xml

@@ -10,8 +10,8 @@
             type="com.topjohnwu.magisk.ui.settings.BaseSettingsItem" />
 
         <variable
-            name="callback"
-            type="com.topjohnwu.magisk.ui.settings.BaseSettingsItem.Callback" />
+            name="handler"
+            type="com.topjohnwu.magisk.ui.settings.BaseSettingsItem.Handler" />
 
     </data>
 
@@ -23,7 +23,7 @@
         android:alpha="@{item.enabled ? 1f : .5f}"
         android:clickable="@{item.enabled}"
         android:focusable="@{item.enabled}"
-        android:onClick="@{(view) -> item.onPressed(view, callback)}"
+        android:onClick="@{(view) -> item.onPressed(view, handler)}"
         tools:layout_gravity="center">
 
         <LinearLayout
@@ -81,7 +81,7 @@
                 android:layout_height="wrap_content"
                 android:checked="@{item.checked}"
                 android:focusable="@{item.enabled}"
-                android:onCheckedChanged="@{(v, c) -> item.onToggle(v, callback, c)}" />
+                android:onCheckedChanged="@{(v, c) -> item.onToggle(v, handler, c)}" />
 
         </LinearLayout>