search

phetsims / utterance-queue

Alerting library powered by aria-live
http://scenerystack.org/
MIT License
1 stars 2 forks source link

Remove default TinyProperty from Utterance.canAnnounceProperties #79

Closed jessegreenberg closed 2 years ago

jessegreenberg commented 2 years ago

This Property is always there, when you add a new Property it doesn't ever change or remove this default. Having it there is convenient for the implementation of the canAnnounceProperty because the DerivedProperty needs at least one dependency. But it makes an assertion like this (maybe to be added to Voicing.ts) irrelevant because the length is always greater than zero

  assert && assert( utterance.canAnnounceProperties.length > 0, 'canAnnounceProperties required, this Utterance might not be connected to Node in the scene graph.' );
jessegreenberg commented 2 years ago

This assertion was added to Voicing.ts as part of https://github.com/phetsims/scenery/issues/1403 so it would be nice to improve this.

jessegreenberg commented 2 years ago

This was done in the above commit. After the change I immediately found an Utterance that was not attached to the scene graph for Voicing which is great.

jessegreenberg commented 2 years ago

Sims passed local fuzzing. I am going to go ahead and close this one but feel free to review if you wish as part of https://github.com/phetsims/scenery/issues/1403 @zepumph.