search

phetsims / calculus-grapher

"Calculus Grapher" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
4 stars 4 forks source link

`originalCurveNode.inputEnabledProperty` disables `eyeToggleButton` #272

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

@amanda-phet @kathy-phet @veillette FYI.

Related to these 2 issues:

We have a problem with originalCurveNode.inputEnabledProperty. It doesn't just disable manipulation of f(x). It also disables originalGraphNode.eyeToggleButton. To reproduce:

  1. Run the sim in Studio
  2. Set calculusGrapher.derivativeScreen.view.graphsNode.originalGraphNode.originalCurveNode.inputEnabledProperty to false.
  3. Note that you can no longer manipulate f(x), but you also cannot press the eyeToggleButton.

In order to make the Studio tree look a certain way, this has all gotten way too complicated and clever. All of this needs to be rethought and revised. This will also impact how visiblity of cueing arrows is controlled when f(x) manipulation is disabled.

The technical details...

Most of the problem is in OriginalGraphNode. The DragListener that manipulates the curve is (correctly) added to child this.chartRectangle. But we're using this.inputEnabledProperty to disable input input to the entire OriginalGraphNode, not just this.chartRectangle. And eyeToggleButton is also a child of OriginalGraphNode, so its input also gets disabled.

Relevant code in OriginalGraphNode.ts:

    // Allow PhET-iO clients to use originalCurveNode.inputEnabledProperty to enabled/disable interactivity.
    // No not instrument. See https://github.com/phetsims/calculus-grapher/issues/240
    this.inputEnabledProperty = DerivedProperty.or( [ predictEnabledProperty, originalCurveNode.inputEnabledProperty ] );
...
    this.chartRectangle.setInputEnabledProperty( this.curveLayerVisibleProperty ); // see https://github.com/phetsims/calculus-grapher/issues/270
    this.chartRectangle.addInputListener( new DragListener( {
      ...

Relevant code in TransformedCurveNode.ts that uses this.inputEnabledProperty, and will need to be changed for visibility of the cueing arrows:

      // Cueing arrows should not be visible if this node is not interactive.
      // Do not instrument, see https://github.com/phetsims/calculus-grapher/issues/240#issuecomment-1452498549.
      visibleProperty: new DerivedProperty(
        [ transformedCurve.wasManipulatedProperty, options.isInteractiveProperty, this.inputEnabledProperty ],
        ( wasManipulated, isInteractive, inputEnabled ) => !wasManipulated && isInteractive && inputEnabled ),
pixelzoom commented 1 year ago

I fixed this by moving the inputEnabledProperty from OriginalGraphNode to its chartRectangle child. This is the correct place for the inputEnabledProperty, because the DragListener is added to chartRectangle. Here's the derivation in OriginalGraphNode.ts:

    // This allows PhET-iO clients to use originalCurveNode.inputEnabledProperty to enabled/disable interactivity,
    // and prevents manipulation of the curves when they are hidden using the eyeToggleButton.
    // See https://github.com/phetsims/calculus-grapher/issues/240 and https://github.com/phetsims/calculus-grapher/issues/272.
    // Do not instrument.
    this.chartRectangle.setInputEnabledProperty( new DerivedProperty(
      [ originalCurveNode.inputEnabledProperty, predictEnabledProperty, this.curveLayerVisibleProperty ],
      ( originalCurveNodeInputEnabled, predictEnabled, curveLayerVisible ) =>
        ( originalCurveNodeInputEnabled || predictEnabled ) && curveLayerVisible
    ) );

Nothing needed to be changed for cueing arrows in TransformedCurveNode.ts. And there's no PhET-iO API change; originalCurveNode.inputEnabledProperty is still what the PhET-iO client uses to disable f(x) manipulation.

@veillette please have a look, to see if I messed anything up. Close if it looks OK.

veillette commented 1 year ago

I tested it in studio and it works as intended. Closing.