Skip to content
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

Events/Legend/crosshair is active for points outside the plot area if the plot is zoomed #997

Open
andnorxor opened this issue May 13, 2020 · 4 comments

Comments

@andnorxor
Copy link

Steps to reproduce:

This is an edge case, but if a dygraph is zoomed in there are invisible points outside the plotArea which still fire events and influence the legend and crosshair behavior.

Those points have negative x coordinates.
image

Actual result
On mouseover those points fire callbacks (ex.: drawHighlightPointCallback) and are visible in the LegendFormatter. Also the crosshair will draw a vertical line outside the plot area.

Expected result
Points outside the plotArea should not fire events regarding the UI.

Possible reason
My assumption is that on a zoom level you don't get a sample at the 0 point of the X-axis but dygraph has to draw a line for this period (0 -> first visible sample), so there's a need for this sample outside the plot area but dygraph does not disable eventHandlers and callbacks for this edge case.

@saraivinha85
Copy link

saraivinha85 commented Oct 13, 2020

Was able to solve this by overriding the mouseMove_ function, like so:

            const that = this.chart
            this.chart.mouseMove_ = (event) => {
                const points = that.layout_.points;
                if (points === undefined || points === null) return;

                const canvasCoords = that.eventToDomCoords(event);
                const canvasx = canvasCoords[0];
                const canvasy = canvasCoords[1];

                const highlightSeriesOpts = that.getOption("highlightSeriesOpts");
                let selectionChanged = false;
                if (highlightSeriesOpts && !that.isSeriesLocked()) {
                    let closest;
                    if (that.getBooleanOption("stackedGraph")) {
                        closest = that.findStackedPoint(canvasx, canvasy);
                    } else {
                        closest = that.findClosestPoint(canvasx, canvasy);
                    }

                    selectionChanged = that.setSelection(closest.row, closest.seriesName);
                } else {
                    const idx = that.findClosestRow(canvasx);
                    if (!that.dateWindow_ || that.rawData_[idx][0] >= that.dateWindow_[0])
                        selectionChanged = that.setSelection(idx);
                }
            }

The trick is this line
if (!that.dateWindow_ || that.rawData_[idx][0] >= that.dateWindow_[0])

that checks if the nearest selection is within the current zoom window.

@mirabilos
Copy link
Collaborator

I think this needs more thought.

I consider it a good thing that once you’re past half the distance that the next point is selected.

The fix is also incomplete. Looking at the crosshair:

  • to the right, the vertical crosshair goes away but the horizontal one renders the next point
  • to the left, both horizontal and vertical crosshairs are rendered
  • didn’t check up/down but I assume this is similar there

The display of the crosshair “outside” is unfortunate… but I don’t think disallowing selection of these points is the solution.

We could catch this in the crosshairs plugin and do various mitigations there:

  • the wood-hammer method, just don’t render the crosshair if the point is not visible (in either axis); this has the chance of throwing the child out with the bathwater
  • on the horizontal axis, drop the vertical crosshair but keep the horizontal one, when the point is outside (viceque versa on the other axis); this mirrors the current “outside to the right” behaviour
  • … possibly others…

Thoughts?

@andnorxor
Copy link
Author

andnorxor commented Feb 18, 2023

IMO this should be fixed in Dygraph and not in the plugin, since it affects all plugins and all code that deals with Dygraph callbacks for the point past the axis.

Also the reason for the point outside the axis is still opaque. My assumption was that ...

[...] on a zoom level you don't get a sample at the 0 point of the X-axis but dygraph has to draw a line for this period (0 -> first visible sample), so there's a need for this sample outside the plot area but dygraph does not disable eventHandlers and callbacks for this edge case.

To illustrate this behavior for a time series, consider the following diagrams:

  • In the first diagram, there is no zoom, and A, B, and C represent points on the time series. A is the first visible sample, and the visible area is marked with an equal sign (=) on the x-axis.
  • In the second diagram, there is a zoom, and point A is needed to draw the line on the left of point B. The visible area is marked with an equal sign on the x-axis, as before.
No zoom.

^
|           C
|          /
|        /
|      B
|    /
|  /
A
|------------------------> time
| =======================| visible area


Zoomed. In this case A is needed for Dygraph draw the line on the left of point B.
       ^
       |           C
       |          /
       |        /    
       |      /      
       |    B   
       |  /   
       |/
      /|
    A  |
-------|------------------------>  time
       | ==============| visible area

If my assumption is correct, the root cause of the issue should be addressed, and Dygraph should calculate the (virtual) point at the 0 value of the X axis and use it solely for drawing a line, without outputting the point past the axis. This would be a more effective solution than adding mitigations on top of the existing behavior.

Naturally, I cannot anticipate all the potential implications of such a fix. However, to condense this: address the root cause of the issue don't add mitigations.

@mirabilos
Copy link
Collaborator

mirabilos commented Feb 18, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants