Fix hover error in appendArrayPointValue #1808
Conversation
|
I don't see the need to test appendArrayPointValue im isolation. We should instead test "real" hover by mocking mouse events or Fx.hover. Moreover, I think it would be best to make _arrayAttrs be always defined for every trace type. When no array attributes are present, arrayAttrs should be |
|
@rreusser have you tried adding any tests for this? |
|
@etpinard Yup. Pushing. |
|
I added it to the |
| low: [0, 0], | ||
| close: [3, 3], | ||
| x: [1, 2, 3], | ||
| y: [0.12345, 0.23456, 0.34567] |
There was a problem hiding this comment.
Oops. Does y do anything for ohlc? This looks like somehow leftover from another test, though it does trigger and respond to the fixed bug correctly.
There was a problem hiding this comment.
y isn't a valid attribute. 🔪
| // See: https://github.com/plotly/plotly.js/issues/1807 | ||
| it('should not fail in appendArrayPointValue', function() { | ||
| Plotly.plot(this.gd, data); | ||
| mouseEvent('mousemove', 203, 213); |
There was a problem hiding this comment.
Maybe assert that the hover label is showing e.g. by
expect(d3.select('.hovertext').size()).toBe(1);There was a problem hiding this comment.
Good call. Added ✅
|
💃 not a perfect solution (see #1807 (comment)) but good enough for now and, more importantly, good enough for |
This PR address the issue in #1807 by exiting early if
arrayAttrsis undefined. This may or may not be an appropriate fix. I could test this by passing it a plain object with no _arrayAttrs, but it seems to be of questionable value sinceappendArrayPointValueisn't tested otherwise. Can add, but I'll wait for a second opinion on the source of the problem.