Closed pixelzoom closed 1 year ago
This is ready for PhET-iO design review by @arouinfar.
If you'd like to compare "before" and "after", use 1.2.0-dev.1 for "before" and master for "after".
Summary of changes:
Properties related to Options were formerly under geometricOptics.global.view.options
. They are now under geometricOptics.global.model.preferences
. This convention has not been discussed; I put them where they seemed to make the most sense. Also note that https://github.com/phetsims/joist/issues/840 plans to link Properties from geometricOptics.global.model.preferences
under geometricOptics.general.model.preferencesModel
. That is common-code work that has not been done yet.
geometricOptics.general.view.navigationBar.phetButton.phetMenu.optionsMenuItem
no longer exists, because there is no Options menu item in the PhET menu.
geometricOptics.general.view.navigationBar.phetButton.phetMenu.optionsDialogCapsule
no longer exists, because there is no Options dialog. Controls that appeared in the Options dialog were moved to the General tab of the Preferences dialog.
Controls that were added to the General tab of the Preferences dialog now appear under geometricOptics.general.view.navigationBar.preferencesButton.preferencesDialogCapsule.archetype.preferencesPanels.generalPreferencesPanel.simPreferences
. The simPreferences
naming convention was chosen in https://github.com/phetsims/joist/issues/837#issuecomment-1224743329. Feel free to recommend a different convention, since that's what this review is about.
@pixelzoom thanks for summarizing the changes. I reviewed both sims, and the Preferences dialog and corresponding elements in the tree all look good. Housing the properties in global.model.preferences
makes a lot of sense to me. Anything else to do here?
All done, closing.
Related to https://github.com/phetsims/joist/issues/837 ... @kathy-phet asked me to focus on Geometric Optics for Preferences dialog integration.
Some of this has already been handled by @zepumph in https://github.com/phetsims/joist/issues/837.