Adding Accordion Support

[ruckustboom]

I'm working on functionality to add Accordions. This is as far as I've got, but I can't think of a clean way to modify the "folds" (such as adding a title, changing the padding, etc.).

[ruckustboom]

I had another idea where I just added a Accordion.fold(String?, () -> TitledPane method, but that would have required adding a bunch of methods to create a node the TitlePane content. That may end up being a better way to go.

[ruckustboom]

Here's a demo:

class AccordionTest : View() {
    override val root = StackPane()

    init {
        with(root) {
            prefHeight = 400.0
            prefWidth = 400.0

            accordion {
                folds {
                    textarea { promptText = "Notes" }
                    label("Test 2")
                    label("Test 3")
                }
            }
        }
    }
}

[edvin]

Thanks for this :)

Some questions :) Why did you call the method folds? Is that a name common for accordions? It also seems that you can't set the titles of the folds this way. Wouldn't it be better to do:

accordion {
    fold("Title here") {
        // Content here
    }
}

The content would then be wrapped in a TitledPane with the supplied title. What do you think?

[edvin]

Forget my question about the function name, folds seems to be a good choice/common name for accordions :)

[ruckustboom]

I can change the name, it just popped into my head and I went with it :)

As far as the fold method goes, I mentioned that in the second comment to this PR. The problem is, TitledPane is not a Pane, so you would have to create a large number of methods for TitledPane. That's not a big deal, but TitledPane only takes a single node as content. You can easily do that in your example with something like:

accordion {
    fold("Title here") {
        content = Label("Cool Stuff")
    }
}

but then (unless I'm missing something) you can't continue to use the TornadoFX DSL for its content.

(I just started messing around with Kotlin and TornadoFX this week, so I may easily have missed something. Sorry :) )

[edvin]

Ah, sorry, it's a bit late here and I'm off my game :)

After you have collected the children into a fake VBox, check if there is only one child, and if so add it as the node for the TitledPane. If there are multiple children in the VBox, just add the vbox as the content node of the TitledPane.

[edvin]

accordion {
    fold("Tab 1") {
        label("I will be added as the TitledPane node")
    }
    fold("Tab 2") {
        label("I will be added to the VBox and the VBox will be the TitledPane node")
        label("Me too!")
    }
}

[edvin]

The same pattern is used in the scrollpane builder:

fun Pane.scrollpane(op: (Pane.() -> Unit)) {
    val vbox = VBox()
    op(vbox)
    val scrollPane = ScrollPane()
    scrollPane.content = if (vbox.children.size == 1) vbox.children[0] else vbox
    this += scrollPane
}

[ruckustboom]

I'll do that.

Should I worry about some sort of method to change the pane used? Like an extension property that could be set to VERTICAL, HORIZONTAL, or STACKED (that would resolve to a VBox, HBox, or StackedPane respectively?

I'm not sure how hard that would be to implement, but it popped into my head when I was trying to come up with solutions.

[edvin]

That's not really necessary, because if you want an HBox instead, simply do:

accordion {
    fold("Tab 1") {
        hbox {
            label("Look ma, I'm in an HBox!")
            label("Me too!")
        }
    }
}

[ruckustboom]

Fair point, I didn't think about that :)

[edvin]

Sweet! Make the changes and I'll happily accept the PR :)

[edvin]

If you want you can also add an entry to the CHANGELOG with reference to this issue.

[ruckustboom]

I've added those changes, but there are still some shortcomings. You can't change any settings of the TitledPane without saving it off to a variable (such as padding). Is that acceptable?

[thomasnield]

I haven't looked at it since I'm on my phone. But if the TitledPane is returned by the builder, you can always call apply() afterwards to make those manipulations in a block.

[ruckustboom]

(Compulsory example :) )

class AccordionTest : View() {
    override val root = StackPane()

    init {
        with(root) {
            prefHeight = 400.0
            prefWidth = 400.0

            accordion {
                fold("Tab 1") {
                    label("I will be added as the TitledPane node")
                }
                fold("Settings") {
                    label("Name")
                    textfield { promptText = "Enter a name" }
                    label("Name 2")
                    textfield { promptText = "Enter another name" }
                }
            }
        }
    }
}

[edvin]

You could add another parameter that is a function that operates on the TitledPane. This would have to be added before the last op, something like:

fun Accordion.fold(title: String? = null, titledPaneOp: (TitledPane.() -> Unit)? = null, op: (Pane.() -> Unit)) {
    val vbox = VBox()
    op(vbox)
    val fold = TitledPane(title, if (vbox.children.size == 1) vbox.children[0] else vbox).apply {
        titledPaneOp?.invoke(this)
    }
    panes += fold
}

It won't be as beautiful when you need it though:

fold("Hello", { padding = Insets.EMPTY }) {
    label("Test")
}

EDIT: Just saw @thomasnield 's comment. That might be a better idea, not sure which of the alternatives I like best. What about you guys?

[edvin]

Hmm.. I think Thomas is right. If you return the TitledPane from the fold function the code becomes:

fold("Hello") {
    label("Test")
}.apply {
    padding = Insets.EMPTY
}

[ruckustboom]

I'm leaning towards Thomas's idea as well. I tried it and it seems to work fine.

[edvin]

I gotta get to bed, but if you make that last change and return the TitledPane, I'm sure Thomas can merge this PR for you :))

[thomasnield]

Sounds good, let me know when you want me to merge and ill look.

[ruckustboom]

Goodnight :)

@thomasnield Using your approach works great. For example:

fold("Settings") {
    label("Name")
    textfield { promptText = "Enter a name" }
    label("Name 2")
    textfield { promptText = "Enter another name" }
}.apply {
    this@accordion.expandedPane = this
}

makes the Settings fold expanded. I'll push it up.

[ruckustboom]

It's pushed. Check it out when you get the change. I'll be off for the night in about 30 min, so if you need more changes after that, they'll have to wait till tomorrow :)

[thomasnield]

Merged. Looks good to me. Thanks for putting this together and I'm glad you got involved so quickly!

[thomasnield]

I do think it is good practice for control builders to return the control anyway, not just so apply() can be called for specialized cases like this but also to be able to save the controls to variables.

[edvin]

Absolutely, I'll make sure they all do right away :) That was the initial intent and almost every builder does that, but I found a couple of stragglers! Good catch, Thomas!