Closed Nancy-Salpepi closed 1 year ago
Thanks @Nancy-Salpepi. I think a better user experience in Studio would be to select between the possible solutes using radio buttons.
The get/setValue interface works, but it's a bit quirky. If the phetioID has a typo or there is a syntax error, Studio will catch the error and a dialog will pop up. The pH field, however, is ignored. This is true when using Set Value interface in Studio as well as when you use phetioClient.invoke
in the console. For example:
soluteProperty
and press Get Value to populate the box.I am pretty sure we asked @pixelzoom to display the pH as part of the solute value so clients have easy access to the stock pH. My guess is that the inclusion of the pH may be to blame for the strange get/setValue behavior highlighted in the example above.
Below is a patch that sets the validValues
for dropper.soluteProperty
, which causes Studio to present those values as radio buttons, like this:
I didn't commit the patch because there's a lot of overhead involved, including:
Testing migration is problematic because common code is not in a stable state (as I understand it). So I'm going to wait until RC testing has been completed, and common-code changes for migration have been completed.
Common migration work shouldn't block this. Please feel free to add rules and test this with 1.5.
I agree, this seems safe to commit to master.
In the above commits, this issue was addressed in master for ph-scale and ph-scale-basics.
In the above commits, this issue was patched in ph-scale 1.6 and its dependencies.
In the above commits, this issue was patched in ph-scale-basics 1.6 and its dependencies.
So much for "safe to proceed". Migration from ph-scale 1.5 => 1.6 works OK. But migration from ph-scale-basics 1.5. => 1.6 fails (in master!) with:
PhetioStateEngine.ts:325 Uncaught Error: Assertion failed: must support phetioState to be set at window.assertions.assertFunction (assert.js:28:13) at PhetioStateEngine.setStateForPhetioObject (PhetioStateEngine.ts:406:15) at PhetioStateEngine.ts:310:14 at Array.forEach (
) at PhetioStateEngine.iterate (PhetioStateEngine.ts:295:22) at PhetioStateEngine.setState (PhetioStateEngine.ts:253:31) at PhetioStateEngine.setFullState (PhetioStateEngine.ts:269:10) at PhetioEngine.implementation (phetioEngine.ts:1035:65) at PhetioCommandProcessor.processCommand (phetioCommandProcessor.ts:417:51) at getReturn (phetioCommandProcessor.ts:295:36)
So I'm blocked. @zepumph @samreid please advise.
PhetioStateEngine.ts:325 Uncaught Error: Assertion failed: must support phetioState to be set
This is issue https://github.com/phetsims/ph-scale/issues/268. On hold until that issue is addressed.
Assertion was resolved by adding missing rules to ph-scale-basics-migration-rules.ts, see https://github.com/phetsims/ph-scale-basics/issues/56.
This is ready for testing in the next RC.
This is not ready for RC. Need to cherry-pick changes from https://github.com/phetsims/ph-scale-basics/issues/56.
@samreid and I will not cherry pick anything into branches for this issue, but will make sure that once other, blocking issues have their content cherry picked, that this is working correctly.
Just to clarify... Changes to ph-scale and API files have already been cherry-picked. What has not been cherry-picked is changes to migration rules. I think that's what @zepumph is saying that they "will not cherry pick" in the preceding comment.
@zepumph and I cherry-picked the related changes, and regenerated the API files. It is working correctly locally. Would be good for @Nancy-Salpepi to confirm in RC.2.
I confirmed in ph-scale and basics 1.6.0-rc.2. Over to QA.
This looks good in rc.3 for pH Scale and pH Scale Basics. Closing!
Test device Macbook air (m1 chip)
Operating System 13.1
Browser safari
Problem description For https://github.com/phetsims/qa/issues/872 and https://github.com/phetsims/qa/issues/873, the solutes should have set values (radio buttons instead of get/set value buttons).