how to handle visibleProperty in view?

[pixelzoom]

Changing the visibility of any Node that has an associated visibility Property should update that Property. Add Event.on( 'visibility', ... ) to all of the things that have view Properties.

[pixelzoom]

Clients should not be changing the visibility of some things (e.g. vertex) via their Node, because it's not based solely on vertexVisibleProperty, it's also based on whether the quadratic describes a parabola and therefore has a vertex. Visibility is (in many cases) a derived Property.

[pixelzoom]

@samreid In VertexManipulator, visibility is derived like this:

      // visibility of this Node
      const visibleProperty = new DerivedProperty(
        [ vertexVisibleProperty, quadraticProperty ],
        ( vertexVisible, quadratic ) =>
          vertexVisible &&  // the Vertex checkbox is checked
          quadratic.vertex !== undefined &&  // the quadratic has a vertex
          graph.contains( quadratic.vertex ) // the vertex is on the graph
      );
      visibleProperty.link( visible => {
        this.interruptSubtreeInput(); // cancel any drag that is in progress
        this.visible = visible;
      } );

vertexManipulator.visibleProperty is currently supplied by NodeIO. For example in Studio:

If I were to instrument VertexManipulator visibleProperty, how would that interact with this visibilityProperty that is provided by NodeIO?

[pixelzoom]

I added (but did not commit) these options to visibleProperty in VertexManipulator above:

      {
        tandem: options.tandem.createTandem( 'visibleProperty' ),
        phetioType: DerivedPropertyIO( BooleanIO ),
        phetioDocumentation: 'whether the vertex manipulator is visible on the graph'
      }

In Studio I now see that *.vertexManipulator.visibleProperty is a read-only DerivedProperty:

So VertexManipulator visibleProperty now seems to be shadowing NodeIO visibleProperty. For manipulators, we don't want clients to change the visibility of Manipulator subclasses directly, so this shadowing could be exploited here. But is this a bug or a feature?

[samreid]

With assertions enabled, the first thing that would happen is an error from the phetio.js indicating the presence of two instances trying to register themselves to the same phetioID. Without assertions enabled, it looks like the last one to be registered takes precedence, but we shouldn't publish a version with assertions failing.

UPDATE: Pulling to take a closer look. UPDATE 2: I'm still stuck on: https://github.com/phetsims/graphing-quadratics/issues/77#issuecomment-431634307 UPDATE 3: I see that the changes aren't pushed, I'll try to check the proposed change locally after getting past https://github.com/phetsims/graphing-quadratics/issues/77#issuecomment-431634307

[pixelzoom]

Have you done pull-all.sh before testing? There are changes in graphing-lines.

[pixelzoom]

Looks like my graphing-lines change didn't get pushed. Wifi challenges while traveling :( Pushed.

[samreid]

Got it, thanks! I'll test the shadowing locally and report back.

[pixelzoom]

So if I want to instrument something in a subclass of Type, I need to choose a different tandem name for it than any tandem in TypeIO? In this case, it's going to cause me to name VertexVisibleProperty's visibleProperty to something very unnatural (reallyVisibleProperty?) and then I'm going to have 2 phetioIDs under *.vertexManipulator that are related to visibility. And I need to do this in all of my Manipulator subtypes. How do we get around this?

[pixelzoom]

... and then I'm going to have 2 phetioIDs under *.vertexManipulator that are related to visibility

One of which the client should definitely not touch.

[pixelzoom]

Perhaps an IO type should only add a Property if it doesn't exist in the type that it's wrapping?

[samreid]

So if I want to instrument something in a subclass of Type, I need to choose a different tandem name for it than any tandem in TypeIO?

Good question, but this is not unique to PhET-iO. For instance, AquaRadioButton.js is hard-coded to create a Circle on line 87 like so:

    this.deselectedCircleButton = new Circle( options.radius, {
      fill: options.deselectedColor,
      stroke: options.stroke
    } );

and there is no way to change that to be some Circle subtype. One strategy that was recently discussed for dealing with this problem is Dependency Injection: https://github.com/phetsims/tasks/issues/952. Another possibility is to pass in a function that creates the component.

[pixelzoom]

The 5 classes where I have a conflict with NodeIO's visiblePropertyare:

