edvin / tornadofx

Lightweight JavaFX Framework for Kotlin
Apache License 2.0
3.68k stars 271 forks source link

ViewModel autocommit is unreliable with required validator #1324

Open aleferna12 opened 3 years ago

aleferna12 commented 3 years ago

When you setup a ViewModel property with autocommit == true and use the required validator on it it behaves weirdly. Lets suppose we setup a model like such:

class Person {
    val tempProperty = SimpleDoubleProperty(0.5)
}
class PersonModel(person: Person): ViewModel() {
    val temp = bind(true) { person.tempProperty }
}
// Later:
val model = PersonModel(Person())
override val root = textfield(model.temp).required()

When we erase the value from the textinput, the validator prevents the value from being commited, but the model property updates (to 0.0). Then when we type 0 into the textfield, the model doesnt detect a change and doesnt commit the model. This means the expected commit of the person value to 0.0 is skipped.

SKeeneCode commented 3 years ago

I cannot seem to reproduce this. In everything I tried when deleting the contents of the textfield the value of temp becomes null, not 0.0.

aleferna12 commented 3 years ago

Weird... tornadofx version - 1.7.20 kotlin version - 1.4.0

Full working example:

class MainView : View("Hello TornadoFX") {
    var person = Person()
    val model = PersonModel(person)
    override val root = hbox(20) {
        label (person.tempProperty)
        label(model.temp)
        textfield(model.temp).required()
    }
}

class Person {
    val tempProperty = SimpleDoubleProperty(1.0)
    val temp by tempProperty
}

class PersonModel(person: Person): ViewModel() {
    val temp = bind(true) { person.tempProperty }
}

Steps to reproduce:

  1. Start the program
  2. Press backspace to erase the value
  3. Notice how the second label, which represents the model's "temp" shows 0.0 (other methods such as printing the value on the terminal also confirm that the value is 0.0)
  4. Type "0" on the textfield
  5. Notice how the value on the first label, which represents the person "tempProperty" is still 1

Hope its more clear now!

SKeeneCode commented 3 years ago

So this does seem to be a bug, and it seems to be introduced by kotlin 1.4.0-rc. If you downgrade below that to 1.3.72 you will get what I assume is the intended behavior of the view modal value being null (which is also why I could not reproduce your problem initially):

image

There is a WIP PR for tornadofx2 which upgrades to 1.4.32. The bug will need to be fixed there I suppose.

aleferna12 commented 3 years ago

Ok, thank you very much for the update :)