On Sat, 13 Jul 2024 10:00:31 GMT, Markus Mack <mm...@openjdk.org> wrote:
>> This PR is a fix for another IOOBE that I discovered while working on #1476. >> >> The PR simplifies the code for adding a series that already contains data by >> adding the data points one-by-one. >> As far as I can see no attempt was previously made to optimize the bulk >> operation except for some trivial O(1) operations, so this should have no >> noticable performance impact. >> >> Accidentally this fixes another bug related to the missing "negative" style >> class when negative data values are added. >> >> Also, the PR aligns the handling of duplicate categories with the behavior >> clarified in #1476, when there are duplicates in the data that was already >> in the series before the series was added to the chart. >> >> Note a change was made to the createTestSeries() test method, letting it >> start at index 1, avoiding the duplicate data items resulting from >> multiplying by 0. >> Without this change `testSeriesRemoveAnimatedStyleClasses` would fail >> because it counts the number of plot children, where the duplicates are now >> removed. > > Markus Mack has updated the pull request incrementally with one additional > commit since the last revision: > > fix "negative" style class when series is changed modules/javafx.controls/src/test/java/test/javafx/scene/chart/BarChartTest.java line 305: > 303: BarChart<String, Number> chart = new BarChart<>(new > CategoryAxis(), new NumberAxis()); > 304: XYChart.Series<String, Number> series = new XYChart.Series<>(); > 305: chart.getData().add(series); speaking of negative values - could we check **all** the possible ways where the 'negative' class is added? or removed? I think it might be possible to add a check to the exiting tests, to reuse the setup. Lastly, I think we also need to test the case when the data point becomes non-negative after explicit XYChart.Data.setValue(), do you think? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1488#discussion_r1678109067
