Skip to content

Commit

Permalink
add doc to explain why we're opt-ing out of iO instrumentation, or no…
Browse files Browse the repository at this point in the history
…t instrumenting
  • Loading branch information
pixelzoom committed Mar 11, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent d4e5827 commit c0ce1c1
Showing 4 changed files with 13 additions and 6 deletions.
6 changes: 3 additions & 3 deletions js/common/view/CurveManipulationIconNode.ts
Original file line number Diff line number Diff line change
@@ -26,7 +26,7 @@ const CHART_TRANSFORM_OPTIONS = {
const TRANSFORMED_CURVE_OPTIONS = {
numberOfPoints: 70,
xRange: CalculusGrapherConstants.CURVE_X_RANGE,
tandem: Tandem.OPT_OUT
tandem: Tandem.OPT_OUT // curves for icons are not instrumented
};

export default class CurveManipulationIconNode extends Node {
@@ -86,7 +86,7 @@ export default class CurveManipulationIconNode extends Node {
// Create the solid curve node.
const solidCurveNode = new CurveNode( solidCurve, chartTransform, {
stroke: stroke,
tandem: Tandem.OPT_OUT
tandem: Tandem.OPT_OUT // CurveNodes for icons are not instrumented
} );

const children = [ chartRectangle, solidCurveNode ];
@@ -99,7 +99,7 @@ export default class CurveManipulationIconNode extends Node {
lineDash: [ 4.5, 2 ],
lineWidth: 1
},
tandem: Tandem.OPT_OUT
tandem: Tandem.OPT_OUT // CurveNodes for icons are not instrumented
} );
children.push( dashedCurveNode );
}
2 changes: 2 additions & 0 deletions js/common/view/GraphsNode.ts
Original file line number Diff line number Diff line change
@@ -117,6 +117,8 @@ export default class GraphsNode extends Node {

this.graphSetNode = new Node();

// graphSetsAnimator is only PhET-iO instrumented if we have more than one GraphSet. If there is only one
// graphSet, then it exists to put that GraphSet in its correct position, and then it never changes.
this.graphSetsAnimator = new GraphSetsAnimator(
( model.graphSets.length > 1 ) ? options.tandem.createTandem( 'graphSetsAnimator' ) : Tandem.OPT_OUT );

6 changes: 4 additions & 2 deletions js/common/view/LabeledLinesNode.ts
Original file line number Diff line number Diff line change
@@ -38,8 +38,10 @@ export default class LabeledLinesNode extends Node {
phetioVisiblePropertyInstrumented: false
}, providedOptions );

// LabeledLineNode instances.
const labeledLineNodes = labeledLines.map( labeledLine => new LabeledLineNode( labeledLine, chartTransform, options.labeledLineOptions ) );
// LabeledLineNode instances are not instrumented. The entire PhET-iO API lives in the model.
// See model.tools.labeledLines and https://github.com/phetsims/calculus-grapher/issues/198
const labeledLineNodes = labeledLines.map( labeledLine =>
new LabeledLineNode( labeledLine, chartTransform, options.labeledLineOptions ) );

options.children = labeledLineNodes;

5 changes: 4 additions & 1 deletion js/common/view/LabeledPointsNode.ts
Original file line number Diff line number Diff line change
@@ -29,7 +29,10 @@ export default class LabeledPointsNode extends Node {
// LabeledPointNode instances
const labeledPointNodes = labeledPoints.map( labeledPoint =>
new LabeledPointNode( labeledPoint, chartTransform, predictEnabledProperty, curveLayerVisibleProperty, {
tandem: Tandem.OPT_OUT // see https://github.com/phetsims/calculus-grapher/issues/198

// LabeledPointNode instances are not instrumented. The entire PhET-iO API lives in the model.
// See model.tools.labeledPoints and https://github.com/phetsims/calculus-grapher/issues/198
tandem: Tandem.OPT_OUT
} ) );

super( {

1 comment on commit c0ce1c1

@pixelzoom
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.