phetsims / utterance-queue

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

Hello world examples are not working. #24

Closed zepumph closed 2 years ago

zepumph commented 2 years ago

While reviewing ways to go for https://github.com/phetsims/utterance-queue/issues/23, I found that our examples from the README don't work. This is bad and embarrasing in general, but it also makes me feel like I don't fully understand how UtteranceQueue works, or if there is a little bug I'm forgetting vs a big bug that we need to know about for these simple cases.

jessegreenberg commented 2 years ago

I just looked into this (as a way to potentially test another change). Oddly, things only seem broken in Chrome. Alerts in the examples are coming through in firefox. Also, I see the AriaHerald elements updating in the document in Chrome. The AT just isn't reading them with Chrome + NVDA.

Alerts are working in sims with Chrome + NVDA though.

jessegreenberg commented 2 years ago

If I comment out this line, alerts work in Chrome: https://github.com/phetsims/utterance-queue/blob/ec3ed28d5bbae31224ce24487ee48d5820d84832/js/AriaHerald.js#L183

jessegreenberg commented 2 years ago

https://github.com/phetsims/scenery-phet/issues/491 Is the related issue for the values used in that timeout.

jessegreenberg commented 2 years ago

I changed the timeout code to

        else {
          console.log( 'clearing' );
          liveElement.textContent = '';
        }
      }, 5000 );

But I see the element and 'clearing' in the console instantly instead of behind a 5000 ms delay. Curious if there is an issue with the steptimer, like it thinks more time has passed than actually has to instantly call the timeout callback.

jessegreenberg commented 2 years ago

The issue for https://github.com/phetsims/utterance-queue/issues/24#issuecomment-905018500 is happening because the arg for the requestAnimationFrame callback is not the elapsed time in ms, but the total time relative to the start of the document lifetime.

Changing the example code involving the stepTimer to this fixes https://github.com/phetsims/utterance-queue/issues/24#issuecomment-905018500 and the original issue when using NVDA and chrome.

  // step phet.axon.stepTimer (in seconds) each frame. This takes care of UtteranceQueue's timing
  let previousTime = Date.now();
  const step = elapsedTime => {
    const dt = elapsedTime - previousTime;
    previousTime = elapsedTime;

    phet.axon.stepTimer.emit( dt / 1000 );
    window.requestAnimationFrame( step );
  };
  window.requestAnimationFrame( step );
jessegreenberg commented 2 years ago

The above commit fixes the second example with Chrome + NVDA. I still am not hearing the alert for the first example. I am finding I need to do this to get it to work in Chrome.


<script src="../sherpa/lib/lodash-4.17.4.js"></script>
<script src="../utterance-queue/build/utterance-queue.min.js"></script>
<script>
  const utteranceQueue = phet.utteranceQueue.UtteranceQueue.fromFactory();

  // wait until the page has finished loading before sending the first alert
  window.setTimeout( event => {
    utteranceQueue.addToBack( 'hello world' );
  }, 2000 );

</script>

@zepumph what do you think, worth making the example more complicated so it works on Chrome? Maybe there is another workaround or even example that would be better for a basic use case that doesn't have this issue.

zepumph commented 2 years ago

You had it really close, but the previous time should be relative, not absolute. I fixed this in the above, commit, and saw that things were working as expected in the examples. Please review.

jessegreenberg commented 2 years ago

Got it, good catch, thanks! Working well, closing.