CanvasLinePlot's lack of support for color Property

[pixelzoom]

CanvasLinePlot does not support {Property} stroke because any change to CanvasLinePlot requires calling update for the parent ChartCanvasNode.

In practice, this is causing 2 problems:

(1) I'm constantly forgetting that stroke can't be a Property, and writing code like this that fails an assertion in CanvasLinePlot setStroke:

    const sumPlot = new CanvasLinePlot( this.chartTransform, [], {
      stroke: FMWColors.sumPlotStrokeProperty
    } );

(2) To use a color Property (e.g. a ProfileColorProperty) requires boilerplate like this, which occurs 3x in Fourier:

    const somePlot = new CanvasLinePlot( this.chartTransform, [], {
      stroke: someStrokeProperty.value
    } );

    const someChartCanvasNode = new ChartCanvasNode( this.chartTransform, [ somePlot ] );

    // CanvasLinePlot stroke does not support Property, so handle updates here.
    someStrokeProperty.link( stroke => {
      somePlot.setStroke( stroke );
      someChartCanvasNode.update();
    } );

I don't know how to resolve this, and maybe we don't. But I thought I'd create this issue, and do a little brainstorming, since this API is proving to be a little wonky in practice.

@samreid thoughts?

[samreid]

Brainstorming a proposal:

1. Change CanvasLinePlot to accept stroke: ColorDef
2. Add a changedEmitter property to CanvasPainter
3. When a CanvasPainter is registered on a ChartCanvasNode, the ChartCanvasNode listens for emits from the changedEmitter and automatically repaints itself.
4. A stroke change in CanvasLinePlot triggers CanvasPainter.changedEmitter.emit

[pixelzoom]

I've built something similar to that proposal into HarmonicPlot and InfiniteHarmonicPlot. Basically the same pattern as used in ChartTransform.

[pixelzoom]

I started to implement the proposal to add a changedEmitter to CanvasLinePlot. CanvasLinePlot got much more complicate. It was necessary to conditionally add/remove a listener if stroke was a Property. It was then necessary to implement dispose. And it also seemed like we'd need to add notification for changing lineWidth and dataSet -- making those fields private, and adding setters/getters. Anyway... I bailed, and I guess I'll continue to use the current API.

Closing.

[pixelzoom]

For #47... This issue was raised by @jonathanolson in the code-review for Fourier (phetsims/fourier-making-waves#165. And I (unsuccessfully) tried to handle it in sim-specific code in phetsims/fourier-making-waves#174. So it seems appropriate to reopen this issue - support for stroke Property is clearly something that CanvasLinePlot should have.

[pixelzoom]

Unassigning because I doubt that we'll see progress on this for awhile. Let me know when you'd like to discuss.