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

Voicing doesn't describe action that interrupts Quick Info statements #326

Closed
Tracked by #899
Nancy-Salpepi opened this issue Jan 25, 2023 · 12 comments
Closed
Tracked by #899

Comments

@Nancy-Salpepi
Copy link

Test device
MacBook Air (m1 chip)

Operating System
13.1

Browser
safari and chrome

Problem description
For phetsims/qa#886, after pressing a Quick Info button, if I press another button or grab the book (without moving it), the statement being uttered is interrupted but I don't hear anything about the action I just took. For example, if I pressed Reset All, I don't hear "Reset All. Everything Reset."

I looked at other sims with Voicing and older versions of Friction and found:
This issue was not present in : Friction dev.23, published GFLB, published JT
This issue is present in: published RaP, in all sims with Voicing in master.

Was this an intentional change that was made at some point?

Steps to reproduce

  1. In Preferences, enable Voicing
  2. Press Overview button
  3. While Voicing is speaking, press the Reset All button (or grab book without moving it, or press Hide Toolbar button)

Visuals

IMG_7842.mov
Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Friction‬ URL: https://phet-dev.colorado.edu/html/friction/1.6.0-rc.1/phet/friction_all_phet.html?printVoicingResponses Version: 1.6.0-rc.1 2023-01-17 16:32:22 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.2 Safari/605.1.15 Language: en-US Window: 1424x710 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.0) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
@jessegreenberg
Copy link
Contributor

I thought we got to the bottom of this, maybe in phetsims/joist#846 or phetsims/joist#752. Ill try to find some history.

@jessegreenberg
Copy link
Contributor

Yes, this issue is exactly phetsims/joist#846. At the end of that issue we added a fix for the "Sim Voicing" toggle switch, but it was noted as a general problem in phetsims/joist#846 (comment).

In phetsims/joist#846 (comment) it was noted

Please note this issue is blocking the publication of Friction. I don't think @jessegreenberg and I have a direction forward just yet.

In phetsims/joist#846 (comment) I brainstormed some ways forward. None of the ideas are good and all would take significant time.

I would recommend accepting this behavior for the publication of Friction and noting it as a known issue, but lets review next week.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jan 31, 2023

We identified that we do not need the call to voicingUtteranceQueue.cancel in VoicingToolbarItem. This can all be removed.

https://github.com/phetsims/joist/blob/7b833a663f0c735e5e4a254ace9a1a693a5835ef/js/toolbar/VoicingToolbarItem.ts#L222-L234

This is safe to remove because priorityProperty will handle managing the UtteranceQueue for us. We reviewed phetsims/joist#752 and phetsims/joist#846 (particularly comment phetsims/joist#752 (comment)) and believe this change is very safe.

@jessegreenberg
Copy link
Contributor

Next steps are to remove that whole block of code and revert any changes related to the muteSwitchUtterance from phetsims/joist#846.

@zepumph
Copy link
Member

zepumph commented Feb 1, 2023

Working on this now.

@zepumph
Copy link
Member

zepumph commented Feb 1, 2023

I committed what @jessegreenberg explained above. @Nancy-Salpepi can you please confirm this is fixed on master, and please also test interrupting the quick info with the sim voicing off toggle (which we want to confirm is still fixed from phetsims/joist#846).

Everything was sounding great to me in friction. When interrupting a quick-info button, I heard: reset all button, grabbed book, and sim voicing toggle.

zepumph added a commit to phetsims/joist that referenced this issue Feb 1, 2023
@Nancy-Salpepi
Copy link
Author

This is working very nicely now in master! I hear everything I should--sim voicing off, reset all button, toolbar hidden, book grabbed, and everything in the nav bar.

@jessegreenberg
Copy link
Contributor

Thanks @zepumph , phetsims/joist@5b7dcff looks nice.

jessegreenberg pushed a commit to phetsims/joist that referenced this issue Feb 8, 2023
@jessegreenberg
Copy link
Contributor

Pushed to joist friction-1.6 branch. I tested in the SHAs and it is working, ready to verify.

@pixelzoom
Copy link
Contributor

@jessegreenberg You added ready-for-review, but no assignee to review. So assigning to you.

@jessegreenberg
Copy link
Contributor

This is in the QA process and is being reviewed in in the next RC.

@jessegreenberg jessegreenberg removed their assignment Feb 10, 2023
@Nancy-Salpepi
Copy link
Author

This is working nicely in rc.3. Closing.

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

4 participants