phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
http://scenerystack.org/
MIT License
9 stars 6 forks source link

`phetioFeatured` defaults for Preferences #875

Closed arouinfar closed 2 years ago

arouinfar commented 2 years ago

In https://github.com/phetsims/molecule-shapes/issues/230 and https://github.com/phetsims/gravity-and-orbits/issues/470, QA reported that preferencesDialog.preferencesTabs.LocalizationTab.visibleProperty was missing from the tree. It's not actually missing, but I realized that it wasn't phetioFeatured: true. It seems reasonable to feature any phetioID mentioned in the Preferences section PhET-iO Guide, so I reviewed the tree to see what else should be featured.

The guide describes how to hide the preferences tabs in the dialog, so I think we should feature preferencesDialogCapsule.archetype.preferencesTabs.*Tab.visibleProperty.

I also found this in the guide:

To aid PhET-iO Studio users in inspecting all Preferences, we created a preferencesModel where all of the preferences are linked and organized for ease of inspection. Recall, any API calls must be done on target phetioID, not the linked name.

Here is a brief description of these preference settings, collected at moleculeShapes.general.model.preferencesModel

The only elements linked under the preferencesModel that we specifically refer to in the documentation are the stateful properties: audioEnabledProperty, localeProperty, and colorProfileProperty. These are all phetioFeatured: true. However, the quote above suggests that users may want to inspect all of the preferencesModel contents, which they can only do if viewing "All" elements. Here's what preferencesModel looks like when viewing Featured vs. All elements.

image image

Should we feature all of the target elements collected here? Or should we specify that users should view "All" elements when inspecting the preferencesModel? If we only want to feature the stateful properties, then voicingEnabledProperty should be phetioFeatured: false.

For the foreseeable future, most of the properties under preferencesModel aren't actually applicable to most sims, so I am leaning towards just featuring the stateful properties. @zepumph @samreid what would you advise?

samreid commented 2 years ago

I made preferences tabs and their visibleProperties phetioFeatured: true in the commit.

The preferencesModel is overwhelming, especially for sims that do not leverage all those features. It seems best to start with that subset being phetioFeatured for the next few releases.

If we only want to feature the stateful properties, then voicingEnabledProperty should be phetioFeatured: false.

I'll mark it as phetioFeatured: false in the next commit.

samreid commented 2 years ago

OK fixed and ready for review, the featured tree now looks like this for GAO:

image

And the tabs and their visibleProperties are now featured.

arouinfar commented 2 years ago

Thanks @samreid. Master generally looks good, but I think you may have accidentally unfeatred colorProfileProperty. It's stateful and cited in an example customization in the guide, so I think it should remain phetioFeatured: true.

samreid commented 2 years ago

Thanks, I added that. Can you please test on phet-dev?

arouinfar commented 2 years ago

Thanks @samreid, looks good!