phetsims / utterance-queue

Alerting library powered by aria-live
MIT License
0 stars 2 forks source link

Replace VoicingManager.speakIgnoringEnabled with a Property controlling speech #69

Open jessegreenberg opened 2 years ago

jessegreenberg commented 2 years ago

It is confusing that the voicingManager.enabledProperty can have a value of false but there can still be speech with speakIgnoringEnabled. A better way would be to provide a separate Property to control speech output while making it so that whenever voicingManager.enabledProperty is false there can be no speech. Here is some scratch illustrating the change we are thinking of.

```patch Index: joist/js/audioManager.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/joist/js/audioManager.js b/joist/js/audioManager.js --- a/joist/js/audioManager.js (revision ddcffa71993bd7e06725b30f5a394124e7cff693) +++ b/joist/js/audioManager.js (date 1634927385639) @@ -135,6 +135,7 @@ sim.isConstructionCompleteProperty, sim.browserTabVisibleProperty, sim.activeProperty, + this.simVoicingEnabledProperty, // passed to the PreferenceDialog, field on AudioManager sim.isSettingPhetioStateProperty, this.audioEnabledProperty ], ( simConstructionComplete, simVisible, simActive, simSettingPhetioState, audioEnabled ) => { Index: scenery/js/accessibility/voicing/voicingManager.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery/js/accessibility/voicing/voicingManager.js b/scenery/js/accessibility/voicing/voicingManager.js --- a/scenery/js/accessibility/voicing/voicingManager.js (revision 414d8cf0bf3cdabc1a9c8e6eb6b7e091ae8369cf) +++ b/scenery/js/accessibility/voicing/voicingManager.js (date 1634927277241) @@ -382,8 +382,8 @@ * * @param {Utterance} utterance */ - speakIgnoringEnabled( utterance ) { - if ( this.initialized ) { + speakIgnoringSpeechAllowed( utterance ) { + if ( this.initialized && this.enabledProperty.value ) { this.requestSpeech( utterance ); } } ```

Likely related to https://github.com/phetsims/joist/issues/743 because that will determine where simVoicingEnabledProperty should live. COuld be on the AudioManager, but maybe it should be on a "PreferencesModel", whatever that ends up looking like.

jessegreenberg commented 2 years ago

This was decided while working on https://github.com/phetsims/gravity-force-lab-basics/issues/303.

jessegreenberg commented 2 years ago

This would happen in utterance-queue now that SpeechSynthesisAnnouncer lives there.

zepumph commented 2 years ago

Could we instead handle this by speaking this before turning it off? That would be in joist. I wonder if that would be nicer than another Property.