Include values of all array attributes in hover/click/select event data

[etpinard]

I was about to just add ids and customdata values in the hover/click/select event data when I realised wouldn't it be nice to include values of all array attributes? We already need to keep track of those attributes for transforms and restyle. That is, calls to PlotSchema.findArrayAttributes aren't going anywhere. So, might as well cache that list and use it during picking/selecting!

For example,

Plotly.newPlot('graph', [{
  y: [1, 2, 1],
  ids: ['a', 'b', 'c'],
  customdata: [{sup: 'hello'}, 30, 'lol'],
  marker: {
    size: [20, 10, 30]
  }
}])
.then(gd => {
  gd.on('plotly_hover', d => console.log(d.points[0]))
})

then hovering on (x=1,y=2) gives:

{
  // as before
  x: 1,
  y: 2,
  data: /**/,
  fullData: /**/,
  xaxis: /**/,
  yaxis: /**/,
  curverNumber: 0,
  pointNumber: 1,

  // new
  id: 'c',
  customdata: 30,
  'marker.size', 10
}

@alexcjohnson @rreusser @monfera Thoughts?

[monfera]

@etpinard I'm strongly in favor of doing as you propose. This is in part because events then convey pertinent information, and the API is simpler for the user. Also, the current practice of relying on attached properties on the div object has several issues, e.g.

• forces us, and the library user, to make and have a container DOM element (there's no fundamental need otherwise; React.js just relaxed an analogous need)
• binds (commits) the API to some implementation detail, which is that
  • the data is linked to the DOM element
  • the data linked to the DOM element locks in some specific format that may constrain future implementation changes
• not a sustainable practice once we have numerous plots on one single WebGL context, and by extension, one <canvas> element
• gives an underdefined boundary for the user where even the contours of the API are hard to pin down (e.g. which properties are fair play to read from calcdata)
• hard to change to a more efficient representation in the future: a DOM property may be read any time, i.e. it must be eagerly updated; while, in case of event supplied data, the event data may be assembled only on demand, e.g. derived from a more efficient internal representation
• currently, tests must be written against calcdata and other internals rather than e.g. event data, which is an obstacle for refactoring

[monfera]

⬆️ my comments may be more pertinent to the plotly_select event, as the event data on it currently includes

{
  curveNumber: 0,
  id: 'myId'
  pointNumber: 15,
  x: 0.56429807,
  y: 24793
}

but doesn't include data, fulldata, calcdata etc. which therefore have to be queried as graphDiv element properties.

[alexcjohnson]

Yeah totally, this will make a lot of applications cleaner 🌟
Just be careful about 🐎 since this will apply to some pretty high-rate events.

[alexcjohnson]

possibly move this to calc since we do that whenever arrays change?

[etpinard]

Good call!

[etpinard]

done in ee1adf8

[etpinard]

@monfera good call about #1770 (comment) - see patch in 3399a58

[etpinard]

Tagging this PR as reviewable.

[alexcjohnson]

not blocking, but if we find that this starts dragging on performance (particularly for selecting I guess) then I suspect nestedProperty would be the primary cost. Could possibly store these instead of attribute strings in _arrayAttrs? It would be a little weird, since they could be referencing data arrays inside an outdated trace if we've redrawn without a recalc, but in as far as recalc is always triggered by changing an array attribute it seems like it would be ok?

[etpinard]

Good call. I didn't notice any serious slow downs so far, so I won't address this in this PR.

[alexcjohnson]

This is great @etpinard - really thorough, nice to get all these harmonized with each other! 💃