phetsims / utterance-queue

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

voiceProperty handled very strangely with ?voicingInitiallyEnabled #102

Closed zepumph closed 1 year ago

zepumph commented 1 year ago

Looks like the voiceProperty isn't set on startup

@jessegreenberg, what about a patch like this, instead of relying on the preferences Dialog code?


Subject: [PATCH] blarg
---
Index: js/SpeechSynthesisAnnouncer.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/SpeechSynthesisAnnouncer.ts b/js/SpeechSynthesisAnnouncer.ts
--- a/js/SpeechSynthesisAnnouncer.ts    (revision 9e7c5bb60be20c882d83995f7fe32efee8f4526a)
+++ b/js/SpeechSynthesisAnnouncer.ts    (date 1675275047691)
@@ -473,7 +473,12 @@
     if ( synth ) {

       // the browser sometimes provides duplicate voices, prune those out of the list
-      this.voicesProperty.value = _.uniqBy( synth.getVoices(), voice => voice.name );
+      const voices = _.uniqBy( synth.getVoices(), voice => voice.name );
+      this.voicesProperty.value = voices;
+
+      if ( this.voiceProperty.value === null && voices.length > 0 ) {
+        this.voiceProperty.value = voices[ 0 ];
+      }
     }
   }
jessegreenberg commented 1 year ago

It is kind of weird it is set in joist code, but also makes sense of the PhET specific voice choices and because the voice is tied to the available voices in the joist UI component to select voice. voicesProperty also includes many non-english voices.

I ended up refactoring the filter for english voices, then adding a bit where ?voicingInitiallyEnabled is used. What do you think?

zepumph commented 1 year ago

This is looking really nice. Thank you for the commit that is actually in the right spot (as opposed to my patch). Nothing else here for me!