edvin / tornadofx

Lightweight JavaFX Framework for Kotlin
Apache License 2.0
3.67k stars 269 forks source link

TreeView - populate with an "externally" created ObservableList freezes application #925

Open MartinMajewski opened 5 years ago

MartinMajewski commented 5 years ago

Hi there,

I have a hard time figuring out how TreeView in TornadoFX works (I am also new to JavaFX) - I neither find the guide nor the examples good or intuitive to get into using the TreeView (or get a general idea of how e.g. populate() works - coming from an OOP world).

However, from what I understand is that populate should return an iterable List of objects of the type the TreeItem shall hold. It is mentioned that returning an ObserableList will work and reflect every change made to it in the TreeView. Right?

So, please take a look at my code below where I want to read in File objects into an ObservableList and populate the Treeview with it. When using file.listFiles().asIterable() which provides an immutable Array of files, it works to the point of showing the tree. However, the listFiles() function is a little bit unpredictable, and therefore I want to be able to walk down the entire folder structure tree. But using an ObservableList makes the app hang on start:

package net.martinmajewski.tornadofx.demo.view

import apple.laf.JRSUIUtils
import javafx.collections.ObservableList
import javafx.scene.control.TreeItem
import javafx.scene.control.TreeView
import tornadofx.*
import java.io.File
import java.io.FileFilter

class FolderStructureTreeView : View("Folder Structure Tree View") {

    val file = File("/Users/martinmajewski/Library/Mobile Documents/com~apple~CloudDocs/3DPrinting")
    var tree = mutableListOf<File>().observable()

    override val root = treeview<File> {

        minWidth = 300.0

        buildTreeList()

        root = TreeItem(tree.get(0))
        root.expandAll()

        populate {
            tree.asIterable()
        }

        cellFormat {
            text = it.name
        }

        onUserSelect {
            println(it.toString())
        }

    }

    fun buildTreeList(){
        file.walkTopDown().forEach { f1 ->
            println("Adding ${f1.name} to list")
            tree.add(f1)
        }
    }
}

Is this a bug or is this me not getting the right way of doing it simply?

As far as I see TreeView has little to none filtering options which would allow hiding entries based on filtering criteria etc. One can only try to remove branches and leaves from the parent. However, this causes other issues like "multiple access to list exceptions" and such and does not reflect changes on the underlying data.

Best wishes, Martin

edvin commented 5 years ago

populate is asked to return a list of the children if it is a branch, or null if it is a leaf. Make sure you return the same instance type as the treeview type, and not TreeItem instances. I've rewritten your example to use the java.nio.Path API instead of File. Notice that I use an extension function called Path.list to return the actual children. This function makes sure to close the file handle via use to avoid file descriptor leaks.

The content of populate then becomes as easy as determining whether the current path represents a folder or a file.

class FolderStructureTreeView : View("Folder Structure Tree View") {
    private val rootFolder = Paths.get("/Users/edvin/Projects")

    override val root = treeview<Path>(TreeItem(rootFolder)) {
        minWidth = 300.0

        populate {
            if (Files.isDirectory(it.value))
                it.value.list()
            else
                null
        }

        cellFormat {
            text = "${it.fileName}"
        }

        onUserSelect(System.out::println)
    }

}

fun Path.list() = Files.list(this).use { it.toList() }

It's important to note that the populate callback is passed a TreeItem instance, so you need to unwrap the value to get to the actual item representing that TreeItrem.

Also notice that I pass in the root item in the treeview builder constructor instead of assigning it separately.

image

You can filter the data either by passing in a function to the list extension function, or doing it before you return the items from populate.

MartinMajewski commented 5 years ago

Hi @edvin and thank you very much for your help. I appreciate it and will look into the provided code in a second.

Maybe you could provide this example also in your guide and code examples?

Off-topic, but may be helpful (?!): Regarding your guide: I come from a C++, Swift, and Java background, working with UI's for many years. However, I haven't touched Java for a long time now and back then worked on Android and Swing based applications. In Swift, you have the "out of the box" UI experience programming for Mac or iOS. In my opinion, it is most motivating to learn a new language and new language concepts when you can directly work on UI based applications than writing CLI programs. Therefore, I chose TornadoFX, but I find it hard to get into it when relying on your guide - and there isn't much else out here yet (reading through JavaFX guides isn't also helpful, because they use mostly Java concepts - which I understand but at this point in time cannot transfer to Kotlin easily)

The reason for not liking the current guide is that you put a lot of effort in firstly showing how to do things to tell us later that this is not a good way thus providing entirely other approaches. This is, in my opinion, a waste of energy and time for the reader. Furthermore, you don't show basic examples of how the API has to be used in general but you wrap it into examples which are sometimes per se hard to understand. The TreeView part is an excellent showcase for this: you use a Person class which contains departments and person names just to get me as a reader confused when you fiddle around with the way of how to remap the entries. You even confess that this is not intuitive in the guide. This approach is so tangled and problem specific that mapping it to other problems becomes hard. So knowing it by yourself, why not start with simple things, maybe even abstract branch-entries like (A, A.1, A.2, B, B.1, etc.) and focus more on explaining the ins and outs of the API calls used. It is, in my opinion, easier for the reader to substitute A.1, B.2 with other values than figuring out why you have to return null in populate, or why you check for the parent being the root object when just four lines above you assign a value to root. It makes me wonder if this check is a necessity and I miss something by not using it (especially when my code fails running) or if this is just a specific solution for your "department is not a person's name" dilemma. From the current state, I wouldn't buy the guide if it was a book. However, I feel like being alone, because I don't see a lot of people complaining about the guide. So maybe this is just me?

Just my two cents. Please don't take it as an offense but as feedback!

edvin commented 5 years ago

No offense taken, in fact you have several valid points. I (and probably the other authors) regret the pattern where we try to explain the underlying workings via inefficient examples instead of just showing best practices right away. Another issue is that the syntax evolved pretty intensely as the guide was being written, so a lot of examples aren't even using best practices anymore.

However, you won't have much success with a Kotlin based UI framework unless you learn Kotlin first. I would suggest you study up on that. Workflow and layout of the framework is made to make it easy to pick up for Kotlin users, so everything will make a lot more sense once you know Kotlin.

When you're up to date, perhaps you want to help remake the guide or contribute in other ways? This is a community project, not a commercial product :)

MartinMajewski commented 5 years ago

@edvin Thanks for the feedback.

Okay, it is not as if I would look at Kotlin code and see only Mandarin or something. I have just a hard (slow) time of reading all the functional parts in one shot and I have to look back and forth in the Kotlin references to recall what certain built-in properties do (like the use property in your list extension above). Furthermore, being used to read Swift some terminology and keywords confuses me still (I'm still a little bit upset that var and val are so similar looking in Kotlin whereas var and let is much easier to distinguish at the first glance in Swift - to name a very basic example).

I will gladly contribute to the guide - maybe being a newcomer would help to write some unbiased lines? From my experience, when reaching a certain still level one can simply forget and miss the source of confusion a beginner is facing.

BTT:

..., or null if it is a leaf.

So what is it actually doing when it gets a null? It is just a terminator value for walking down the tree's current branch? Or does it perform some sort of "attaching" operation to the branch still?

JavaFX uses a more raw approach of creating trees, branches and leafs, which I understand immediately but it is very verbose and therefore not that "Kotlin-ish". :-)

Also notice that I pass in the root item in the treeview builder constructor instead of assigning it separately.

Reason? Is there an important point not following the guide here or just the evolution of the API since writing?