Esri / cedar

JavaScript Charts for ArcGIS
https://esri.github.io/cedar
256 stars 238 forks source link

v1 events #282

Open tomwayson opened 7 years ago

tomwayson commented 7 years ago

support same events as v0?

how to implement?

ajturner commented 7 years ago

I think the v0 API is good to start.

chart.on('mouseover', function(event,data){ });
chart.on('mouseout', function(event,data){ });
chart.on('click', function(event,data){ });

Questions

tomwayson commented 6 years ago

AmCharts has a rich API that can easily accommodate the above:

https://docs.amcharts.com/3/javascriptcharts/AmSerialChart#events https://docs.amcharts.com/3/javascriptcharts/AmPieChart#events https://docs.amcharts.com/3/javascriptcharts/AmXYChart#events

https://www.amcharts.com/kbase/?s=events&kbc=javascript-charts

Looks like our on() would call addListener() and our off() would call removeListener().

tomwayson commented 6 years ago

The challenge is going to be that we will expose the .on() method on the cedar Chart() instance, but that currently doesn't have a reference to the underlying AmChart() instance to call addListener() on. That's easy enough to solve, just change this line to: this._chart = cedarAmCharts(this._container, this.definition(), this.data()).

The cedar chart instance needs to persist the amChart instance, but it should defer to cedar-amcharts to actually add/remove events, so:

// Chart.ts
import { on, off } from '@esri/cedar-amcharts'

export default class Chart {
  public on (eventName, callback) {
    on(this._chart, eventName, callback)
  }
}

However, the real challenge is that this._chart won't exist until render() is called for the first time, so users would not be able to add events before drawing the chart (a very common workflow). One way to handle this would be to manage an internal queue of events to apply once the chart is created.

export default class Chart {
  public on (eventName, callback) {
    if (this._chart) {
      on(this._chart, eventName, callback)
    } else {
      this._pendingListeners.push({name, callback})
    }
  }
  public render() {
    this._chart = cedarAmCharts(this._container, this.definition(), this.data(), this._pendingListeners)
    this._pendingListeners = [];
  }
}

Then in cedarAmCharts() we would apply the events when creating the chart, maybe even by adding listeners to the chart config. Of course someone could conceivably call .off() before the chart is rendered, so that would have to remove the item from _pendingListeners.

I think persisting this._chart raises larger questions about whether or not we should be creating a new chart every time render() is called, but I think we can defer those questions for a later time.