edvin / tornadofx

Lightweight JavaFX Framework for Kotlin
Apache License 2.0
3.68k stars 272 forks source link

Chart builders with data parameter for all charttypes #1132

Open HoldYourWaffle opened 4 years ago

HoldYourWaffle commented 4 years ago

Is there a reason why only the piechart builder has a data parameter? https://github.com/edvin/tornadofx/blob/e63f086a5d56b373db0b058b665bf1221732a584/src/main/java/tornadofx/Charts.kt#L7-L14 https://github.com/edvin/tornadofx/blob/e63f086a5d56b373db0b058b665bf1221732a584/src/main/java/tornadofx/Charts.kt#L33-L37

edvin commented 4 years ago

I believe that's because PieChart only has a single dimension, whereas the other charts support multiple dimensions so it's more convenient to generate series of data using the series builder after creating the chart.

HoldYourWaffle commented 4 years ago

That makes sense. However, in my application the Series are generated programmatically too, so I'm directly binding data to an ObservableList<XYChart.Serie> like this (pseudo-code):

val series = observableListOf<XYChart.Serie>();

linechart {
    data.bind(series);
}

Would it make sense to add a convenience constructor parameter for this use-case?

edvin commented 4 years ago

Sure :) It's in line with how the other builders work, so a PR would be welcome :)

ctadlock commented 4 years ago

@HoldYourWaffle I’ll get this merged in. Can you also submit one for the jdk10 branch to make it easy? ;)

HoldYourWaffle commented 4 years ago

@ctadlock Here you go