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

quadraticProperty and savedQuadraticProperty have different phetioReadOnly metadata #200

Closed
arouinfar opened this issue Jun 29, 2023 · 6 comments

Comments

@arouinfar
Copy link

For phetsims/qa#959

@Nancy-Salpepi asked over Slack:

Wondering if it is correct that the quadraticProperty doesn’t have the Get/Set/Copy buttons like the savedQuadraticProperty has?

The reason for this discrepancy is that quadraticProperty is read-only, but then I started wondering why we treat these two properties differently. Consulting the PhET-iO design doc, I found these requirements:

Read out / Pre-set the values a, b, c.
Read out all info about the current curve (vertex, aos, focus, directrix, a, b, c)
Read out/ Pre-set the “saved” curve

This appears to be the reason why the quadraticProperty is read-only but the savedQuadraticProperty is not. That said, I don't know if we would have made the same decisions if starting over today. If clients want to create a quadratic with the API, they need to communicate with three different properties (one for each coefficient in the equation). It seems more convenient for instructional designers to export the desired quadratics using the 'Get Command' functionality in Studio, and then share the code with the wrapper developer. This is the method we recommend for creating a saved quadratic for comparison purposes.

@pixelzoom would you have any reservations about changing quadraticProperty to phetioReadOnly: false?

@pixelzoom
Copy link
Contributor

pixelzoom commented Jul 2, 2023

@pixelzoom would you have any reservations about changing quadraticProperty to phetioReadOnly: false?

No. I'll make it so.

@pixelzoom
Copy link
Contributor

Done in the above commits. @arouinfar please review, close if OK.

@arouinfar
Copy link
Author

@pixelzoom would you have any reservations about changing quadraticProperty to phetioReadOnly: false?

No. I'll make it so.

Looks like you accidentally did this one backwards and made savedQuadraticProperty phetioReadOnly: true instead. Can you make them both phetioReadOnly: false?

@arouinfar arouinfar assigned pixelzoom and unassigned arouinfar Jul 5, 2023
@pixelzoom
Copy link
Contributor

That's not possible - quadraticProperty is a derived Property.

Do you really want me to restore savedQuadraticProperty to phetioReadOnly: false? It's type is PropertyIO<NullableIO<QuadraticIO>> and there is no way to set a value of that type via Studio. It would be a dubious affair setting it via the programming API.

@pixelzoom pixelzoom assigned arouinfar and unassigned pixelzoom Jul 5, 2023
@arouinfar
Copy link
Author

That's not possible - quadraticProperty is a derived Property.

Understood, thanks @pixelzoom.

Do you really want me to restore savedQuadraticProperty to phetioReadOnly: false? It's type is PropertyIO<NullableIO> and there is no way to set a value of that type via Studio. It would be a dubious affair setting it via the programming API.

Yes, please. The original design spec included presetting the saved quadratic. This is feasible to do with he API if Studio is being used to predetermine the saved quadratic. I've detailed how to use the 'Get Command' functionality to do this in examples.md:

If you would like to change the saved quadratic in your wrapper code, use the savedQuadraticProperty. We recommend that you predetermine the saved quadratics using Studio. The “Get Command” button in the PhET-iO Studio Element Panel will populate the textbox with the corresponding phetioClient.invoke command which can be copied to your clipboard.

  • graphingQuadratics.exploreScreen.model.savedQuadraticProperty
  • graphingQuadratics.standardFormScreen.model.savedQuadraticProperty
  • graphingQuadratics.vertexFormScreen.model.savedQuadraticProperty
  • graphingQuadratics.focusAndDirectrixScreen.model.savedQuadraticProperty

@pixelzoom
Copy link
Contributor

Changes to savedQuadraticProperty and the API file have been reverted. 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

2 participants