phetsims / keplers-laws

"Kepler's Laws" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 1 forks source link

Target Orbit panel needs a `visibleProperty` #228

Closed arouinfar closed 8 months ago

arouinfar commented 8 months ago

Discovered while creating examples.md for #223

There isn't currently a way for clients to hide the target orbit feature. The Target Orbit Panel needs a visibleProperty that is client-controllable. A potentially complicating factor is that the sim needs to control the visibility on the All Laws screen because the panel is only relevant to the first and third laws.

@AgustinVallejo or @pixelzoom please create view.panels.rightPanels.targetOrbitPanel.visibleProperty and make it phetioFeatured: true.

pixelzoom commented 8 months ago

@AgustinVallejo I think the patch below will address this, but I don't have time to test it and will be out tomorrow.

Subject: [PATCH] move flipPolarityButton below checkboxes, so it will be next to 'Earth' checkbox, https://github.com/phetsims/faradays-electromagnetic-lab/issues/43
---
Index: js/common/view/KeplersLawsPanels.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/KeplersLawsPanels.ts b/js/common/view/KeplersLawsPanels.ts
--- a/js/common/view/KeplersLawsPanels.ts   (revision c477e6b7c3a23aaa311ff2e5d50c68e30e713037)
+++ b/js/common/view/KeplersLawsPanels.ts   (date 1704411075897)
@@ -76,7 +76,11 @@
         targetOrbitPanel,
         orbitalInformationPanel,
         visibilityPanel
-      ]
+      ],
+      phetioVisiblePropertyInstrumented: true,
+      visiblePropertyOptions: {
+        phetioFeatured: true
+      }
     } );
   }
 }
AgustinVallejo commented 8 months ago

Added this. It had to be a linked property since there's already a dependency with the isSecondLawProperty (so the panel doesn't show up when 2nd law is set). Is this behavior ok @arouinfar? If so, please close

arouinfar commented 8 months ago

Thanks @AgustinVallejo. The general structure looks good on main, but we'll also need to feature targetOrbitPanel so that the linked visibleProperty shows up in the featured tree.

AgustinVallejo commented 8 months ago

Done, assigning back

arouinfar commented 8 months ago

Looks good on main, thanks! Closing.