phetsims / calculus-grapher

"Calculus Grapher" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
4 stars 4 forks source link

Changes for the Predict feature #232

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

In Slack#calculus-grapher, @amanda-phet said:

We have feedback from three teachers about the “predict” feature, pasted in to the design doc here. Please have a look before we have a brief design meeting Thursday to discuss.

pixelzoom commented 1 year ago

I discussed briefly with @amanda-phet today, and the 2 options that she is considering are:

(1) Do nothing.

(2) Make the 'Predict' feature optional, via a query parameter and/or Preference, off by default.

I was happy to hear that moving/adding 'Predict' to graphs other than f(x) was not being considered. That would be very problematic at this stage, for both the UI design and implementation.

amanda-phet commented 1 year ago

Discussed 2/16/23

Another option between 1-2 that is hidden a "medium" amount.

(1.5) Checkbox within the sim to show extra/advanced features

Preferences menu is a good place to put "global" preferences- things that impact multiple screens.

We like option (2), with a preference toggle AND query parameter.

amanda-phet commented 1 year ago

Let's name it predict.

Need to consider PhET-iO, so this might take a little more time than 30 minutes.

pixelzoom commented 1 year ago

We did not discuss whether the student's Predict curve should be reset if they toggle the Predict preference on/off. I don't see any harm in keeping whatever Predict curve they had most recently set up - they can erase it if they wish. Doing nothing involves less code, and avoid problems when restoring PhET-iO state. So I'm going to leave the Predict curve unaffected by the state of the Predict preference.

If for some reason we want to reset the Predict curve when the Predict preference is toggled, add this to CalculusGrapherModel.ts:

    // If the Predict feature is enabled via Preferences, and we're not restoring PhET-iO state, then
    // reset the predictCurve.
    CalculusGrapherPreferences.predictPreferenceEnabledProperty.link( predictPreferenceEnabled => {
      if ( predictPreferenceEnabled && !phet.joist.sim.isSettingPhetioStateProperty.value ) {
        this.predictCurve.reset();
      }
    } );

For the same reasons, I'm also not resetting the state of the f(x)/Predict radio buttons, or the "Show f(x)" checkbox.

pixelzoom commented 1 year ago

@amanda-phet this is ready for review in master.

The query parameter is predict:

  /**
   * Whether the 'Predict' feature is available
   */
  predict: {
    type: 'boolean',
    defaultValue: false,
    public: true
  },

Below is a screenshot of the Preferences dialog.

And as noted in https://github.com/phetsims/calculus-grapher/issues/232#issuecomment-1433840013... If you turn on the Predict preference, make changes to the Predict features (select the Predict radio button, edit the curve, select the "Show f(x)" checkbox, etc.), turn off the Predict features, then turn the Predict features back on, you'll see that the state related to the Predict feature has been preserved. This is the default behavior, and is compatible with PhET-iO. If this is not acceptable, then we need to have a design discussion. For example, if you want all Predict-related features to be reset whenever the Predict preference is turned on, that will require more work, and some special code to properly handle restoring PhET-iO state.

screenshot_367

amanda-phet commented 1 year ago

I am fine with the predict state being preserved. If that's the only thing needed for review here, I will close this issue.