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

Should `allelesVisible` be settable via a Property? #317

Closed pixelzoom closed 1 year ago

pixelzoom commented 2 years ago

In https://github.com/phetsims/studio/issues/268, it was determined that using query parameters with wrappers is problematic, and that setting a Property (or calling an API function) should be preferred.

The allelesVisible query parameter determines whether the Alleles panel is added to the Pedigree graph. It does not have an associated Property or API function.

With allelesVisible=true (the default):

screenshot_1806

With allelesVisible=false:

screenshot_1805

If you run the sim with allelesVisible=false, changing the value of graphs.pedigreeNode.visibleProperty does not affect the visibility of the panel. The panel is added to the scenegraph only when allelesVisible=true.

Is this OK? Or do we need to add allelesVisibleProperty so that the client can change this from Studio, instead of using the query parameter?

Propority high, because Natural Selection is up next for https://github.com/orgs/phetsims/projects/44.

pixelzoom commented 2 years ago

I investigated this a bit. Being able to dynamically show/hide the Alleles panel will be a big project, because the Pedigree graph is not implemented to resize dynamically. An alternative would be to make the Pedigree graph have one size, regardless of whether the Alleles panel is visible, leaving a big empty spae to the left. But presumably we would like the Pedigree graph to use that available space.

pixelzoom commented 2 years ago

@kathy-phet and I discussed this a bit. Her feeling was that this is OK as is, and I agree. If the client wants to hide the Alleles panel and maximize the space used for the Pedigree graph, then they should use the allelesVisible query parameter. If they want to dynamically show/hide the AllelesPanel for some reason (and we had a difficult time thinking of a reason) then they can use the pedigreeNode.allelesPanel.visibleProperty and live with a bit of blank space to the right of the graph.

@amanda-phet please comment. And if our conclusion is OK with you, please close.

amanda-phet commented 1 year ago

Sounds good to me.