Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure warmer alerts are not interrupting break away alerts #242

Closed
zepumph opened this issue Sep 24, 2021 · 14 comments
Closed

Make sure warmer alerts are not interrupting break away alerts #242

zepumph opened this issue Sep 24, 2021 · 14 comments

Comments

@zepumph
Copy link
Member

zepumph commented Sep 24, 2021

I think I will need @jessegreenberg's help with this. What if we added and announcer option like important:true, which overrides if something else is going to interrupt it.

@zepumph
Copy link
Member Author

zepumph commented Sep 24, 2021

@jessegreenberg and I started on phetsims/scenery#1287 and tested by making the break away utterance priority 5, and the temp increasing utterance priority 3. It was working pretty well until we found phetsims/scenery#1288. I will start there once I work on this again.

@zepumph
Copy link
Member Author

zepumph commented Sep 24, 2021

One other thing is that we don't want to use addToFront here anymore, instead priority will make more sense. Adding to front doesn't mean that we release an alert first, later alerts were being announced earlier because they had been in the queue longer. Instead let's do this patch:

Index: js/friction/view/BreakAwayAlerter.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/friction/view/BreakAwayAlerter.js b/js/friction/view/BreakAwayAlerter.js
--- a/js/friction/view/BreakAwayAlerter.js	(revision d96681578de0f4865bdc7cf444d2c99980f19e54)
+++ b/js/friction/view/BreakAwayAlerter.js	(date 1632522545836)
@@ -89,9 +89,10 @@
 
     this.utterance.alert.contextResponse = alertContent;
 
-    this.forEachUtteranceQueue( utteranceQueue => utteranceQueue.addToFront( this.utterance ) );
+    this.forEachUtteranceQueue( utteranceQueue => utteranceQueue.clear() );
+    voicingUtteranceQueue.clear();
 
-    voicingUtteranceQueue.addToFront( this.utterance );
+    this.alert( this.utterance );
 
     this.alertedBreakAwayProperty.value = true;
     this.tooSoonForNextAlert = true;

@zepumph
Copy link
Member Author

zepumph commented Sep 30, 2021

After the above commit, I am able to always hear the entire break-away alert, no matter what. @terracoda and @Matthew-Moore240, will you please review this issue to make sure the break away alert, and all the alerts leading up to it/after it, sound correct to you?

@zepumph
Copy link
Member Author

zepumph commented Sep 30, 2021

Please note that we don't have the ability to prioritize this so heavily with description/screen reader support. Thus, it is still quite easy to hear a break away alert interrupted by another alert, like warming or cooling. Noted because of testing in #243.

@terracoda
Copy link
Contributor

In voicing warming responses are not interrupting break aways, but I am not hearing any gradual warming responses,. I hear "Very hot. Atoms break away." and then the gradual cooling responses.

Using VoiceOver, I am hearing the gradual warming responses and I am hearing the break aways, and I am hearing the gradual cooling responses, too.

Not sure why very few gradual warming responses are firing for voicing?

@terracoda terracoda removed their assignment Oct 4, 2021
@terracoda
Copy link
Contributor

terracoda commented Oct 4, 2021

I was testing using the keyboard. When I use a mouse, I hear gradual warming responses, breakaways and gradual cooling responses woven together more frequently. It might just be a keyboard issue.

Gradual warning responses should be able to fire during active dragging without a release.

@terracoda
Copy link
Contributor

@Matthew-Moore240, please describe what you find when you test.

@terracoda
Copy link
Contributor

And maybe work in #241 will resolve any further concerns I have.

@zepumph
Copy link
Member Author

zepumph commented Oct 4, 2021

This sim was published with these alerts only available with keyboard on end drag. This didn't seem to be a problem in previous conversations about the keyboard because most non visual users would likely use the keyboard sparingly, and with short presses.

Now that we are working on voicing , over in #233, we noted that updating based on the granularity of "end drag" is not enough, so now it occurs more often for mouse. If you would like me to include that same logic for keyboard, I'm happy to. I'll go ahead and commit that change and we can see if that is the direction we like for keyboard and mouse/voicing and description.

@Matthew-Moore240
Copy link

With Mouse:
I hear one breakaway alert but then I get six "super hots" in a row while the thermometer is at the bottom followed by three "cooling" alerts. This seems like an extreme pile-up of alerts.

With Keyboard:
I get the exact same issues as with a mouse.

@zepumph
Copy link
Member Author

zepumph commented Oct 5, 2021

With Mouse:
I hear one breakaway alert but then I get six "super hots" in a row while the thermometer is at the bottom followed by three "cooling" alerts. This seems like an extreme pile-up of alerts.

This will be handled over in #253. I am working on the timing of the "maximum temperature alert" and also the warming alerts in general.

@zepumph
Copy link
Member Author

zepumph commented Oct 6, 2021

Yes that will be handled in #253. I think all that is left for this issue is to make sure that for keyboard and mouse, we get enough warming alerts at the right times. I want to do some experimentation to see if it is easy to get keyboard to work beyond in just endDrag.

@zepumph
Copy link
Member Author

zepumph commented Oct 7, 2021

After looking into this more for keyboard, I think it makes sense the way it is. There is no easy way to update this logic based on timing during the drag. Is it acceptable to only hear this on endDrag @terracoda?

@terracoda
Copy link
Contributor

For mouse input warming responses are not interrupting break-aways. This issue has been resolved.

In fact, with mouse input I am hearing a nice mix of all responses..warming when rubbing and cooling when not rubbing...warming again when rubbing more then very hot then break away.

With alt keyboard input, warming responses are not interrupting break-aways either, but with alt keyboard input I rarely, if ever, hear warming responses.

I mentioned this over in #253.

Closing this issue and opening a new one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants