search

phetsims / ph-scale

"pH Scale" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/ph-scale
GNU General Public License v3.0
0 stars 7 forks source link

Uninstrument Text and RichText #287

Closed pixelzoom closed 1 month ago

pixelzoom commented 1 year ago

... the next time this sim is published. This will be an API change, and will require migration rules.

zepumph commented 1 year ago

https://github.com/phetsims/joist/issues/934

pixelzoom commented 1 year ago

At the same time ashttps://github.com/phetsims/beers-law-lab/issues/332, I'm going to do this eagerly, to see how adding migration rules goes. @samreid @zepumph FYI.

pixelzoom commented 1 year ago

waterFaucetNode.waterText should remaining instrumented, because waterFaucetNode.waterText.visibleProperty is featured.

In general, if a Text/RichText has any featured child elements, consult with @arouinfar before uninstrumenting.

I also discussed associated derived string Properties with @arouinfar. If they are used to labels a control, keep them instrumented. If they are are readouts of values that available elsewhere, remove instrumentation.

pixelzoom commented 1 year ago

Done in the above commits.

Also note that ABSwitch element (graphScaleSwitch and graphUnitsSwitch) are now more sparsely instrumented, to conform to the decisions made in https://github.com/phetsims/sun/issues/853 and https://github.com/phetsims/acid-base-solutions/issues/190.

These changes should be reviewed by the PhET-iO designer before republishing.

pixelzoom commented 1 month ago

Uninstrumenting Text and RichText has been the general approach for some time now, and https://github.com/phetsims/phet-io-sim-specific/commit/ca0f101e76c52b708e6921f5d042e17fc0af2c1d added the appropriate migration rules. So this issue can be closed.