ChartZoomManager is missing a MouseEvent.MOUSE_CLICKED handler

[TomSchorsch]

I extended the fxutils in my own application so that a double click would restore the original zooming (turn it all off).

To do this I needed a MOUSE_CLICKED handler for the ChartZoomManager for the rectangle.

Unfortunately the eventHandler is private so I could not merely subclass and add it in.

ChartZoomManager needs:

    handlerManager.addEventHandler( false, MouseEvent.MOUSE_CLICKED, new EventHandler<MouseEvent>() {
        @Override
        public void handle( MouseEvent mouseEvent ) {
            //Don't check filter here, we're either already started, or not
            onMouseClick(mouseEvent);
        }
    } );

or the eventHandler needs to be made protected (or other options).

[gillius]

It has been a long time since I've worked on this code...

So using just the code you've provided, there's no onMouseClick method so you are proposing one is added with protected? That wouldn't be consistent with the other onMouse* methods which are private.

The event handler is just attached to the chartPane, which is passed into the ChartZoomManager anyway. Does it work to just simply register an event handler on the Pane passed into the zoom manager, for any events you want to handle? Then if you detect a double click you can just call setAutoRanging(true) on the axes?

This is all based on the fact that it seems that just adding your own event listener to the pane/chart lets you do that, since your proposed code doesn't modify or disable any other event listeners on its own.

[TomSchorsch]

Jason, Thanks for the quick reply. Sorry for my confusing bug-report.

So, I really proposed two different fixes. (1) add in the code I sent (in a manner consistent with the other handlers i.e. private) or (2) make "eventHandler" protected so someone that subclasses ChartZoomManager can add their own handlers.

I wanted to leave your code base alone and just do extensions to it. I found that I can easily copy your entire ChartZoomManager and make any needed changes to my version. So I am not in an impasse coding-wise. I have worked around the issue myself. The bug-report was me being a good citizen and reporting a possible change/fix.

To further explain, I was trying to extend/subclass your ChartZoomManager and do appropriate things to my subclass (and with a lot of Zoom related handlers in general), but I could not in this instance because "eventHandler" was private in ChartZoomManager. Making eventHandler protected would allow me to access it in my subclass. That was my 2nd fix suggestion (and the real one).

As you suggested in your email, I could have by-passed the ChartZoomManager event-handler completely as a different option.

Anyway, I want you to know I am grateful for your code-base.

Purely as an FYI, I have made additional "improvements" starting with your code base.

(1) I had a need for a Seconds Since Midnight Axis which given a double would display it on the tics as HH:MM:SS.sss (or sub-variations there-of depending on the zoom level). I tried sub-classing StableTicsAxis to create StableTicsSSMAxis but ended up having to copy it wholly and making the needed changes because of one small change that I needed. (The private function getRange needed to send the high, low,and TickSpacing to the AxisTicFormatter whenever it changed so my SSM formatter could react differently based on the zoom level). So again,sub-classing wouldn't work.

(2) The ZoomManager restore you already know about. I think the only zoom out mechanism available was using a scroll mouse. I couldn't figure out any UI controls to reset the chart after zooming or to zoom out in general other than with the scroll mouse. I use the double click as a 10% zoom out (unless one of the control keys is pressed, then it is a restore chart). Also a reverse drag on the chart (from lower-right to upper left) is a restore chart as well. I think I do this all in my version of the ChartZoomManager.

(3) Finally, I didn't like that zooming on the X or Y axis you could only drag in one direction. I had trouble zooming into the lower left corner and wanted to be able to drag to the left on the X axis as well as to the right to be able to easily zoom into that corner. In my version both the X and Y axis can be dragged (and thus zoomed) in either direction but the chart is still limited to zooming only with a top-left to bottom-right drag. I think this was also done in my version of the ChartZoomManager.

All of these changes/additions were (relatively) easy using your code-base as a starting point. When I went down the route of replacing your ChartZoomManager (versus extending it) I stopped looking at what would be possible in an extension and just made the changes directly to my version of ChartZoomManager. Finally, once I decided to put my own code on GitHub I decided to clean things up and make suggested changes to the code-base of others that I relied on - hence the bug-report.

Thanks again, Tom Schorsch

On Thu, May 14, 2020 at 9:12 PM Jason Winnebeck notifications@github.com wrote:

It has been a long time since I've worked on this code...

So using just the code you've provided, there's no onMouseClick method so you are proposing one is added with protected? That wouldn't be consistent with the other onMouse* methods which are private.

The event handler is just attached to the chartPane, which is passed into the ChartZoomManager anyway. Does it work to just simply register an event handler on the Pane passed into the zoom manager, for any events you want to handle? Then if you detect a double click you can just call setAutoRanging(true) on the axes?

This is all based on the fact that it seems that just adding your own event listener to the pane/chart lets you do that, since your proposed code doesn't modify or disable any other event listeners on its own.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gillius/jfxutils/issues/22#issuecomment-629001513, or unsubscribe https://github.com/notifications/unsubscribe-auth/APO5J6XVSC5YP6UUUOZKS6TRRSXLRANCNFSM4NAXCOUA .