phetsims / ratio-and-proportion

"Ratio and Proportion" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 4 forks source link

Some preferences maintained when opening standard wrapper #520

Closed Nancy-Salpepi closed 2 years ago

Nancy-Salpepi commented 2 years ago

Test device MacBook Air (m1 chip)

Operating System MacOS 12.5.1

Browser Safari 15.6.1

Problem description For https://github.com/phetsims/qa/issues/831 and related to https://github.com/phetsims/joist/issues/744 STUDIO: In the audio tab of the preferences menu, the following states carry over to the standard wrapper:

Should any of the preferences carry over to the standard wrapper?

From slack: Nancy Salpepi 2:51 PM Hi Michael! Quick question about the preferences menu and studio….what settings should be maintained when I open the standard wrapper? For example, extra sounds will stay checked, but interactive highlights won’t.

Michael Kauzmann :house_with_garden: 4:11 PM No, we specifically decided against it. Please make an issue with the confusion/worry and assign to Amy to note, confirm and to close. Also please tag https://github.com/phetsims/joist/issues/744

zepumph commented 2 years ago

We spoke about this during PhET-iO meeting today, and confirmed that "preferences" in general should not be stateful (persisting through to the standard wrapper). General audio (audioManager.enabledProperty) should be stateful. We will make sound and extraSound phetioState:false.

zepumph commented 2 years ago

Just to confirm steps here, please apply this patch and regenerate API files,

Index: js/soundManager.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/soundManager.ts b/js/soundManager.ts
--- a/js/soundManager.ts    (revision ffd8068d7888fdd28420a78fd944253f65b266de)
+++ b/js/soundManager.ts    (date 1663344496555)
@@ -114,11 +114,13 @@

     this.enabledProperty = new BooleanProperty( phet.chipper.queryParameters.supportsSound, {
       tandem: tandem.createTandem( 'enabledProperty' ),
+      phetioState: false, // This is a preference, global sound control is handled by the audioManager
       phetioDocumentation: 'Determines whether sound is enabled.'
     } );

     this.extraSoundEnabledProperty = new BooleanProperty( phet.chipper.queryParameters.extraSoundInitiallyEnabled, {
       tandem: tandem.createTandem( 'extraSoundEnabledProperty' ),
+      phetioState: false, // This is a preference, global sound control is handled by the audioManager
       phetioDocumentation: 'Determines whether extra sound is enabled. Extra sound is additional sounds that ' +
                            'can serve to improve the learning experience for individuals with visual disabilities. ' +
                            'Note that not all simulations that support sound also support extra sound. Also note ' +
samreid commented 2 years ago

I applied the patch and regenerated API files, closing.

samreid commented 2 years ago

Or perhaps a review phase would be good if there's time?

zepumph commented 2 years ago

Yes great thanks.