-
Notifications
You must be signed in to change notification settings - Fork 101
Bugfix/705 zoomer plugin issues with inverted axes #706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Bugfix/705 zoomer plugin issues with inverted axes #706
Conversation
And do not reset inverted setting of x axis when adding the chart.
The cached offset was overwritten by a possibly older method to calculate it which was not usable on the first run of calculations.
| logScaleLengthInv = axisLength / logScaleLength; | ||
| offset = axisLength; | ||
|
|
||
| offset = isVerticalAxis ? getHeight() : getWidth(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a left over of the bigger changes lately. axisLength from line 606 is already populated during initial rendering while getWidth() seems to be zero and causes the inverted axis to be not placed properly.
| setPrefWidth(-1); | ||
| setMaxWidth(Double.MAX_VALUE); | ||
|
|
||
| xAxis.invertAxisProperty().bindBidirectional(invertedSlide); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This did set xAxis.invertAxisProperty() to false for an axes that was already inverted before adding the zoomer plugin (invertedSlide is initially false).
Bidirectional binding was not required here IMO.
| chart6.getFirstAxis(Orientation.VERTICAL).invertAxis(true); | ||
| final Zoomer zoomer6 = new Zoomer(); | ||
| registerZoomerChangeListener(zoomer6, chart6.getTitle()); | ||
| chart6.getPlugins().add(zoomer6); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reproduction according to Jonas' report.
|
|
Any news on this? |
|
Is there an estimate on when you might get to looking at the PR? |



Fixes #705 and related problems.
@wirew0rm could you please review and accept these changes.