phetsims / molecule-polarity

"Molecule Polarity" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
2 stars 6 forks source link

Review `phetioFeatured` overrides for 1.3 release #150

Closed arouinfar closed 1 year ago

arouinfar commented 1 year ago

Related to #142

There have been many common code changes since 1.2, so it would be good to review the phetioFeatured overrides for the upcoming release.

One thing that seems odd is that examples.md includes an example that hides the Electronegativity slider, but that leaves behind an empty panel. This looks weird, and we should likely set slider.visibleProperty to phetioFeatured: false and update the example in #149.

image
pixelzoom commented 1 year ago

... This looks weird, and we should likely set slider.visibleProperty to phetioFeatured: false and update the example in #149.

image

I could also address this by setting phetioVisiblePropertyInstrumented: false for the slider, so it can't be hidden. But it's more work to override the default. And so what if someone does this -- there are lots of things that you can do with PhET-iO that you shouldn't. So my recommendation is to leave the implementation as-is, set phetioFeatured: false if you'd like, and definitely choose a different example for examples.md.

arouinfar commented 1 year ago

I reviewed the tree, and the only issue I found with the overrides was slider.visibleProperty.

So my recommendation is to leave the implementation as-is, set phetioFeatured: false if you'd like, and definitely choose a different example for examples.md.

Agreed. I've updated the overrides, but since I can't regenerate the API files, I'm not going to commit it. I'll take care of the examples in #149

@pixelzoom can you please take care of the commit?

phetioFeatured overrides ```js /* eslint-disable */ window.phet.preloads.phetio.phetioElementsOverrides = { "moleculePolarity.global.model.preferences.dipoleDirectionProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.eFieldEnabledProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.angleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.atomA.electronegativityProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.atomA.partialChargeProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.atomA.positionProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.atomB.electronegativityProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.atomB.partialChargeProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.atomB.positionProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.atomC.electronegativityProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.atomC.partialChargeProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.atomC.positionProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.bondAB.dipoleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.bondAngleABCProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.bondBC.dipoleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.model.molecule.dipoleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.view.controlPanel.eFieldControl.onOffSwitch.toggleSwitch.visibleProperty": { "phetioFeatured": false }, "moleculePolarity.threeAtomsScreen.view.controlPanel.eFieldControl.onOffSwitch.visibleProperty": { "phetioFeatured": false }, "moleculePolarity.threeAtomsScreen.view.controlPanel.eFieldControl.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.view.controlPanel.viewControls.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.view.controlPanel.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.view.electronegativityPanels.atomAElectronegativityPanel.slider.visibleProperty": { "phetioFeatured": false }, "moleculePolarity.threeAtomsScreen.view.electronegativityPanels.atomAElectronegativityPanel.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.view.electronegativityPanels.atomBElectronegativityPanel.slider.visibleProperty": { "phetioFeatured": false }, "moleculePolarity.threeAtomsScreen.view.electronegativityPanels.atomBElectronegativityPanel.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.view.electronegativityPanels.atomCElectronegativityPanel.slider.visibleProperty": { "phetioFeatured": false }, "moleculePolarity.threeAtomsScreen.view.electronegativityPanels.atomCElectronegativityPanel.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.view.electronegativityPanels.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.view.moleculeNode.hintArrowsNode.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.view.viewProperties.bondDipolesVisibleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.view.viewProperties.molecularDipoleVisibleProperty": { "phetioFeatured": true }, "moleculePolarity.threeAtomsScreen.view.viewProperties.partialChargesVisibleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.model.eFieldEnabledProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.model.molecule.angleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.model.molecule.atomA.electronegativityProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.model.molecule.atomA.partialChargeProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.model.molecule.atomA.positionProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.model.molecule.atomB.electronegativityProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.model.molecule.atomB.partialChargeProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.model.molecule.atomB.positionProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.model.molecule.bond.dipoleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.model.molecule.dipoleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.view.controlPanel.eFieldControl.onOffSwitch.toggleSwitch.visibleProperty": { "phetioFeatured": false }, "moleculePolarity.twoAtomsScreen.view.controlPanel.eFieldControl.onOffSwitch.visibleProperty": { "phetioFeatured": false }, "moleculePolarity.twoAtomsScreen.view.controlPanel.eFieldControl.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.view.controlPanel.surfaceControl.radioButtonGroup.visibleProperty": { "phetioFeatured": false }, "moleculePolarity.twoAtomsScreen.view.controlPanel.surfaceControl.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.view.controlPanel.viewControls.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.view.controlPanel.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.view.electronegativityPanels.atomAElectronegativityPanel.slider.visibleProperty": { "phetioFeatured": false }, "moleculePolarity.twoAtomsScreen.view.electronegativityPanels.atomAElectronegativityPanel.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.view.electronegativityPanels.atomBElectronegativityPanel.slider.visibleProperty": { "phetioFeatured": false }, "moleculePolarity.twoAtomsScreen.view.electronegativityPanels.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.view.moleculeNode.hintArrowsNode.visibleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.view.viewProperties.bondCharacterVisibleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.view.viewProperties.bondDipoleVisibleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.view.viewProperties.partialChargesVisibleProperty": { "phetioFeatured": true }, "moleculePolarity.twoAtomsScreen.view.viewProperties.surfaceTypeProperty": { "phetioFeatured": true } }; ```
pixelzoom commented 1 year ago

In the above commits, overrides.js was updated and the API file re-generated. Back to @arouinfar for review, close if it looks OK.

arouinfar commented 1 year ago

Thanks @pixelzoom! Looks good in master, closing.