API improvement for GraphNode

[pixelzoom]

Related to #262 (Self code review)

There's a part of the GraphNode API that I keep bumping into, and it feels unnecessarily complicated. It's GraphNodeOptions.createCurveNode.

The default value in GraphNode.ts is:

      createCurveNode: ( chartTransform: ChartTransform, chartRectangleFill: TPaint, plotBounds: Bounds2 ) => new CurveNode( curve, chartTransform, {
        plotBoundsMethod: CalculusGrapherConstants.PLOT_BOUNDS_METHOD, // see https://github.com/phetsims/calculus-grapher/issues/210
        plotBounds: plotBounds, // see https://github.com/phetsims/calculus-grapher/issues/259
        stroke: graphType.strokeProperty,
        discontinuousPointsScatterPlotOptions: {
          fill: chartRectangleFill
        },
        tandem: providedOptions.tandem.createTandem( 'curveNode' )
      } ),

And the override in OriginalGraphNode.ts is:

   // Creates PhET-iO element 'graphsNode.originalCurveNode'
    const originalCurveNodeTandem = providedOptions.tandem.createTandem( 'originalCurveNode' );
    const createOriginalCurveNode = ( chartTransform: ChartTransform, chartRectangleFill: TPaint, plotBounds: Bounds2 ) =>
      new TransformedCurveNode( originalCurve, curveManipulationProperties, chartTransform, {
        // ... 22 lines of code
      } );

    const options = optionize<OriginalGraphNodeOptions, SelfOptions, GraphNodeOptions>()( {

      // GraphNodeOptions
      createCurveNode: createOriginalCurveNode,

And then this cast is needed in OriginalGraphNode.ts, which requires disabling ESlint:

    // We need to know that this.curveNode is of type TransformedCurveNode, as created by createCurveNode above.
    const originalCurveNode = this.curveNode as TransformedCurveNode;
    assert && assert( originalCurveNode instanceof TransformedCurveNode ); // eslint-disable-line no-simple-type-checking-assertions

I think it would be cleaner to:

• replace GraphNodeOptions.createCurveNode with a flag (like hasDefaultCurveNode: boolean) that tells GraphNode whether to create a default CurveNode instance
• Add field this.originalCurveNode to OriginalGraphNode.ts and instantiate it directly

This I shall investigate.

[pixelzoom]

Summary of the important bits in the above commit:

• Deleted GraphNodeOptions.createCurveNode
• Added GraphNodeOptions.hasDefaultCurveNode which determines whether GraphNode creates a non-interactive CurveNode
• Set hasDefaultCurveNode: false for OriginalGraphNode, since we'll be creating our own interactive TransformedCurveNode
• Added field originalCurveNode to OriginalGraph, and instantiated it directly: this.originalGraphNode = new TransformedCurveNode(...).
• Removed the aforementioned cast and // eslint-disable-line.

This looks much nicer to me. And I can sleep better without the // eslint-disable-line :)

@veillette have a look, let me know if you have questions/concerns, or feel free to close.

PS: I confirmed that Screenshot is still working correctly after this change.

[veillette]

I reviewed this. Our overriding of CurveNode was always a bit awkward. Much better. Cl;osing