phetsims / buoyancy

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

Updates to preference for percentageSubmergedVisible #126

Closed zepumph closed 1 month ago

zepumph commented 1 month ago

From https://github.com/phetsims/buoyancy/issues/112. We need to be able to toggle the default of this preference based on the sim running. In Buoyancy we want the preference to default off (not showing the panel), then in buoyancy basics we do want it by default. The query parameter should still override these defaults though, so we many need to outfit a null/true/false value so we know if the query parameter was provided. Another idea is to just use QueryStringMachine.containsKey() to know if the value was provided vs is the default.

samreid commented 1 month ago

I'll take a look at this. Self-assigning.

samreid commented 1 month ago

I recommend a solution like this:

```diff Subject: [PATCH] Address TODOs, see https://github.com/phetsims/chipper/issues/1429 --- Index: js/common/DensityBuoyancyCommonQueryParameters.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/DensityBuoyancyCommonQueryParameters.ts b/js/common/DensityBuoyancyCommonQueryParameters.ts --- a/js/common/DensityBuoyancyCommonQueryParameters.ts (revision 0c8d369951cb271e2ae1464488db56a95af5f0c2) +++ b/js/common/DensityBuoyancyCommonQueryParameters.ts (date 1712764806826) @@ -11,6 +11,8 @@ export const VolumeUnitsValues = [ 'liters', 'decimetersCubed' ] as const; export type VolumeUnits = ( typeof VolumeUnitsValues )[number]; +const defaultPercentageSubmergedVisible = phet.joist.packageJSON.name !== 'buoyancy-basics'; + const DensityBuoyancyCommonQueryParameters = QueryStringMachine.getAll( { gEarth: { @@ -31,7 +33,7 @@ // Displays/hides the percentage submerged readout accordion box percentageSubmergedVisible: { type: 'boolean', - defaultValue: true, + defaultValue: defaultPercentageSubmergedVisible, public: true }, ```

@zepumph does this look good? Feel free to commit or let me know if I should.

zepumph commented 1 month ago

@samreid and I got to a commit point. Using packageJSON.name was such a nicer way to handle this than anything I was going to recommend. We also cleaned up how the preference is managed, since it was added to density too.

@DianaTavares, the behavior is now as follows:

Please review and feel free to close.

DianaTavares commented 1 month ago

Thanks team!

The behavior is correct! But in Buoyancy basics, yes the panel is on in the preference menu, but

Like this: image

Basics don't have the “compare” screen, but there also the % submerged panel is on, but close by default.

zepumph commented 1 month ago

Currently we don't actually have any code implemented for buoyancy basics. This is just an exact copy of the buoyancy explore screen. I wonder if we can add this to a list for when we implement the buoyancy basics sim.

zepumph commented 1 month ago

Wait. In Buoyancy, should the panel default to closed also?