Commit 79aa261c authored by Viktor De Pasquale's avatar Viktor De Pasquale Committed by John Wu

Fixed manager beginning to hide immediately on field change

Bug was caused by lenient usage of "value" property defined in the "line item" in settings. Developer error allowed to use the internal value, that was not properly protected, in a way that did not conform with the latest "Observer" rewrite.

Additional comments were added to hopefully prevent bugs of this kind in the future. The property is now properly protected so it gives away clues that this access is considered "not cool".
parent ac2a9da4
...@@ -60,7 +60,20 @@ sealed class SettingsItem : ObservableItem<SettingsItem>() { ...@@ -60,7 +60,20 @@ sealed class SettingsItem : ObservableItem<SettingsItem>() {
abstract class Value<T> : SettingsItem() { abstract class Value<T> : SettingsItem() {
/**
* 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 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 protected var callbackVars: Pair<View, Callback>? = null
...@@ -76,7 +89,7 @@ sealed class SettingsItem : ObservableItem<SettingsItem>() { ...@@ -76,7 +89,7 @@ sealed class SettingsItem : ObservableItem<SettingsItem>() {
protected inline fun <reified T> setV( protected inline fun <reified T> setV(
new: T, old: T, setter: (T) -> Unit, afterChanged: (T) -> Unit = {}) { new: T, old: T, setter: (T) -> Unit, afterChanged: (T) -> Unit = {}) {
set(new, old, setter, BR.value, BR.description, BR.checked) { set(new, old, setter, BR.description, BR.checked) {
afterChanged(it) afterChanged(it)
callbackVars?.let { (view, callback) -> callbackVars?.let { (view, callback) ->
callbackVars = null callbackVars = null
......
...@@ -80,19 +80,23 @@ object Hide : SettingsItem.Input() { ...@@ -80,19 +80,23 @@ object Hide : SettingsItem.Input() {
override val title = R.string.settings_hide_manager_title.asTransitive() override val title = R.string.settings_hide_manager_title.asTransitive()
override val description = R.string.settings_hide_manager_summary.asTransitive() override val description = R.string.settings_hide_manager_summary.asTransitive()
@get:Bindable
override var value = "Manager" override var value = "Manager"
set(value) = setV(value, field, { field = it }) { set(value) = setV(value, field, { field = it })
notifyPropertyChanged(BR.error)
} override val inputResult
get() = if (isError) null else result
@get:Bindable @get:Bindable
val isError get() = value.length > 14 || value.isBlank() var result = value
set(value) = set(value, field, { field = it }, BR.result, BR.error)
override val inputResult get() = if (isError) null else value @get:Bindable
val isError
get() = value.length > 14 || value.isBlank()
override fun getView(context: Context) = DialogSettingsAppNameBinding override fun getView(context: Context) = DialogSettingsAppNameBinding
.inflate(LayoutInflater.from(context)).also { it.data = this }.root .inflate(LayoutInflater.from(context)).also { it.data = this }.root
} }
object Restore : SettingsItem.Blank() { object Restore : SettingsItem.Blank() {
...@@ -119,20 +123,22 @@ object DownloadPath : SettingsItem.Input() { ...@@ -119,20 +123,22 @@ object DownloadPath : SettingsItem.Input() {
set(value) = set(value, field, { field = it }, BR.result, BR.path) set(value) = set(value, field, { field = it }, BR.result, BR.path)
@get:Bindable @get:Bindable
val path get() = File(Environment.getExternalStorageDirectory(), result).absolutePath.orEmpty() val path
get() = File(Environment.getExternalStorageDirectory(), result).absolutePath.orEmpty()
override fun getView(context: Context) = DialogSettingsDownloadPathBinding override fun getView(context: Context) = DialogSettingsDownloadPathBinding
.inflate(LayoutInflater.from(context)).also { it.data = this }.root .inflate(LayoutInflater.from(context)).also { it.data = this }.root
} }
object UpdateChannel : SettingsItem.Selector() { object UpdateChannel : SettingsItem.Selector() {
override var value = Config.updateChannel override var value = Config.updateChannel
set(value) = setV(value, field, { field = it }) { Config.updateChannel = it } set(value) = setV(value, field, { field = it }) { Config.updateChannel = it }
override val title = R.string.settings_update_channel_title.asTransitive() override val title = R.string.settings_update_channel_title.asTransitive()
override val entries get() = resources.getStringArray(R.array.update_channel).let { override val entries
if (BuildConfig.DEBUG) it.toMutableList().apply { add("Canary") }.toTypedArray() else it get() = resources.getStringArray(R.array.update_channel).let {
} if (BuildConfig.DEBUG) it.toMutableList().apply { add("Canary") }.toTypedArray() else it
}
override val entryValRes = R.array.value_array override val entryValRes = R.array.value_array
} }
......
...@@ -40,7 +40,7 @@ ...@@ -40,7 +40,7 @@
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:inputType="textCapWords" android:inputType="textCapWords"
android:text="@={data.value}" android:text="@={data.result}"
android:textAppearance="@style/AppearanceFoundation.Body" android:textAppearance="@style/AppearanceFoundation.Body"
android:textColor="?colorOnSurface" android:textColor="?colorOnSurface"
tools:text="@tools:sample/lorem" /> tools:text="@tools:sample/lorem" />
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment