Tornadofx: Some More Control Builders

[thomasnield]

As I'm documenting the guide, I'm doing a sweep for more simple controls that probably should be added

ToggleButton
RadioButton
Slider
TextFlow
ChoiceBox
PasswordField
Separator
HyperLink
ColorPicker
FileChooser
Pagination
HTMLEditor

I know the TextFlow is a lesser-used control, but considering JavaFX lacks html formatting for label text (like Swing) it is helpful to format a string of text with different colors. I use this feature quite a bit to concatenate data with different conditional formatting rules.

Let me know if we want to scratch any of these, and I'll work on adding whatever we wan)

[thomasnield]

Of course, if anybody (including newcomers) want to knock some of these out I'll welcome the help. Many of these are pretty easy to implement.

[edvin]

Very good initiative! It would be great if some newcomers would like to add a couple of these! I suggest making a comment on this thread before you start with a control to avoid more people working on the same control.

Look at https://github.com/edvin/tornadofx/blob/master/src/main/java/tornadofx/Builders.kt for inspiration and examples.

[edvin]

I will hold off on https://github.com/edvin/tornadofx/issues/57 until this issue is resolved.

[thomasnield]

Good call. We will probably need to do a code freeze in some form when #57 gets implemented.

[edvin]

Yeah, but I figure it can be done in a couple of hours, so it shouldn't be a huge problem :)

[ruckustboom]

For the PasswordField, would want to pass in an initial value (such as a saved password)? This can always be done with apply { text = value }, but is it common enough to add an optional parameter to the Pane.passwordfield method?

Also, I'm assuming there's no real use case for double binding the text of a password field like there is for a text field, but let me know if I'm wrong.

[thomasnield]

Absolutely. Add a default value for the password() function. We try to streamline towards common use cases and that definitely is one.

I agree with that sentiment regarding binding like a text field. Doesn't feel like that would be very common, and I don't think that would encourage secure design.

[ruckustboom]

Sounds good

[ruckustboom]

For the ToggleButton, how should grouping be handled? I'm thinking either create a Pane.togglegroup method that doesn't add anything to the pane but does return a ToggleGroup, or have a toggle group scope of some sort in which toggle buttons can be added to the pane and automatically have the group attached. Of course, nothing is stopping us from doing both.

[ruckustboom]

The second option has some issues. If we make Pane.togglegroup return a ToggleGroup, it's context has no pane to add its buttons. If we have it return something like Pair<Pane, ToggleGroup>, there is more overhead/boilerplate for adding a button to it later.

Here's what I've thought of so far:

fun Pane.togglebutton(text: String = "", group: ToggleGroup? = null, op: (ToggleButton.() -> Unit)? = null) = opcr(this, ToggleButton(text).apply { if (group != null) toggleGroup = group }, op)

fun Pane.togglegroup(op: (Pair<Pane, ToggleGroup>.() -> Unit)? = null): Pair<Pane, ToggleGroup> {
    val group = Pair(this, ToggleGroup())
    op?.invoke(group)
    return group
}

fun Pair<Pane, ToggleGroup>.button(text: String = "", op: (ToggleButton.() -> Unit)? = null) = opcr(first, ToggleButton(text), op)

which would require something like:

val group = togglegroup {
    button("Alice")
}
togglebutton("Bob", group.second)

to add a button to the group after (I don't like the .second reference).

[ruckustboom]

Currently I just have the first option implemented

fun Pane.togglegroup() = ToggleGroup()

fun Pane.togglebutton(text: String = "", group: ToggleGroup? = null, op: (ToggleButton.() -> Unit)? = null)
        = opcr(this, ToggleButton(text).apply { if (group != null) toggleGroup = group }, op)

[ruckustboom]

(This problem also applies to RadioButtons)

[thomasnield]

Cafe internet is not cooperating with me. Give me a moment...

[thomasnield]

Okay these are some good points, let me think through this. Hacking my way through this right now.

[ruckustboom]

Just a heads up, I've also tried adding the pane as the userdata of the toggle group (though this pretty much bypasses type safety). My next idea either an extension property or a small class that extends from ToggleGroup

[thomasnield]

There's got to be cleaner way to do this without the Pair. Trying a few things.

[thomasnield]

Okay, maybe we need to step back because I think you and I are abstracting ToggleGroup into something it is not. Could not, in theory, ToggleButtons in a given ToggleGroup be completely separated? Sometimes ToggleButton is not used in a group at all.

While it seems natural, we start fighting JavaFX's design (whether we agree with it or not) if we treat ToggleGroup like it is a Pane that contains ToggleButtons.

Why not do this?

class MyView: View() {
    override val root = VBox()

    val grp = ToggleGroup()
    init {
        with(root) {

            togglebutton("1").apply {
                toggleGroup = grp
            }

            togglebutton("2").apply {
                toggleGroup = grp
            }

            togglebutton("3").apply {
                toggleGroup = grp
            }
        }
    }
}

If that's too repetitive for you, just create an internal extension function for re-usability.

class MyView: View() {
    override val root = VBox()

    val optionGrp = ToggleGroup()
    init {
        with(root) {
            togglebutton("1").addToOptionGrp()
            togglebutton("2").addToOptionGrp()
            togglebutton("3").addToOptionGrp()
        }
    }

    private fun ToggleButton.addToOptionGrp() {
        toggleGroup = optionGrp
    }
}

What do you think @edvin?

[thomasnield]

By the way, I'll strikethrough controls on the OP list as they get implemented.

[edvin]

I'm here, having a look at togglegroup now :)

[edvin]

One suggestion (still thinking though):

class MyView: View() {
    override val root = VBox()

    val optionGrp = ToggleGroup()
    init {
        with(root) {
            togglebutton("1", optionGrp)
            togglebutton("2", optionGrp)
            togglebutton("3", optionGrp)
        }
    }
}

[edvin]

Hmm... Maybe I'm missing something, but that seems quite simple to implement, and with no major drawbacks as far as I can see :)

[thomasnield]

You mean nesting ToggleButton builders into a ToggleGroup builder?

[edvin]

The main thing is that it's easy to understand, and not much boiler plate. Ofcourse we could save "the current toggle group" in a state variable, but I fear it will feel a bit magical.

[edvin]

I don't know if it's worth having a toggleGroup builder. Hmm.. I could try to mock up an implementation that does magic, and see how it feels though.

[thomasnield]

It certainly would be convenient if there's an elegant way to do it. My guess is it will require creating an extension class or something. But maybe we can use a fake Pane pattern, but I tried that and got a little dizzy.

I kind of liked your earlier example with the ToggleGroup being passed as an optional second argument.

[ruckustboom]

I've got Hyperlink and PasswordField done, as well as the non-magic ToggleButton and RadioButton. I'll submit a PR, and magic ToggleGroup stuff can be done later. :)

PR: #78

[edvin]

Very nice @t-boom ! I'll try one more thing with ToggleButton, hang on.

[ruckustboom]

Oops, didn't see your message soon enough :)

[ruckustboom]

I can pull the radiobutton and togglebutton code out of the PR

[edvin]

I think I have an idea, kind of what @t-boom already suggested:

init {
    with(root) {
        // Create a new togglegroup and assign it to root
        togglegroup() 
        // The togglebutton builder function checks if the node it's added to
        // is associated with a togglegroup, and adds it to that group
        togglebutton("Alice") 

        hbox {
            // Add another togglegroup
            togglegroup()
            // Buttons in the hbox will be added to the second group
            togglebutton("Bob")
        }
    }
}

The parent node could populate properties["tornadofx.togglegroup"] with the actual togglegroup. Then we could also add features to Node like hasToggleGroup and getToggleGroup() etc.

I can't see type safety becoming an issue with this approach :)

What do you guys think? (Not saying this is the optimal solution though)

[thomasnield]

That is an intriguing idea. I can't think of a way it would backfire so lets go with it. The only drawback I can think of is there is no clear structural relationship between the toggle buttons and the toggle group syntactically, but with some documentation that can quickly be overcome.

[ruckustboom]

Edge case: What if we want to not use a toggle group after one is created (such as Charlie below)?

init {
    with(root) {
        // Create a new togglegroup and assign it to root
        togglegroup() 
        // The togglebutton builder function checks if the node it's added to
        // is associated with a togglegroup, and adds it to that group
        togglebutton("Alice") 

        // I don't want this button in the toggle group
        togglebutton("Charlie")

        hbox {
            // Add another togglegroup
            togglegroup()
            // Buttons in the hbox will be added to the second group
            togglebutton("Bob")
        }
    }
}

Another thing to worry about is trying to add to an _earlier_ toggle group after one has been created

init {
    with(root) {
        // Create a new togglegroup and assign it to root
        togglegroup() // <- This is Fred
        // The togglebutton builder function checks if the node it's added to
        // is associated with a togglegroup, and adds it to that group
        togglebutton("Alice") 
        // Start a new toggle group
        togglegroup()
        // The togglebutton builder function checks if the node it's added to
        // is associated with a togglegroup, and adds it to that group
        togglebutton("Charlie") 

        // I want to add this one to Fred
        togglebutton("Dave")

        hbox {
            // Add another togglegroup
            togglegroup()
            // Buttons in the hbox will be added to the second group
            togglebutton("Bob")
        }
    }
}

[thomasnield]

In a way it kind of turns a Pane into a ToggleGroup which I like : )

[thomasnield]

Does the toggle group associate with its parent?

[edvin]

Saving private Charlie:

togglebutton("Charlie", null)

Marry Dave and Fred <3:

val fred = togglegroup()
...
togglebutton("Dave", fred)

[ruckustboom]

Ah, simple indeed. I like it :)

[edvin]

Cool! Then I think we have a winner. You might as well add it to the PR before I merge? :)

[thomasnield]

I agree looks good.

[edvin]

I think you can do something like this to solve the togglegroup assignment:

fun Node.getToggleGroup(): ToggleGroup? = properties["tornadofx.togglegroup"] as ToggleGroup?

fun Pane.togglebutton(text: String = "", group: ToggleGroup? = getToggleGroup(), op: (ToggleButton.() -> Unit)? = null) {
    ...
}

So, default will be to lookup the group in the current Pane, and a passed null value will override that.

[edvin]

This was a good talk, I think we landed on a nice solution. Thanks, guys! :)

[ruckustboom]

Sorry, someone else will have to. Something came up and I have to go (most likely for the rest of the day). My availability this weekend will be sporadic at best, so I don't know if I'll be around to do much more till next week :frowning:

[edvin]

No worries, @t-boom :) I'm quite busy this weekend myself, and probably Thomas as well, so either you'll have a look next week, or I can merge and do the rest then if you're still busy. Have a nice weekend!

[thomasnield]

I'll see if I can make time to handle the PR in the next day or two. Thanks for getting involved @t-boom. We're glad to have you onboard.

[thomasnield]

Okay I merged and made those tweaks to @t-boom's work. Updated list at top of this thread: 5 down, 7 to go.

[thomasnield]

I also added TextFlow and Text builders.

class MyApp: SingleViewApp() {
    override val root = VBox()

    init {
        with(root) {
            text("A TextFlow Demo")

            textflow {
                text("Hello") {
                    fill = Color.RED
                }
                text(" World") {
                    fill = Color.BLUE
                    font = Font.font(20.0)
                }
            }
        }
    }
}

[edvin]

Fantastic work!

[ruckustboom]

Just to keep information together: slider and separator are merged in now, and I have a PR for htmleditor.

[thomasnield]

Excellent, just crossed those off and I'll follow up with HTMLEditor shortly. Thanks again for spending time on this.

[ruckustboom]

HTMLEditor is now merged as well.

[ruckustboom]

@thomasnield My pleasure. This is an awesome project.

[edvin]

I'm adding chooseFile in Dialogs.kt since the FileChooser is not a UI component. Docs and usage:

Ask the user to select one or more files from a file chooser dialog. The mode will dictate how the dialog works, by allowing single selection, multi selection or save functionality. The file dialog will only allow files that match one of the given ExtensionFilters.

This function blocks until the user has made a selection, and can optionally block a specified owner Window.

If the user cancels, the returned file list will be empty.

// Select multiple files
val filters = arrayOf(FileChooser.ExtensionFilter("Text files", "*.txt"))
val files: List<File> = chooseFile("Select some text files", filters, FileChooserMode.Multi)
println("The user chose $files")

// Present save dialog
val files: List<File> = chooseFile("Save document", filters, FileChooserMode.Save)
if (files.isNotEmpty()) save(files.first())

[edvin]

Added colorpicker builder with optional initial color an enum to select mode.

colorpicker()

colorpicker(Color.MAROON)

colorpicker(Color.MAROON, ColorPickerMode.MenuButton) {

colorpicker(Color.MAROON, ColorPickerMode.MenuButton) {
    customColors.addAll(Color.BLANCHEDALMOND, Color.AQUAMARINE)
}

[edvin]

I think that's it :) Any comments, or should we close this issue?

[thomasnield]

Nice!!!