dygraphs icon indicating copy to clipboard operation
dygraphs copied to clipboard

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

Open andnorxor opened this issue 4 years ago • 4 comments

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.

andnorxor avatar May 13 '20 09:05 andnorxor

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.

saraivinha85 avatar Oct 13 '20 08:10 saraivinha85

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?

mirabilos avatar Feb 16 '23 22:02 mirabilos

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.

andnorxor avatar Feb 18 '23 20:02 andnorxor

andnorxor dixit:

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.

Another point can be argued that plugins can decide by themselves what they want to do with values that lie outside of the x or y or both axēs, as their answer may differ.

I’m not deciding anything yet or something, just looking for what options we have.

Zoomed. In this case a is needed for Dygraph draw the line on the left of point B.

Let me zoom in a bit more and draw the mouse pointer.

   ^              /
   |            /
   |          /
   |        B
   |      /
   |    /
   | ↖/
   |/
  /|
A  |

-------|------------------------> time | ==============| visible area

In this case, point A is highlighted, not B, because the mouse pointer is closer to A than to B. This is consistent behaviour within dygraphs, and I played a little with this in the crosshairs demo (since that’s the easy one to exhibit the issue).

If there’s a change in highlighting behaviour, where midway between B and C the highlighted point switches, but you don’t do that midway between A and B even if A itself is not visible then the UI becomes inconsistent and jarring.

If my assumption is correct, the root cause of the issue should be

We’re looking for what the problem (not the symptom) is and then which root to look at, right now.

Dygraph should calculate the 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.

You mean like this:

   ^
   |           C
   |          /
   |        /
   |      /
   |    B
   |  /
   |/
   x
  /|
A  |

-------|------------------------> time | ==============| visible area

Inserting a fake point x and then highlighting this (and switching at the midway point between A and B? x and B?)?

This is something I personally, right now without having thought about it more than three minutes or so, would like to never see: fake data invented.

The legend (and the crosshair) get real data points, and never an interpolated value (modulo rollPeriod).

But, again, let’s collect options and opinions at first. That is assuming I even understood your proposed change correctly…

bye, //mirabilos

„Cool, /usr/share/doc/mksh/examples/uhr.gz ist ja ein Grund, mksh auf jedem System zu installieren.“ -- XTaran auf der OpenRheinRuhr, ganz begeistert (EN: “[…]uhr.gz is a reason to install mksh on every system.”)

mirabilos avatar Feb 18 '23 21:02 mirabilos