edvin / tornadofx

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

Working with controls that don't have builders available. #95

Closed radicaled closed 8 years ago

radicaled commented 8 years ago

Here's a modal dialog I have:

class NewDocumentDialog : Fragment() {
    override val root = VBox()

    init {
        title = "New Document"
        with(root) {
            label("New Document")
            textfield { promptText = "New Document Name" }
            add(ButtonBar().apply {
                prefHeight = 20.0
                prefWidth = 410.0

                this.buttons.add(Button("Create").apply {
                    onAction = EventHandler { createDocument() }
                })
                this.buttons.add(Button("Cancel").apply {
                    onAction = EventHandler { close() }
                })

                // can't use -- gets affixed to root node (VBox).
                // TODO: fun ButtonBar.button ...?
                // button("Create").onAction = EventHandler { createDocument() }
                // button("Cancel").onAction = EventHandler { close() }
            })
        }
    }

    fun createDocument() {
    }

    fun close() {
        this.closeModal()
    }
}

I've been using the

add(Control.apply {
   ...
});

pattern to work with controls that don't have builders available, usually custom ones. I'm wondering now if I'm using the right approach of add and apply, and if those with more experience have come up with a better way to handle custom controls.

Also, this feels kind of like a misappropriation of the issues tracker -- is there a mailing list for tornadofx that's not listed in the README?

edvin commented 8 years ago

Thanks for reporting this! I had missed the ButtonBar builder, so I'll add that right away. To solve your problem with using builders inside an object that is not a Pane, I propose adding a small function called children that will change the scope so that builders created inside will be added to a designated list of nodes. Then your example can be written like this:

class NewDocumentDialog : Fragment() {
    override val root = VBox()

    init {
        title = "New Document"
        with(root) {
            label("New Document")
            textfield { promptText = "New Document Name" }
            add(ButtonBar().apply {
                prefHeight = 20.0
                prefWidth = 410.0

                // Append any builders created inside to `ButtonBar.buttons`
                children(buttons) {
                    button("Create").setOnAction { createDocument() }
                    button("Cancel").setOnAction { close() }
                }
            })
        }
    }

    fun createDocument() {
    }

    fun close() {
        this.closeModal()
    }
}

Also note the setOnAction function that reduces boilerplate when adding action listeners to buttons.

Does this seem reasonable to you?

edvin commented 8 years ago

I've commited the children function above as well as the buttonbar builder, so your example can now be written like this:

with(root) {
    label("New Document")
    textfield { promptText = "New Document Name" }
    buttonbar {
        prefHeight = 20.0
        prefWidth = 410.0

        children(buttons) {
            button("Create").setOnAction { createDocument() }
            button("Cancel").setOnAction { close() }
        }
    }
}

@thomasnield The children function should probably be mentioned in the guide, I'll add it to the Wiki now :)

edvin commented 8 years ago

I also added special support for adding buttons to a buttonbar, so the above example is now down to:

buttonbar {
    prefHeight = 20.0
    prefWidth = 410.0

    button("Create").setOnAction { createDocument() }
    button("Cancel").setOnAction { close() }
}

I have explained this further with examples and more info in the Wiki:

https://github.com/edvin/tornadofx/wiki/Type-Safe-Builders#what-if-there-is-no-builder-for-the-control-im-adding

thomasnield commented 8 years ago

@edvin noted!

ruckustboom commented 8 years ago

I noticed the children() function operates in Pane scope (which makes sense as all the builders are made for Pane). What would happen if you added something it wasn't expecting? For example:

with(root) {
    label("New Document")
    textfield { promptText = "New Document Name" }
    buttonbar {
        prefHeight = 20.0
        prefWidth = 410.0

        children(buttons) {
            button("Create").setOnAction { createDocument() }
            vbox { label("Uh-oh") }  // What happens here?
            button("Cancel").setOnAction { close() }
        }
    }
}

I can't just test it out right now, which is why I'm asking.

edvin commented 8 years ago

That works fine, since ButtonBar.buttons is basically an HBox with some alignment tricks for buttons :)

ruckustboom commented 8 years ago

I didn't realize that. Good to know.

edvin commented 8 years ago

Are you satisfied @radicaled ? Should we close the issue?

ruckustboom commented 8 years ago

Would this basically be the same workflow for ToolBar?

edvin commented 8 years ago

Yeah, for ToolBar you would use children(items), but ToolBar also have direct support for the button builder.

ruckustboom commented 8 years ago

I know that, but I was testing an idea a couple days ago that required more than buttons on a toolbar. This children trick should make that a lot easier.

radicaled commented 8 years ago

Are you satisfied @radicaled ? Should we close the issue?

Yeah, thanks, this looks really good @edvin ! Better than what I was going to do.

edvin commented 8 years ago

Great @radicaled :) @t-boom The builder pattern we use have certain limitations, and this is one of them. The alternative is to subclass everything though, and that sucks more IMO :) Once you know about the caveats though, it works really well :)