phetsims / graphing-quadratics

"Graphing Quadratics" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
1 stars 4 forks source link

Should `pointOnParabolaProperty` be read-only? #195

Closed arouinfar closed 1 year ago

arouinfar commented 1 year ago

For #180

The Focus & Directrix screen has manipulators for the vertex, focus, and a movable point along the parabola. The Point on Parobla has a settable pointOnParabolaProperty, but there is no way to enforce that the point remains on the parabola.

image

I'm not sure how useful it is for the client to have API control of pointOnParabolaProperty. However, I think it would also be a bit of a nasty surprise to make this read-only if a client has been relying on it. We allow API control of things like positionProperty even though we can't validate the coordinates. There, we instruct clients to predetermine coordinates in Studio and use 'Get Command' for later use in their wrapper.

I see two options:

(1) Make pointOnParabolaProperty read-only. (2) Leave it alone and document in examples.md.

I'm not sure which is preferable. If this was for 1.0, I would go with (1) but I'm leaning towards (2) due to migration concerns. @pixelzoom thoughts?

pixelzoom commented 1 year ago

pointOnParabolaProperty is not something that the client should be setting, so I think it should be read-only. Let's discuss via Zoom.

pixelzoom commented 1 year ago

@arouinfar and I discussed. pointOnParabolaProperty should be read-only. I'll make it so, and verify whether a migration rule is needed.

pixelzoom commented 1 year ago

Done in the above commit. This is an API change, but no migration rule appears to be required (according to the Migration wrapper). @arouinfar please review, close if OK.

arouinfar commented 1 year ago

Looks good, thanks @pixelzoom. Closing.