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

Add Solute link to nameProperty #246

Closed pixelzoom closed 1 year ago

pixelzoom commented 2 years ago

Related to https://github.com/phetsims/ph-scale/issues/243 ...

We removed name from SoluteIO because name is now dynamic, a Property, and as @samreid said in https://github.com/phetsims/ph-scale/issues/243#issuecomment-1242815159:

Copying the current value of one instrumented property so it will show up in the state of another PhetioObject doesn't sound like a good idea to me.

So when https://github.com/phetsims/axon/issues/414 has been addressed, I will link Solute to its nameProperty.

pixelzoom commented 2 years ago

Before I actually do this, since it will be an API change... In https://github.com/phetsims/ph-scale/issues/243, recall that we removed 'name' from SoluteIO, because it's now dynamic. Would you like each Solute's nameProperty linked back to its corresponding translated string Property?

Here's how the the Studio tree currently looks. The addition would be a *nameProperty link under each Solute. For example, batteryAcid.nameProperty would link to phScale.general.model.strings.phScale.choice.batteryAcidStringProperty.

screenshot_1866
arouinfar commented 1 year ago

Great question @pixelzoom. If I remember correctly, global.model.solutes previously contained the nameProperty for each solute. Now that solute names are now under general.model.strings it seems best to link back to the appropriate stringProperty. Please proceed @pixelzoom.

pixelzoom commented 1 year ago

Done in the above commit, for ph-scale and ph-scale-basics. Screenshot below shows the ph-scale-basics Studio tree.

@arouinfar please review. Feel free to close.

screenshot_1887
arouinfar commented 1 year ago

Looks good, thanks @pixelzoom!