Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Solute link to nameProperty #246

Closed
pixelzoom opened this issue Sep 10, 2022 · 4 comments
Closed

Add Solute link to nameProperty #246

pixelzoom opened this issue Sep 10, 2022 · 4 comments

Comments

@pixelzoom
Copy link
Contributor

Related to #243 ...

We removed name from SoluteIO because name is now dynamic, a Property, and as @samreid said in #243 (comment):

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 phetsims/axon#414 has been addressed, I will link Solute to its nameProperty.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Sep 13, 2022

Before I actually do this, since it will be an API change... In #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
Copy link
Contributor

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
Copy link
Contributor Author

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

@pixelzoom pixelzoom assigned arouinfar and unassigned pixelzoom Sep 26, 2022
pixelzoom added a commit that referenced this issue Sep 26, 2022
pixelzoom added a commit to phetsims/beers-law-lab that referenced this issue Sep 26, 2022
@arouinfar
Copy link
Contributor

Looks good, thanks @pixelzoom!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants