edvin / tornadofx

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

runAsyncWithOverlay with view root #873

Open DeMol-EE opened 5 years ago

DeMol-EE commented 5 years ago

I'm trying to get a view to refresh each time it is docked but I'm running into trouble. To illustrate what's happening, I've set up a minimal code sample based off the IDEA template for a new tornadofx project with maven. I've edited the generated MainView to look as follows:

package com.example.demo.view

import tornadofx.*

class MainView : View("Hello TornadoFX") {

    private val wurds = observableList<String>()

    override val root = hbox(10)

    init {
        wurds.onChange {
            redrawContent()
        }
    }

    private fun redrawContent() {
        root.replaceChildren {
            wurds.forEach {
                label(it)
            }
        }
    }

    override fun onDock() {
        println("ondock")
        onRefresh()
    }

    override fun onRefresh() {
        println("onrefresh")
        with(root) {
            runAsyncWithOverlay {
                Thread.sleep(1000)
                listOf("Hello", "Edvin")
            } ui {
                wurds.setAll(it)
            }
        }
    }
}

My idea was to use an observable list as a sort of model of what labels should be shown and registering a listener to changes in the model from init. When changes happen, and each time the view is docked, I want the model to be refreshed based on the results of a database query (here replaced by sleeping the thread). The code above does not execute but instead gets stuck in an infinite loop of docking and calling refresh. I believe the underlying reason must be that runAsyncWithOverlay actually undocks and redocks the view but I'm not sure.

How would I ideally achieve what I'm trying to do?

edvin commented 5 years ago

runAsyncWithProgress needs to be run on an element who's parent is capable of holding children, as it will swap out the progress node for the node you called it on. Therefore, you can't run it on the root node.

Here is a piece of code where this is taken into account, by wrapping the root in a stackpane:

class MainView : View("Hello TornadoFX") {
    private val wurds = observableList<String>()
    var container: HBox by singleAssign()

    override val root = stackpane {
        setPrefSize(800.0, 600.0)
        container = hbox(10)
    }

    init {
        wurds.onChange {
            redrawContent()
        }
    }

    private fun redrawContent() {
        container.replaceChildren {
            wurds.forEach {
                label(it)
            }
        }
    }

    override fun onDock() {
        println("ondock")
        container.runAsyncWithProgress {
            Thread.sleep(1000)
            listOf("Hello", "Edvin")
        } ui {
            wurds.setAll(it)
        }
    }
}

I also want to point out that we have built in functionality for syncing a list of items to a list of nodes:

class MainView : View("Hello TornadoFX") {
    private val wurds = observableList<String>()
    var container: HBox by singleAssign()

    override val root = stackpane {
        setPrefSize(800.0, 600.0)
        container = hbox(10) {
            bindChildren(wurds) {
                label(it)
            }
        }
    }

    override fun onDock() {
        container.runAsyncWithProgress {
            Thread.sleep(1000)
            listOf("Hello", "Edvin")
        } ui {
            wurds.setAll(it)
        }
    }
}
DeMol-EE commented 5 years ago

Thanks for the quick reply! I had concocted a similar solution by wrapping the node in a borderpane instead of a stackpane. I didn't know about bindChildren and I would swear it's not mentioned in the TornadoFX Guide... not the version I downloaded at least!

I find myself reusing that template a couple of times where I have a view that I want to refresh on dock. I've actually created a subclass of View which implements onRefresh by means of runAsync {loadData()} ui {renderData(it)}, expecting subclasses to implement those new methods. Did you ever consider creating a UI component which does this and found it was not worth it?

edvin commented 5 years ago

Feel free to add a couple of lines to the guide about bindChildren if you have time :)

Beware that onRefresh has specific semantics with regards to the Workspace, so you shouldn't reuse it unless you use Workspaces.

I almost never have the need to perform any rendering after getting to the data, as I use ItemViewModels bound to the UI elements. I can think of only a few use cases where "rendering" after getting your data is necessary so if you feel you need this all the time you might be doing something wrong or at least not optimal :) If you want, you can post some code samples on Slack where you feel you need this pattern, and I'll have a look to see if there is better ways of achieving what you want.

DeMol-EE commented 5 years ago

Haha, I actually might put that in! Now I wonder how many other little gems are hidden in the code ;-) I also have a pull request for mac friendly accelerators for workspace actions but I'm too shy to submit it... also because I can't get the code to build on my machine because my maven profile fails to find some plugins. I'll take a closer look at it tomorrow.

Simply put, I'm working on an application that is a frontend to a database that is being manipulated by multiple agents simultaneously. While it's not necessarily required to refresh each time a view is docked, hitting refresh is usually what you would do as a first action when visiting the view anyway. Consider the objects in Java memory as a continuously possibly dirty cache for the database, which contains the single truth.

Though I find the bindings between ItemViewModels and builders for making UI elements very impressive, I can't always bind what I want to a property (recall binding tableview columns to a list? that was also me and the solution there was to re-create the table each time - which is more or less what I'm doing here, too). I'm going to play around with bindChildren though and see where that gets me.

edvin commented 5 years ago

Hehe, don't be shy :)

One of the only good reasons for needing to redraw the UI is actually when each "item" has a different data model, and you need your table columns or fields to reflect that. If this is not the case, then you very seldom need to redraw the UI, and all that is needed when you hit refresh is to put a new item in the ViewModel and the UI will automatically update.

If you have a set of different, but known data model layouts, you could use View subclasses and show the correct view instead. This will probably be less messy, if it fits with your scheme.