Skip to content
This repository has been archived by the owner on Sep 28, 2024. It is now read-only.

BorderPane Builder #16

Merged
merged 4 commits into from
Feb 25, 2016
Merged

BorderPane Builder #16

merged 4 commits into from
Feb 25, 2016

Conversation

thomasnield
Copy link
Collaborator

I'm trying to implement BorderPane builders and get something to this effect.

class BorderPaneTest : App() {

    override val primaryView = BorderPaneItem::class

}

class BorderPaneItem() : View() {
    override val root = BorderPane()
    init {
        with(root) {
            top {
                button("Top") {
                    borderpaneConstraints {
                        alignment = Pos.TOP_CENTER
                    }
                }
            }
            bottom {
                button("Bottom") {
                    borderpaneConstraints {
                        alignment = Pos.TOP_LEFT
                    }
                }
            }
            left {
                button("Left") {
                    borderpaneConstraints {
                        alignment = Pos.TOP_LEFT
                    }
                }
            }
            right {
                button("Right") {
                    borderpaneConstraints {
                        alignment = Pos.BOTTOM_RIGHT
                    }
                }
            }
        }
    }
}

I'm getting the "dupe children" issue and I think this has to do with builders adding the elements twice due to a parent inadvertently getting targeted. I think you did some sort of trick with the GridPane builder by using some "fake" Pane.

What do you think?

@thomasnield
Copy link
Collaborator Author

Oh wait, I think I see what you did there. Clever. Let me try this again.

@thomasnield
Copy link
Collaborator Author

Much better! I think I'm getting somewhere. The only limitation is only one node can be passed and the rest will be ignored. We can change that though. Perhaps we can check the count of components provided and handle putting them in a Pane if there is more than one.

@thomasnield
Copy link
Collaborator Author

Okay I changed that. Just committed my rough proposal for handling multiple Node items, by defaulting them to HBox or VBox. It does not really make sense in my option to provide the option for own layout, just because you should be able to nest one within the builder.

@thomasnield
Copy link
Collaborator Author

I'll need to simplify and cleanup how multiple children are handled. Will do tomorrow if we move forward with this.

@edvin
Copy link
Owner

edvin commented Feb 24, 2016

If there is only one children, it's important that it is not wrapped in anything. The use should be able to:

top {
    hbox {
        button("Button 1")
        button("Button 2")
    }
}

This would keep it in line with how the rest of the builders work. If there are multiple children, we could either throw an exception, or wrap in an hbox or something by default. Unsure what's the best approach, try to experiment a little with it :)

You could use an HBox as the fake, and keep it if there are multiple children, and unwrap if there is a single child maybe?

I'm going to be gone for most of the day, but hopefully I'll be able to look at it tonight. (I'm GMT+1)

@edvin
Copy link
Owner

edvin commented Feb 24, 2016

Hmm.. I think VBox is a better default because of the nature of the BorderPane.

@thomasnield
Copy link
Collaborator Author

Pretty much my thoughts too. Will do. And no worries.

@thomasnield
Copy link
Collaborator Author

Will get to this later today or tomorrow. But I just wanted to say I used TornadoFX along with RxJava to re-write and enhance a complex UI. It is phenomenal! What took me a week without RxJava, Kotlin, and TornadoFX, I did in less than an hour! My application is actually halfway done now.

@edvin
Copy link
Owner

edvin commented Feb 24, 2016

Great! I feel the same way. I'm much more productive with this stack, and it makes me go the extra mile, so I add features in my apps that earlier felt like a chore to implement, so I might have skipped them :)

@thomasnield
Copy link
Collaborator Author

You and I have very similar ambitions and goals : )

@edvin
Copy link
Owner

edvin commented Feb 24, 2016

It seems that way. I'm really glad you got involved in TornadoFX :)

@thomasnield
Copy link
Collaborator Author

Me too, I'm happy to contribute. And thanks for starting it. This library definitely shows what Kotlin is capable of, and is probably the most exciting one I found yet.

@thomasnield
Copy link
Collaborator Author

I think everything is implemented here. Anything else need to be done?

@edvin
Copy link
Owner

edvin commented Feb 25, 2016

Sweet! I will review and merge it tonight! Still on vacation so things are going a bit slower than usual.

edvin pushed a commit that referenced this pull request Feb 25, 2016
@edvin edvin merged commit ed45832 into edvin:master Feb 25, 2016
@edvin
Copy link
Owner

edvin commented Feb 25, 2016

Looking good. Nice work, @thomasnield 👍

@thomasnield thomasnield deleted the borderpane_builder branch February 27, 2016 18:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants