search

phetsims / natural-selection

"Natural Selection" is an educational simulation in HTML5, by PhET Interactive Simulations
GNU General Public License v3.0
3 stars 7 forks source link

Pass visibleProperty option to Node where possible #262

Closed pixelzoom closed 3 years ago

pixelzoom commented 3 years ago

The ability to pass visibleProperty directly to a Node (via options) is a relatively new scenery feature. It typical eliminates the need to set visibleProperty to phetioReadOnly: true, because the Node is directly connected to the model Property. This feature did not exist when Natural Selection was instrumented, and I only discovered this pattern in the past couple of days (it's not documented anywhere afaik). Now that #251 (1.3 release) is coming up, I'm assuming that we'll want to apply this pattern.

naturalSelection.introScreen.view.graphs.populationNode.populationGraphNode.dataProbeNode.visibleProperty is a good example. It can be connected directly to naturalSelection.introScreen.model.graphs.populationModel.dataProbe.visibleProperty.

I'll make a first pass through this, then @amanda-phet and @kathy-phet will need to review.

pixelzoom commented 3 years ago

The data probe was the only place where this was applicable. dataProbeNode.visibleProperty (view) is now linked to dataProbe.visibleProperty (model). Here's what that looks like in Studio:

screenshot_212

Internally, the Nodes for each of the "flags" on the data probe are also passed a model Property. But none of those Nodes are instrumented, so it doesn't result in any API changes.

@amanda-phet there's not much to review here, and the API didn't change, but assigning to you in case you want to take a quick look. Feel free to close.

amanda-phet commented 3 years ago

Thanks. This looks fine to me. I was trying to wrap my head around what was different- oftentimes making something visible is done in the view, but when visibility is controlled by a checkbox it seems to be done in the model. So it seems totally fine to me!