phetsims / number-suite-common

"Number Suite Common" contains code for use by sims that are part of the Number Suite
GNU General Public License v3.0
0 stars 0 forks source link

Don't include voiceProperty in readAloud multilink #60

Closed chrisklus closed 1 year ago

chrisklus commented 1 year ago

In NumberSuiteCommonUtteranceQueue, back from working on major changes in https://github.com/phetsims/number-suite-common/issues/47, I could not figure out how to reliably NOT include the announcer's voiceProperty in the multilink that speaks the "Hear..." (readAloud) feature when it's turned on. The only time the voiceProperty changes and we then want to trigger a readAloud is when the Locale toggle switch/radio button is changed. We cannot (should not) link directly to isPrimaryLocaleProperty because then we are relying on an order dependency such that the correct voice has been set before we readAloud. However, there are several place where we are changing the voiceProperty but we don't want to trigger a readAloud, which means there are many workaround guards in place to cancel the speech started from the change.

@zepumph and I revisited this yesterday because we were going to have to add yet another workaround for changes to solve https://github.com/phetsims/number-suite-common/issues/56. @zepumph had the idea to think about this as something that should be powered by input, not a Property change. So, thanks to his good work in https://github.com/phetsims/sun/issues/839 and https://github.com/phetsims/sun/issues/840, we were able to trigger the readAloud feature when changing between primary and secondary locale directly from the UI control. And, the listener we used fires AFTER updating the Property, so no ordering issues with using the correct voice.

chrisklus commented 1 year ago

Changes committed above. This is working really nicely! If any bugs come up from these changes i'll handle them in side issues. Closing!