AxisOfSymmetryNode DirectrixNode VertexNode VertexManipulatorNode FocusManiplulatorNode

In all of these cases, I have a Node whose visibility should not be controlled via NodeIO.visibleProperty because visibility is derived from both the state of an associated checkbox Property, and the nature of the quadratic (whether or not it describes a parabola.)

[samreid]

It looks like tandem collisions are now only checked when running with?ea&phetioValidateTandems=true . In that case, I see

Assertion failed: Cannot register two different instances to the same id: graphingQuadratics.vertexFormScreen.view.vertexManipulator.visibleProperty

I'm a little surprised to see the collision check buried under phetioValidateTandems, I'm not sure if that is a recent or intentional change.

[pixelzoom]

One strategy that was recently discussed for dealing with this problem is Dependency Injection: phetsims/tasks#952. Another possibility is to pass in a function that creates the component.

I don't see how either of those techniques could naturally be applied to the classes mentioned in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-431635088. Renaming the conflicting tandem would probably be less of a mess.

It also doesn't address the fact that the client should not be using NodeIO visibilityProperty for instances of these classes, but is provided full access to it. But maybe that's a problem that we're punting on under the "all or nothing" PhET-iO model.

[pixelzoom]

Btw... I think this issue demonstrates deep flaws with the "all or nothing" approach that PhET-iO has decided to take. In this case, we have Nodes whose visibility is derived, so there is no question that clients should not be setting visibility of those Nodes directly - doing so will only put the sim in a bad state. I doubt that this sim is unique, I'm sure that other sims will (and do) have Nodes with similar visibility requirements, and I'm sure that these requirements will extend to other Properties. But PhET is going down a path where it's impossible to specify ("cherry pick") which Properties the client can and cannot change - to expose one bit of instrumentation, we have to expose it all. If we can't control access to (and give good information about) visibleProperty (the most fundamental of view Properties), then I don't have a good feeling about the fundamental philosophy of the PhET-iO API.

[samreid]

It seems for this case we either need to: (a) make the visibility property readOnly or (b) make it so when you set the node visibilityProperty, it back-computes the DerivedProperty dependencies and chooses values that result in visible:false

The latter seems unscalable, so it seems we should focus on the former. It seems short-sighted to focus on visibilityProperty/readOnly when there is a full combination of flags we may need to support in general, this snippet from NodeIO.js

    var visibleProperty = new NodeProperty( node, 'visibility', 'visible', {

      // pick the following values from the parent Node
      phetioReadOnly: node.phetioReadOnly,
      phetioState: node.phetioState,
      phetioType: PropertyIO( BooleanIO ),

      tandem: node.tandem.createTandem( 'visibleProperty' ),
      phetioDocumentation: 'Controls whether the Node will be visible (and interactive), see the NodeIO documentation for more details.'
    } );

    var pickableProperty = new NodeProperty( node, 'pickability', 'pickable', {

      // pick the following values from the parent Node
      phetioReadOnly: node.phetioReadOnly,
      phetioState: node.phetioState,

      tandem: node.tandem.createTandem( 'pickableProperty' ),
      phetioType: PropertyIO( NullableIO( BooleanIO ) ),
      phetioDocumentation: 'Sets whether the node will be pickable (and hence interactive), see the NodeIO documentation for more details'
    } );

    // Adapter for the opacity.  Cannot use NodeProperty at the moment because it doesn't handle numeric types
    // properly--we may address this by moving to a mixin pattern.
    var opacityProperty = new NumberProperty( node.opacity, {

      // pick the following values from the parent Node
      phetioReadOnly: node.phetioReadOnly,
      phetioState: node.phetioState,

      tandem: node.tandem.createTandem( 'opacityProperty' ),
      range: { min: 0, max: 1 },
      phetioDocumentation: 'Opacity of the parent NodeIO, between 0 (invisible) and 1 (fully visible)'
    } );
    opacityProperty.link( function( opacity ) { node.opacity = opacity; } );
    node.on( 'opacity', function() { opacityProperty.value = node.opacity; } );

Note that currently the same node.phetioReadOnly is used for visibility, opacity, pickability.

