RFR: 8198830: BarChart: auto-range of CategoryAxis not working on dynamically setting data [v2]
Andy Goryachev
angorya at openjdk.org
Fri Jun 28 16:34:27 UTC 2024
On Fri, 28 Jun 2024 07:39:46 GMT, Markus Mack <mmack at openjdk.org> wrote:
>> This PR fixes the placement of `BarChart` bars and category tick marks, particularly when adding data / multiple series and enabling animations.
>> It covers all cases mentioned in the JBS ticket and adds a unit test.
>>
>> The settable `barGap` and `categoryGap` now should also behave as expected.
>>
>> There's a change regarding consistent placement of bars when there is more than one series:
>> Now, for example, if there are two series S1 and S2, S1 bars will always be on the left and S2 bars will always be on the right side of the tick mark.
>> This means that if some data category (=x-value) is only present in S2, but not S1, the bar will still be drawn on the right. The previous behavior, if the marks weren't off completely, would have put it on the left which I'd say looks inconsistent.
>> There's a test for this situation as well.
>>
>> Note this does not fix [JDK-8334873](https://bugs.openjdk.org/browse/JDK-8334873) where bars get stuck while having different widths. There seem to be additional issues which seem not directly related to the changed code and are probably out of scope for this PR (unless we see some regressions).
>
> Markus Mack has updated the pull request incrementally with two additional commits since the last revision:
>
> - fix indentation
> - update copyright
Thank you @drmarmac for fixing all this bug (and adding unit tests!)
Verified that the bug is fixed, including the two scenarios mentioned in the comments, with animation on and off.
The fix is pretty straightforward, I think one reviewer is sufficient.
modules/javafx.controls/src/main/java/javafx/scene/chart/BarChart.java line 428:
> 426: }
> 427:
> 428: index++;
this is correct. not sure how the existing tests did not find this problem.
-------------
Marked as reviewed by angorya (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/1492#pullrequestreview-2148450033
PR Review Comment: https://git.openjdk.org/jfx/pull/1492#discussion_r1659001879
More information about the openjfx-dev
mailing list