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

Update metadata of several model properties #91

Closed
15 tasks done
arouinfar opened this issue Aug 30, 2021 · 8 comments
Closed
15 tasks done

Update metadata of several model properties #91

arouinfar opened this issue Aug 30, 2021 · 8 comments

Comments

@arouinfar
Copy link
Contributor

arouinfar commented Aug 30, 2021

For #76

Lots of model properties need some combination of phetioDocumentation and/or units.

density.*Screen.model.gravityProperty

  • add phetioDocumentation
  • add units

density.*Screen.model.invisibleBarrierBoundsProperty

  • add phetioDocumentation
  • change to phetioReadOnly: true

density.*Screen.model.liquidMaterialProperty

  • add units to density and viscosity values

density.*Screen.model.liquidViscosityProperty

  • add units

density.introScreen.model.showMassesProperty

  • add phetioDocumentation: Displays a mass readout on each block

density.*Screen.model.densityProperty

  • add units

density.Screen.model.massProperty

  • add units

density.introScreen.model.pool.liquidYInterpolatedProperty

  • add phetioDocumentation

density.*Screen.model.pool.liquidVolumeProperty

  • add phetioDocumentation

  • add units

  • For each block, buoyancyForceInterpolatedProperty, contactForceInterpolatedProperty, gravityForceInterpolatedProperty, massProperty, materialProperty (density/viscosity), and volumeProperty should have units.

  • For each block, buoyancyForceInterpolatedProperty, contactForceInterpolatedProperty, gravityForceInterpolatedProperty should be phetioHighFrequency: true

  • All instances of inputEnabledProperty should have this phetioDocumenation: Sets whether the element will have input enabled, and hence be interactive.

@jonathanolson
Copy link
Contributor

add phetioDocumentation: Displays a mass readout on each block

This will be a bit weird for phrasing in Buoyancy. Do we need to switch the documentation for things between the sims?

@arouinfar
Copy link
Contributor Author

This will be a bit weird for phrasing in Buoyancy. Do we need to switch the documentation for things between the sims?

Do you have any ideas for something that could work for both sims? It's preferable to keep the documentation the same, if possible.

jonathanolson added a commit to phetsims/density-buoyancy-common that referenced this issue Sep 1, 2021
@jonathanolson
Copy link
Contributor

Implemented changes, can you verify?

Do you have any ideas for something that could work for both sims? It's preferable to keep the documentation the same, if possible.

I've used the word "mass" instead of "block", however that could be somewhat confusing with that phrase.

@kathy-phet
Copy link

What about the phrase "object" or "shape" instead of "mass". I do think "Displays a mass readout on each mass" will be hard to understand.

jonathanolson added a commit to phetsims/density-buoyancy-common that referenced this issue Sep 1, 2021
@jonathanolson
Copy link
Contributor

Updated to "Displays a mass readout on each object" above.

@arouinfar
Copy link
Contributor Author

arouinfar commented Sep 3, 2021

@jonathanolson I missed the scale in my first review. Please make all of the following phetioReadOnly: true:

  • density.mysteryScreen.model.scale.inputEnabledProperty
  • density.mysteryScreen.model.scale.materialProperty
  • density.mysteryScreen.model.scale.scaleForceInterpolatedProperty (also needs units of N)

jonathanolson added a commit to phetsims/density-buoyancy-common that referenced this issue Sep 7, 2021
@jonathanolson
Copy link
Contributor

Implemented above, can you verify?

@jonathanolson jonathanolson removed their assignment Sep 7, 2021
@arouinfar
Copy link
Contributor Author

Looks good, closing.

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

3 participants