Closed jessegreenberg closed 3 years ago
https://github.com/phetsims/utterance-queue/commit/325044a5a42e58f8d64c80bf7efc5a56c7887691 is really the commit that implemented this.
To summarize changes -
announcer
, which implements an announce
function. The announce
function requests speech in some form (aria-live or web speech). This param is required.Everything else is pretty much the same. One thing I really wasn't sure about was the PhET-iO portion in utteranceQueue. TODO was added in the code. @zepumph can we review this today during meeting?
I noticed that https://github.com/phetsims/utterance-queue/commit/325044a5a42e58f8d64c80bf7efc5a56c7887691 broke the a11y view because you got rid of getAriaLiveContainer
. I know why and how, but it seems like we need to put a bit more thought into how we want that piece of the API to work. Maybe Display needs to take a bit more responsibility in managing AriaHerald, since we may make UtteranceQueue without a default announcer.
As it pertains to UtteranceQueue as a potential third-party library, it would be nice if we could make AriaHerald a default announcer. That may clear up the API in that case. I understand though that managing HTML elements really goes beyond the scope of Utterance queue. Maybe we could have UtteranceQueue implement an internal AriaHerald as the default announcer, and turn it into an option instead of a parameter. And if announcer was a public member (which it should be I think), then Display can just grab the elements as needed:
this.utteranceQueue = new UtteranceQueue( ariaHerald, !this._accessible );
const elements = this.utteranceQueue.announcer.ariaLiveContainer;
We can discuss more though. What you have may be better though.
I created to https://github.com/phetsims/utterance-queue/issues/17 to specifically deal with the commented out phet-io event in utteranceQueue. Updating the todo now.
Review comment I found while working on https://github.com/phetsims/scenery/issues/1183:
* @param {Object} announcer
parameter in UtteranceQueue, can we make an Announcer.js
abstract base type that outlines the interface that UtteranceQueue. This will give us a place to document this (with @abstract methods that can throw an assertion error until overridden), and also allows room for expanding if we want to increase the complexity of the Announcer
interface.@jessegreenberg what is left for this issue? We have been using WebSpeaker in UtteranceQueue for a while now.
- can we make an
Announcer.js
abstract base type that outlines the interface that UtteranceQueue.
I thought this would be nice while working on https://github.com/phetsims/utterance-queue/issues/20, I think I'll take a stab at that.
I made an Announcer
base class, I had to swap EnabledComponent in VoicingManager to be composed to make it work. @jessegreenberg can you please review. Anything else here?
I reviewed the last comments in the issue as well as the new Announcer base class. I think this can be closed.
I would like to be able to queue up web speech much like we do with utterance-queue. Can the queue be extended to support more than just output with aria-herald?
I brainstormed briefly with @zepumph and we discussed creating a way change the output method of utterance-queue with an option so that you could have one instance for aria and another instance for speech synthesis.
I am going to try it out.