phetsims / models-of-the-hydrogen-atom

"Models of the Hydrogen Atom" is an educational simulation in HTML5, by PhET Interactive Simulations at the University of Colorado Boulder.
GNU General Public License v3.0
2 stars 3 forks source link

modelModeProperty is confusing. #66

Closed pixelzoom closed 2 months ago

pixelzoom commented 2 months ago

From the PhET-iO design document:

modelsOfTheHydrogenAtom.spectraScreen.model.modelModeProperty This changes the scene between experiment/model, and I find the name strange (because it starts with the word “model” which is one of the options). Probably sceneModeProperty, or modeProperty? Or ask CM for ideas. NS also model.model looks strange.

pixelzoom commented 2 months ago

Impromptu design meeting on 9/23/24 with @DianaTavares @arouinfar @Nancy-Salpepi @pixelzoom

I explained that modelModeProperty corresponds to the ModelMode enumeration type, defined like this:

export const ModelModeValues = [ 'experiment', 'model' ] as const;
export type ModelMode = ( typeof ModelModeValues )[number];

By PhET-iO convention, we name Properties based on their type. Hence the name modelModeProperty. No one had a better idea for renaming ModelMode.

Some alternative names for modelModeProperty that we considered were modeProperty and experimentModelProperty. No one was excited about these, and they have their own problems.

We also noted that there is experimentModelSwitch whose type is ABSwitch<ModelMode>.

pixelzoom commented 2 months ago

I replaced modelModeProperty: Property<ModelMode> with isExperimentProperty: Property<boolean>, and deleted the ModelMode enumeration type. PhET-iO metadata is shown below. @DianaTavares @arouinfar please review, let me know if this seems like a reasonable solution. The last person to review can close the issue.

screenshot_3500
pixelzoom commented 2 months ago

I initially created this issue in the wrong repo, so it was transferred from ph-scale to models-of-the-hydrogen-atom.

DianaTavares commented 2 months ago

I find isExperimentProperty very clear with boolean values! but leave @arouinfar to review because she has more experience with PhET-iO.

arouinfar commented 2 months ago

Thanks @pixelzoom! The changes look good on main. I agree with @DianaTavares, isExperimentProperty is very clear.

Closing!