phetsims / center-and-variability

"Center and Variability" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 2 forks source link

PhET-iO needs for March 30th (if possible) #105

Closed amanda-phet closed 2 years ago

amanda-phet commented 2 years ago

I believe some of these can already be checked, but leaving the complete list here so QA can test this when all of these capabilities are ready.

chrisklus commented 2 years ago

@Nancy-Salpepi here is what should be tested in Studio - I don't believe the first request exists yet, so @samreid and I will have to add that next week. I'm not sure if the second one is complete, so that would be helpful to know. And hopefully the last three are good to go.

Tagging https://github.com/phetsims/qa/issues/789

Nancy-Salpepi commented 2 years ago

@chrisklus For 2 on the list, I don't see visibleProperty for any of the checkboxes on either screen.

The Eraser Button also needs to be instrumented.

Nancy-Salpepi commented 2 years ago

@amanda-phet did you want individual control for the kick buttons? Currently there is only one visible property for the group: view.kickButtonGroup.visibleProperty.

Nancy-Salpepi commented 2 years ago

@chrisklus I don't see the get value, set value, copy set value code options for Mean and Median on either screen.

Screen Shot 2022-03-24 at 8 27 45 PM
KatieWoe commented 2 years ago

If there is a way to do 1 I haven't found it yet. Let me know if I'm missing something obvious but I can't seem to find this control, either in Featured or All.

Nancy-Salpepi commented 2 years ago

The first doesn't exist yet @KatieWoe . See https://github.com/phetsims/center-and-variability/issues/105#issuecomment-1076576977

KatieWoe commented 2 years ago

Ahh, sorry about that. Thanks for pointing it out.

samreid commented 2 years ago

We renamed eraserButton => eraseButton more like a verb, like the reset all button. We speculated about a "resetter button" which we didn't like the sound of.

samreid commented 2 years ago

I don't see the get value, set value, copy set value code options for Mean and Median on either screen.

Those options are not available for items marked as "read only". We don't want to allow PhET-iO users to override the mean or median, since it should be derived from the model values. There are controls to change the contributing values (ball locations) within the sim.

We added the kick button visibleProperty, added a pickableProperty for the soccer ball group, instrumented all the checkboxes.

@Nancy-Salpepi @KatieWoe @amanda-phet can you please review? If all is well, please relabel for cherry-picking.

Nancy-Salpepi commented 2 years ago

@samreid @chrisklus I have a question about the property view.topCheckboxGroup.sortDataCheckbox.enabledProperty

If I check the Sort Data checkbox, disable that checkbox and then launch the sim, the box will become unchecked once I move a card (and can't recheck it unless I press reset all). So should we be able to disable this property?

Nancy-Salpepi commented 2 years ago

When testing view.soccerBallNodeGroup.pickableProperty on the Mean and Median Screen, an error came up. After refreshing Master, I was not able to reproduce, but noting anyway.

Screen Shot 2022-03-29 at 4 18 42 PM
kathy-phet commented 2 years ago

@samreid @chrisklus - I don't think we use pickable property any more. I thought we used inputEnabledProperty. e.g, phScale.macroScreen.view.pHMeterNode.probeNode.inputEnabledProperty

@arouinfar - Can you comment?

arouinfar commented 2 years ago

I don't think we use pickable property any more. I thought we used inputEnabledProperty. e.g, phScale.macroScreen.view.pHMeterNode.probeNode.inputEnabledProperty

@arouinfar - Can you comment?

We deprecated pickableProperty in favor of inputEnabledProperty in https://github.com/phetsims/scenery/issues/1158. While I am fairly certain that "pickable" is still a thing found in the code, it's not something we should be exposing in the Studio tree. If a Node needs to be uninteractive without appearing to be disabled, it should have an inputEnabledProperty.

kathy-phet commented 2 years ago

maybe there should be an assertion if pickableProperty is in a phet-io name or something?

samreid commented 2 years ago

@samreid @chrisklus I have a question about the property view.topCheckboxGroup.sortDataCheckbox.enabledProperty

If I check the Sort Data checkbox, disable that checkbox and then launch the sim, the box will become unchecked once I move a card (and can't recheck it unless I press reset all). So should we be able to disable this property?

@chrisklus and I reproduced this behavior, and discussed it. It seems plausible that a client may want the checkbox to serve as a readout of whether the data is sorted, so disabling it may be reasonable in that case.

samreid commented 2 years ago

@chrisklus and I switched to use inputEnabledProperty instead of pickableProperty and opened a new issue about forbidding that tandem name in general. We weren't able to reproduce the problem described in https://github.com/phetsims/center-and-variability/issues/105#issuecomment-1082346338, so this issue is ready for review. @amanda-phet can you please review and mark for cherry-picking if it seems good?

amanda-phet commented 2 years ago

I confirmed the following locations for achieving what I listed:

And likewise on the Mean & Median screen:

I didn't list every single ID, just the ones I want to reference later. Ready for cherry-picking!

chrisklus commented 2 years ago

@samreid and I spot-checked this for https://github.com/phetsims/center-and-variability/issues/122, closing.