planet.massProperty should be Read Only

phetsims / keplers-laws

"Kepler's Laws" is an educational simulation in HTML5, by PhET Interactive Simulations.

GNU General Public License v3.0

1 stars 1 forks source link

planet.massProperty should be Read Only #255

Closed Nancy-Salpepi closed 8 months ago

Nancy-Salpepi commented 9 months ago

Test device MacBook Air M1 chip

Operating System 14.3

Browser Safari 17.3

Problem description For https://github.com/phetsims/qa/issues/1034, the acceptable range for the planet.massProperty is only 1 value (50). Discussed with @arouinfar over slack and it makes more sense for this property to be Read Only.

Visuals

AgustinVallejo commented 9 months ago

I believe the current implementation of BodyInfo doesn't allow to pass options (such as phetioReadonly: true) into those individual properties. If I recall correctly that's why we changed the range to [50,50]. Could you chime in, @pixelzoom?

pixelzoom commented 9 months ago

BodyInfo supports specifying metadata for positionProperty and velocityProperty via options positionPropertyOptions and velocityPropertyOptions respectively. Following the same pattern, I added massPropertyOptions, specified phetioReadOnly: true for planet.massProperty, and regenerated the API file.

@AgustinVallejo please review. And you'll need to cherry-pick to the 1.2 release branch.

AgustinVallejo commented 9 months ago

Good changes! Thanks @pixelzoom. Assigning back to QA to double check and mark as ready to cherry pick

Nancy-Salpepi commented 9 months ago

Looks good!

Nancy-Salpepi commented 8 months ago

looks good in rc.2