search

phetsims / utterance-queue

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

Move most voicingManager logic to this repo #34

Closed zepumph closed 2 years ago

zepumph commented 2 years ago

@jessegreenberg and I think it would be an improvement to factor out the basic announcer functionality for general use in utterance-queue. @jessegreenberg mentioned that @chrisklus and him had a use case where it would have been nice to have a way to use speech synthesis without the full, "voicing" scenery feature.

Let's name this factored out type in utterance-queue to SpeechSynthesisAnnouncer

so VoicingManager extends SpeechSynthesisAnnouncer extends Announcer

In another issue we will rename AriaHerald to AriaLiveAnnouncer for consistency.

chrisklus commented 2 years ago

For a paper trail, this was needed over in https://github.com/phetsims/number-play/issues/32

jessegreenberg commented 2 years ago

This is really the way to go to address https://github.com/phetsims/number-play/issues/136, Id like to work on this next.

jessegreenberg commented 2 years ago

First, I need to move voicingManager to utterance-queue and preserve history. Over slack @zepumph showed me copy-history-to-different-repo.sh in perennial. I think it worked, the patch file looks reasonable. I see commits for voicingManager now in the git log of utterance-queue repo. Then I moved voicingManager into the right location in utterance-queue with WebStorm. After that I still see the history for voicingManager. I am going to push this to utterance-queue and then rename utterance-queue/js/voicingManager to utterance-queue/js/SpeechSynthesisAnnouncer. Then I am going to begin refactoring it as a superclass for scenery/js/voicingManager.

jessegreenberg commented 2 years ago

Mostly done with the above commits. Sims are running, SpeechSynthesis is working, unit tests are passing. Trying it out now in number-play.

zepumph commented 2 years ago

Above fixed a regression where voicingManager was not respecting responseCollector Properties by default.

zepumph commented 2 years ago

@jessegreenberg and I think this is ready for review.

zepumph commented 2 years ago

This looks really nice, and I liked seeing how Number Play is using these items. I was curious though if NumberPlay is a prototype, or a pattern. Would you recommend creating such an Announcer if we did this again? Perhaps we need a "MultilingualSpeechSynthesisAnnouncer"?

Feel free to close.

jessegreenberg commented 2 years ago

Yea something like that sounds nice. I am not sure what the goals are for speech synthesis and multi-language use outside of Voicing is at this point so it doesn't seem worth abstracting further at this time. But it would be good to do if there is a desire to use that more.