Turn on `phetioFeatured` for necessary elements

[AgustinVallejo]

Related to https://github.com/phetsims/keplers-laws/issues/206

[AgustinVallejo]

I believe this is ready for review.

[pixelzoom]

I don't see any phetioFeatured requests in https://github.com/phetsims/keplers-laws/issues/220 (Tree Review), so phetioFeatured is probably something that @arouinfar hasn't reviewed yet. If there are no phetioFeatured requests, feel free to close this issue. Otherwise let us know what you'd like changed.

[arouinfar]

@AgustinVallejo @pixelzoom I've identified additional elements to feature, as well as a few to unfeature. For brevity, I only (un)featured elements on a single screen, but please apply these decisions to all relevant screens.

/* eslint-disable */
window.phet.preloads.phetio.phetioElementsOverrides =
  {
    "keplersLaws.firstLawScreen.model.stopwatch.isRunningProperty": {
      "phetioFeatured": true
    },
    "keplersLaws.firstLawScreen.model.stopwatch.isVisibleProperty": {
      "phetioFeatured": true
    },
    "keplersLaws.firstLawScreen.model.stopwatch.positionProperty": {
      "phetioFeatured": true
    },
    "keplersLaws.firstLawScreen.model.stopwatch.timeProperty": {
      "phetioFeatured": true
    },
    "keplersLaws.firstLawScreen.model.time.timeProperty": {
      "phetioFeatured": false
    },
    "keplersLaws.firstLawScreen.view.panels.firstLawPanels.moreOrbitalDataPanel": {
      "phetioFeatured": true
    },
    "keplersLaws.firstLawScreen.view.panels.rightPanels.visibilityPanel.visibleProperty": {
      "phetioFeatured": true
    },
    "keplersLaws.firstLawScreen.view.zoomButtonGroup.visibleProperty": {
      "phetioFeatured": true
    },
    "keplersLaws.global.model.phetioTargetOrbits.targetOrbit1Property": {
      "phetioFeatured": true
    },
    "keplersLaws.global.model.phetioTargetOrbits.targetOrbit2Property": {
      "phetioFeatured": true
    },
    "keplersLaws.global.model.phetioTargetOrbits.targetOrbit3Property": {
      "phetioFeatured": true
    },
    "keplersLaws.global.model.phetioTargetOrbits.targetOrbit4Property": {
      "phetioFeatured": true
    },
    "keplersLaws.global.model.preferences.moreOrbitalDataVisibleProperty": {
      "phetioFeatured": true
    },
    "keplersLaws.secondLawScreen.view.panels.secondLawPanels.periodDivisionsPanel.visibleProperty": {
      "phetioFeatured": true
    },
    "keplersLaws.secondLawScreen.view.panels.secondLawPanels.sweptAreaAccordionBox.visibleProperty": {
      "phetioFeatured": true
    },
    "keplersLaws.thirdLawScreen.view.panels.thirdLawPanels.graphAccordionBox.visibleProperty": {
      "phetioFeatured": true
    },
    "keplersLaws.thirdLawScreen.view.panels.thirdLawPanels.starMassPanel.massNumberControl": {
      "phetioFeatured": true
    },
    "keplersLaws.thirdLawScreen.view.periodTimerNode": {
      "phetioFeatured": true
    },
  };

[arouinfar]

I noticed that view.panels.rightPanels.orbitalInformationPanel contains sections for the checkboxes related to each specific law:

This means that there's quite a bit of irrelevant content in the tree for the first three screens. I suspect it's not possible to uninstrument the irrelevant sets of checkboxes, so I was hoping we could instead unfeature them.

[pixelzoom]

@arouinfar said:

I noticed that view.panels.rightPanels.orbitalInformationPanel contains sections for the checkboxes related to each specific law: ... This means that there's quite a bit of irrelevant content in the tree for the first three screens. I suspect it's not possible to uninstrument the irrelevant sets of checkboxes, so I was hoping we could instead unfeature them.

These should not be instrumented for screens where they are not relevant. It looks like there are some more places where Tandem.OPT_OUT should be applied conditionally.

@AgustinVallejo let me know if you need any assistance with this.

[AgustinVallejo]

All the requests in this issue have been done, including the changes in scenery-phet and solar-system-common. We opened https://github.com/phetsims/my-solar-system/issues/320 to cherry pick into MSS RC.

Please review and close if ready @arouinfar

[arouinfar]

Thanks @AgustinVallejo. I reviewed in main, and things are looking good.

However, things under model.periodTracker.fadingStopwatch seem a bit odd. I suspect that these elements are related to the highlighted path that is drawn while the Period Timer is actively recording. If possible, can we make these phetioFeatured: false? They should also be phetioReadOnly: true too. I don't see why clients should be in control of this data, or why they would care about it either.

[AgustinVallejo]

Done, assigning back for review!

[arouinfar]

Thanks @AgustinVallejo. Looks like you applied the changes only to fadingStopwatch and not its children. Can you please also make the same changes to its children? They're currently all still featured and some are also writeable.

[AgustinVallejo]

The problem here is that fadingStopwatch uses the Stopwatch common code component for tracking the time. However, that component has its children featured by default. Here we'd have to either: a) Completely change the fading implementation in Keplers b) Change Stopwatch.ts so more custom options can be passed to the individual children properties.

Will discuss with @arouinfar and @pixelzoom to see what's optimal.

[arouinfar]

That's unfortunate news. The fadingStopwatch is an implementation detail that isn't relevant to clients, but I also don't think it's worth additional development effort. Let's just leave it as-is.

[pixelzoom]

This is an unfortunate consequence of https://github.com/phetsims/scenery-phet/issues/827 (https://github.com/phetsims/scenery-phet/issues/827) where the request to feature all Stopwatch Properties was implemented in https://github.com/phetsims/scenery-phet/commit/ea2803ca07028d90d35a61de18f4b50f5d7f3b56.

I think I agree with @arouinfar in this case -- just leave it as-is, and don't worry about it. It's highly unlikely that anyone will change these Properties.