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

Chart builders with data parameter for all charttypes #1132

Open
HoldYourWaffle opened this issue Dec 9, 2019 · 5 comments · May be fixed by #1145
Open

Chart builders with data parameter for all charttypes #1132

HoldYourWaffle opened this issue Dec 9, 2019 · 5 comments · May be fixed by #1145

Comments

@HoldYourWaffle
Copy link
Contributor

Is there a reason why only the piechart builder has a data parameter?

tornadofx/src/main/java/tornadofx/Charts.kt

Lines 7 to 14 in e63f086

/**
* Create a PieChart with optional title data and add to the parent pane. The optional op will be performed on the new instance.
*/
fun EventTarget.piechart(title: String? = null, data: ObservableList<PieChart.Data>? = null, op: PieChart.() -> Unit = {}): PieChart {
val chart = if (data != null) PieChart(data) else PieChart()
chart.title = title
return opcr(this, chart, op)
}

tornadofx/src/main/java/tornadofx/Charts.kt

Lines 33 to 37 in e63f086

/**
* Create a LineChart with optional title, axis and add to the parent pane. The optional op will be performed on the new instance.
*/
fun <X, Y> EventTarget.linechart(title: String? = null, x: Axis<X>, y: Axis<Y>, op: LineChart<X, Y>.() -> Unit = {}) =
LineChart<X, Y>(x, y).attachTo(this, op) { it.title = title }

@edvin
Copy link
Owner

edvin commented Dec 10, 2019

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
Copy link
Contributor Author

HoldYourWaffle commented Jan 1, 2020 •
edited
Loading

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
Copy link
Owner

edvin commented Jan 2, 2020

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

HoldYourWaffle added a commit to HoldYourWaffle2/tornadofx that referenced this issue Jan 2, 2020
@HoldYourWaffle
Add convenient data parameter to chart builders
cbf2459 
Fixes edvin#1132
@HoldYourWaffle HoldYourWaffle linked a pull request Jan 2, 2020 that will close this issue
Open
@ctadlock
Copy link
Collaborator

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

@HoldYourWaffle HoldYourWaffle mentioned this issue May 1, 2020
Open
@HoldYourWaffle
Copy link
Contributor Author

@ctadlock Here you go

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add convenient data parameter to chart builders HoldYourWaffle2/tornadofx
3 participants
@edvin @ctadlock @HoldYourWaffle