The way things are set up right now, the customization could not be set on Node by setting this.something=... attributes because NodeIO is called from super(). This implies that values would have to be passed to Node through its options or maybe we could implement override methods that are called from NodeIO (I haven't tried this in ES6--not sure if subclass override methods can be called before super completes). Other solutions would change the way IO types are set up (say, inlining them into core types or other heavy-handed changes).

Another workaround would be to set the node as readOnly, then provide another interface for changing its pickability. This sounds broken because one pickability interface would say read-only.

Another possibility would be for the subclass to unregister the registered visibilityProperty and to substitute its own.

Another possibility would be dependency injection--the node subclass could declare the visibilityProperty and NodeIO would only create it if not provided.

Another possibility is to provide Node options like visibilityReadOnly, visibilityDocumentation, etc that override the defaults in NodeIO.

A lot of choices here and we are looking for the one with the best long-term maintainability/scalability characteristics. It's unclear to me which is best.

[pixelzoom]

I have no idea how to resolve this. We could punt and not register visibleProperty for the 5 classes listed in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-431635088. But then the PhET-iO client will have no way to determine if those things are visible on the graph. And this issue will undoubtedly be encountered for other sims.

Assigning to the PhET-iO team to propose a solution. High priority since this blocks publication of this sim.

[pixelzoom]

Assigning to the PhET-iO team to propose a solution.

Yes, I did review @samreid's proposals in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-431670997. But I have no idea how to proceed, or which proposal might be best. So I'm suggesting that those with deeper knowledge of PhET-iO should discuss and propose a solution.

[samreid]

Proposed short term fix, for publishing 1.0: Let's mark those 5 Node subtypes as phetioReadOnly and not register the DerivedProperty visibleProperty instances at all.

Pros:

1. This will be straightforward to implement
2. In the future, if we change pickableProperty to readOnly: false this would not be a breaking API change
3. Client will not be able to put the sim in a buggy state

Cons:

1. Client will not be able to make these 5 types non-pickable.
2. This may not be the long term solution.

@pixelzoom @zepumph @chrisklus sound reasonable?

[pixelzoom]

We'll have to confirm that the proposal in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-432273173 is OK with the Graphing Quadratics design team.

Team (@amanda-phet, @kathy-phet, @ariel-phet): In order to work around a complicated problem with PhET-iO (read previous comments in this issue, if you're interested), the proposal is to make Graphing Quadratic manipulators "read only" for PhET-iO. The client will be able to inspect their state, and the data stream will contain messages related to interactions with the manipulators. But the client will not be able to change any of a manipulator's properties. Most significant is that they will not be able to disable interaction with the manipulators, which we believe is a desirable PhET-iO feature. Depending on how the PhET-iO problem is addressed, we may be able to add this feature in the future. Does anyone object to this change for the 1.0 release?

[ariel-phet]

@pixelzoom I think that is fine to move forward with. We do not even know if a client would desire this ability or not, so it seems reasonable to punt on it for 1.0 publication given the associated complexity.

[pixelzoom]

@zepumph @chrisklus please weigh in on the proposal in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-432273173. Does that sound reasonable to you?

[pixelzoom]

@amanda-phet @kathy-phet please weigh in on https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-432297426. Labeling with meeting:design in case you need to discuss at today's design meeting.

[zepumph]

Yes I think that this is best for now.

[amanda-phet]

I am fine with this, but @kathy-phet is taking time to closely review this issue and she should make the final call.

[pixelzoom]

One more note... If everyone signs off on the proposal in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-432297426, we are doing so with the understanding that this is a workaround, not a solution. As noted in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-431635796, I think this issue demonstrates deep flaws with the "all or nothing" approach that PhET-iO has decided to take, as well as problems with the interaction between IO types and their wrapped types. Before closing this issue, a more general PhET-iO issue should be created to identify a general solution.

[jonathanolson]

An alternative approach to having clients every directly control visibility of certain nodes (marking some read-write or some read-only) would be to create visible/hidden Properties for each node that we want to support toggling this for (and have those exposed via the API). API-wise this would be clear, and code-wise it would make things simple (if not somewhat verbose).

Otherwise I share @pixelzoom's general thoughts, and the "hope the sim doesn't do X after we set a value" doesn't seem scalable.

[samreid]

We would like to try an approach that has this API:

  class VertexManipulator extends GQManipulator {

    /**
     * @param {NumberProperty} hProperty - h coefficient of the vertex form of the quadratic equation
     * @param {NumberProperty} kProperty - k coefficient of the vertex form of the quadratic equation
     * @param {Property.<Quadratic>} quadraticProperty - the interactive quadratic
     * @param {Graph} graph
     * @param {ModelViewTransform2} modelViewTransform
     * @param {BooleanProperty} vertexVisibleProperty
     * @param {BooleanProperty} coordinatesVisibleProperty
     * @param {Object} [options]
     */
    constructor( hProperty, kProperty, quadraticProperty, graph, modelViewTransform,
                 vertexVisibleProperty, coordinatesVisibleProperty, options ) {

    options = _.extend( {

      // GQManipulator options
      radius: modelViewTransform.modelToViewDeltaX( GQConstants.MANIPULATOR_RADIUS ),
      color: GQColors.VERTEX,
      coordinatesForegroundColor: 'white',
      coordinatesBackgroundColor: GQColors.VERTEX,
      coordinatesDecimals: GQConstants.VERTEX_DECIMALS,
      tandem: Tandem.required,
      phetioDocumentation: 'a manipulator for the vertex.',
      phetioOverrides: {
        visibleProperty: {
          phetioReadOnly: true
        },
        textProperty:{
          phetioDocumentation: 'Hello!'
        }
      }
    }, options );

[pixelzoom]

Notes from 11/1/18 phet-io meeting with @kathy-phet and devs:

• [x] we'll try what is described in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-435135353 (phetioOverrides herein)
• [x] proceed with PhET-iO 1.0 assuming that phetioOverrides is not available, add it in a further release
• [x] GQ will not Instrument {DerivedProperty} visibleProperty in order to avoid phetioID namespace collision
• [x] if phetioOverrides works out, GQ will set phetioOverrides: { visibleProperty: { phetioReadOnly: true } } to prevent clients from modifying NodeIO's visibility directly.

[samreid]

In developing this, I realized a better name than phetioOverrides would be phetioComponentOptions so first commit will use that.

[samreid]

The preceding commit adds phetioComponentOptions and example usage in GQManipulator which looks like this:

        phetioComponentOptions: {
          visibleProperty: {
            phetioReadOnly: true
          }
        }

I tested in Studio and this has the correct behavior that the rest of the NodeIO is mutable, but the visibleProperty is readOnly.

I chose an implementation strategy that is centralized in PhetioObject, but after reflection think we should implement something type specific--for instance since NodeIO knows it will have visibleProperty and pickableProperty and opacityProperty subcomponents, perhaps the API and implementation should reflect that.

[samreid]

Here's a patch that uses the Node-specific API and implementation for Node subcomponents:

Index: graphing-quadratics/js/common/view/GQManipulator.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- graphing-quadratics/js/common/view/GQManipulator.js (revision 5d8b8bb0cd3ab9fe7b743b4d60a0f154521cf1af) +++ graphing-quadratics/js/common/view/GQManipulator.js (date 1541135175000) @@ -44,10 +44,8 @@ // Manipulator options haloAlpha: 0.15, tandem: Tandem.required, - phetioComponentOptions: { - visibleProperty: { - phetioReadOnly: true - } + phetioVisiblePropertyOptions: { + phetioReadOnly: true } }, options ); Index: tandem/js/PhetioObject.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- tandem/js/PhetioObject.js (revision 1b93c58b6de5068171726eb1dc12e4d9111a29f8) +++ tandem/js/PhetioObject.js (date 1541135318000) @@ -38,7 +38,6 @@ // and otherwise can be suppressed. phetioStudioControl: true, // By default, Studio creates controls for many types of instances. This option // can be set to false to direct Studio to omit the control for the instance. - phetioComponentOptions: null // }; var OPTIONS_KEYS = _.keys( DEFAULTS ); @@ -92,9 +91,6 @@ // false to direct Studio to omit the control for the instance. this.phetioStudioControl = null; - // @public {Object} options to pass through to direct child subcomponents, see NodeIO - this.phetioComponentOptions = null; - if ( options ) { this.initializePhetioObject( {}, options ); } @@ -161,7 +157,6 @@ this.phetioHighFrequency = options.phetioHighFrequency; this.phetioPlayback = options.phetioPlayback; this.phetioStudioControl = options.phetioStudioControl; - this.phetioComponentOptions = options.phetioComponentOptions || {}; // Instantiate the wrapper instance which is used for PhET-iO communication if ( PHET_IO_ENABLED && this.tandem.supplied ) { Index: scenery/js/nodes/Node.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- scenery/js/nodes/Node.js (revision 5daa4a4564067377034af5d605a9c3fe542261dc) +++ scenery/js/nodes/Node.js (date 1541135318000) @@ -4987,6 +4987,10 @@ } } ); + // @public - for NodeIO customization + this.phetioVisiblePropertyOptions = options.phetioVisiblePropertyOptions; + this.phetioPickablePropertyOptions = options.phetioPickablePropertyOptions; + this.phetioOpacityPropertyOptions = options.phetioOpacityPropertyOptions; this.initializePhetioObject( { phetioType: NodeIO }, options ); return this; // allow chaining Index: scenery/js/nodes/NodeIO.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- scenery/js/nodes/NodeIO.js (revision 5daa4a4564067377034af5d605a9c3fe542261dc) +++ scenery/js/nodes/NodeIO.js (date 1541135278000) @@ -43,7 +43,7 @@ tandem: node.tandem.createTandem( 'visibleProperty' ), phetioDocumentation: 'Controls whether the Node will be visible (and interactive), see the NodeIO documentation for more details.' - }, node.phetioComponentOptions.visibleProperty ) ); + }, node.phetioVisiblePropertyOptions ) ); var pickableProperty = new NodeProperty( node, 'pickability', 'pickable', _.extend( { @@ -54,11 +54,11 @@ tandem: node.tandem.createTandem( 'pickableProperty' ), phetioType: PropertyIO( NullableIO( BooleanIO ) ), phetioDocumentation: 'Sets whether the node will be pickable (and hence interactive), see the NodeIO documentation for more details' - }, node.phetioComponentOptions.pickableProperty ) ); + }, node.phetioPickablePropertyOptions ) ); // Adapter for the opacity. Cannot use NodeProperty at the moment because it doesn't handle numeric types // properly--we may address this by moving to a mixin pattern. - var opacityProperty = new NumberProperty( node.opacity, { + var opacityProperty = new NumberProperty( node.opacity, _.extend( { // pick the following values from the parent Node phetioReadOnly: node.phetioReadOnly, @@ -67,7 +67,7 @@ tandem: node.tandem.createTandem( 'opacityProperty' ), range: new Range( 0, 1 ), phetioDocumentation: 'Opacity of the parent NodeIO, between 0 (invisible) and 1 (fully visible)' - } ); + }, node.phetioOpacityPropertyOptions ) ); opacityProperty.link( function( opacity ) { node.opacity = opacity; } ); node.on( 'opacity', function() { opacityProperty.value = node.opacity; } );

An example usage of this API would be something like:

        phetioVisiblePropertyOptions: {
          phetioReadOnly: true
        }

I'm not certain which strategy will be better--the latter is more clear what is supported and more direct, but will end up duplicating a little work each time we use this pattern.

[pixelzoom]

I'm not clear on the previous 2 comments... Have you changed direction from having one option with nested options for each instrumented Property, like this:

        phetioComponentOptions: {
          visibleProperty: {
            phetioReadOnly: true
          }
        }

to an option per Property, something like this?

        phetioVisiblePropertyOptions: {
          phetioReadOnly: true
        }

[samreid]

I committed as described in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-435272304 and additionally explored and provided a patch that implements the API described in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-435272907. I commented that I'm not certain which strategy will be better.

[zepumph]

I think I like the commit more than the patch. I think we should add documentation on what keys are supported, i.e. right now it is just the three NodeIO properties, but in the future could be more, and thus should be documented as such.

With that documentation we could also keep track of the "allowed" keys in phetioObject and validate that there aren't incorrect keys.

That seems a bit preferable to me rather than having an extra key for each property override, especially since these are defined in PhetioObject.

      this.phetioComponentOptions = options.phetioComponentOptions || {};

      assert && Object.keys( this.phetioComponentOptions ).forEach( ( componentName ) => {
          assert( SUPPORTED_COMPONENTS.indexOf( componentName ) >= 0 );
        }
      );

[pixelzoom]

I'm not totally clear on what path we're going down here.

Here's an example for SCENERY/Text, whose IO type is TextIO. The example shows all of the possible IO Properties that can be configured. This example reflects my understanding of what we discussed in yesterday's phet-io meeting -- one phetioComponentOptions, with a key that corresponds to each IO Property name in the IO type hierarchy.

const helloText = new Text( 'hello', {
  ...
  // phet-io options
  tandem: options.tandem.createTandem( 'helloText' ),
  phetioDocumentation: '...',
  phetioComponentOptions: {

    // TextIO Property options
    textProperty: { phetioReadOnly: true, ... },

    // NodeIO Property options
    visibleProperty: { ... },
    enabledProperty: { ... },
    pickableProperty: { ... }
  }
} );

But it sounds like maybe that has morphed into something like this, with a separate phetio option for each IO Property:

const helloText = new Text( 'hello', {
  ...
  // phet-io options
  tandem: options.tandem.createTandem( 'helloText' ),
  phetioDocumentation: '...',
  phetioTextPropertyOptions: { phetioReadOnly: true, ... },
  phetioVisiblePropertyOptions: { ... },
  phetioEnabledPropertyOptions: { ... },
  phetioPickablePropertyOptions: { ... }
} );

Or something else? Please clarify.

[samreid]

There are two proposals on the table, one is phetioComponentOptions and one is e.g., phetioVisiblePropertyOptions. @zepumph said he prefers the former. Which do you prefer?

[pixelzoom]

I have a strong preference for phetioComponentOptions, for the following reasons:

• it forces all of the info related to IO Property configuration to be grouped together
• it's encourages grouping and document which options are for which IO types, as I did in my example above (e.g. "// TextIO Property options")
• the keys match the Property names in the IO types, which makes them easy to associate, and is consistent with the spirit of PhET-iO naming conventions
• it requires only 1 additional option with 'phetio' prefix (phetioComponentOptions)
• if we decide to look for keys that are not relevant, it will be cleaner to validate phetioComponentOptions than trying to validate options

[samreid]

Sounds good, let's ignore the patch in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-435272907 and move forward with what is committed. Right NodeIO and TextIO leverage it, but it not implemented to automatically handle arbitrary new sub components.

[pixelzoom]

Right NodeIO and TextIO leverage it, but it not implemented to automatically handle arbitrary new sub components.

I don't follow. To make this applicable to any IO type, phetioComponentOptions would be passed to the IO type's constructor, then propagated to is supertype's constructor. And each IO type in the hierarchy would access the key:value pairs that correspond to it's Properties. Is that the way it's implemented? (Sorry, not in a place where I can pursue the implementation right now.)

[samreid]

Currently, the phetioComponentOptions are stored on the PhetioObject and options are created via _.extend like so:

  function NodeIO( node, phetioID ) {
    assert && assertInstanceOf( node, scenery.Node );
    ObjectIO.call( this, node, phetioID );

    var visibleProperty = new NodeProperty( node, 'visibility', 'visible', _.extend( {

      // pick the following values from the parent Node
      phetioReadOnly: node.phetioReadOnly,
      phetioState: node.phetioState,
      phetioType: PropertyIO( BooleanIO ),

      tandem: node.tandem.createTandem( 'visibleProperty' ),
      phetioDocumentation: 'Controls whether the Node will be visible (and interactive), see the NodeIO documentation for more details.'
    }, node.phetioComponentOptions.visibleProperty ) );

I investigated ways to make this fully automatic based on tandem names, but (at least the strategies I thought about so far) seemed too sketchy and "magical".

[zepumph]

I investigated ways to make this fully automatic based on tandem names, but (at least the strategies I thought about so far) seemed too sketchy and "magical".

For right now that seems ok. Could you please comment on my proposal in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-435492702 to provide some sort of validation/documentation in PhetioObject.

[pixelzoom]

After @samreid responds to @zepumph's question in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-435546924, we can probably close this issue.

[samreid]

In the preceding commits, I declared and checked phetioComponentOptions against a whitelist as @zepumph indicated in https://github.com/phetsims/graphing-quadratics/issues/80#issuecomment-435492702. @zepumph can you please review and close if all is well?

[chrisklus]

Reviewed by @zepumph, @samreid, and @chrisklus. Closing.