microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.83k stars 29.5k forks source link

Play audio-cues for suggest widget #147230

Open jooyoungseo opened 2 years ago

jooyoungseo commented 2 years ago

CC @isidorn

System Info

Version: 1.67.0-insider (system setup) Commit: b84feecf9231d404a766e251f8a37c730089511b Date: 2022-04-11T05:15:52.676Z Electron: 17.4.0 Chromium: 98.0.4758.141 Node.js: 16.13.0 V8: 9.8.177.13-electron.0 OS: Windows_NT x64 10.0.22000

Reproducible steps

  1. Enable Audio Cues Line Has Inline Suggestion.
  2. Create test.py.
  3. Type i to trigger auto-suggestion for import.
  4. See if you can hear any audio cues for Line Has Inline Suggestion: I don't hear anything.
isidorn commented 2 years ago

Thanks for creating this issue, and for giving inline cues a try. I think the inline suggestion accessibility flow is not super polished, so let me first assign to @heidet to comment

hediet commented 2 years ago

The inline suggestion cue works only for inline suggestion providers, such as copilot or the new inline quick suggest.

It is not designed to work for auto-suggestions. @isidorn should Joh add an audio cue for auto-suggestions?

isidorn commented 2 years ago

@hediet I think no action needed for now, but fyi @jrieken as this is a good accessibility test case for the inline suggestion. And we should probably think more about this.

@jooyoungseo I suggest to not use inline suggestions, but the suggest widget since it should be much more accessible until we polish this experience.

jooyoungseo commented 1 year ago

@meganrogge -- I had already created an issue here related to #174606.

meganrogge commented 1 year ago

This seems like a good request, but to clarify, these are not inline-completion suggestions. These are coming from the suggest widget

jooyoungseo commented 1 year ago

@meganrogge -- Yes, but a UX design question is whether we want to make them different for screen reader users. From a blind end-user's perspective, they are pretty similar.

Aw, one more thing. Please correct me if I am wrong, but it seems like suggestion widget is also using div.monaco-tokenized-source just like copilot because the aria-live alert for suggestion widget was also impacted by this when I tested this.

meganrogge commented 1 year ago

I think that's a very good question to ask.

Re the dom elements, I tested the changes in your PR that is linked to this issue and it did not work for me, which makes me think the code has changed since you created that.

meganrogge commented 1 year ago

To recap, we want to remove the Suggestion prefix from the suggestion box screen reader content and replace it with an audio cue. Since arrow key navigation is required for this type of suggestion, we will have a slightly different sound for this one.

@isidorn would appreciate your thoughts here. should we loop in the sound designer?

isidorn commented 1 year ago

Is the Suggestion prefix being read for every suggestion or just when the suggest widget is shown? If the first, then we should indeed remove it.

I would like to understand what would be the benefit of adding audio cues here. Since the Suggest Widget appears on screen, and focus moves to it I think the screen reader nicely reads it's content. What is missing from the current experience?

meganrogge commented 1 year ago

@isidorn as I type, I can visually see the suggestions have been updated, but the screen reader does not read them.

jooyoungseo commented 1 year ago

@isidorn and @meganrogge

Sighted people experience: visually (non-verbally) see suggestion popup and read (verbally) suggestion tokens.

Current design for screen reader users: [verbal cue: "suggestion box"] -> suggestion_token: sequential via a verbal channel only.

Improved design: [non-verbal audio cue] <-> suggestion_token: parallel/faster/more discernible using verbal and non-verbal channel at the same time.

jooyoungseo commented 1 year ago

One may wonder why we would need audio cue because simply removing the suggestion prefix would seem to suffice. If we remove the suggestion prefix, there is no contextual hint for screen reader users whether the currently being read token is from their typing or auto suggestion widget. We may want to signal the popup with the audio cue.

meganrogge commented 1 year ago

Yes that makes sense. So we will play the audio cue when there are suggestions for the first time or when they get updated. @isidorn what do you think of this?

AFre100 commented 1 year ago

@meganrogge was revisiting our standup meeting recording, I came across something that I was hoping you could clarify...

When asked about your take on the risk-benefit ratio for updating an existing sound w/an improved one, your take was "what's most important is the user knowing how to navigate between these things, and they already do, so a new. better sound would be welcome."

Did you mean they know how to navigate:

a) the features as concepts themselves (not the sounds reinforcing them), already (meaning they'd figure out how to relearn an updated audio cue because they're well grounded in the feature as a north star concept), or

b)the audio cue options/settings w/in VSC (meaning they could simply use the menu of sounds in settings in order to become aware of updated sounds, and/or configure/toggle the sound?

thanks!

meganrogge commented 1 year ago

When a user runs a command, they hear the content and an audio cue. Changing the audio cue does not change their workflow - they will still run the same command and hear the same content, only now a better audio cue. And yes, also the audio cue's function/name has not changed, so if they knew how to configure it before, they still know how to do so now.

AFre100 commented 1 year ago

@meganrogge and if it's a sound a particular user didn't necessarily ask to be changed, which sounds significantly different from the og sound and is now the default cue, do you think it makes more sense to totally omit the og sound, or would it still be an option to use w/configuration?

meganrogge commented 1 year ago

We have decided a better default, so it does not make sense to have a setting for this. I view this change as more of a bug fix than a feature request. @isidorn do you agree?

isidorn commented 1 year ago

@meganrogge I agree.

jooyoungseo commented 1 year ago

@AFre100 and @meganrogge -- NVDA has really great examples that can be used for suggestion widget audio cues.

jooyoungseo commented 1 year ago

@meganrogge and @AFre100 -- this is a critical issue on Mac. I have noticed that suggestion widget (ctrl+space) doesn't even prompt verbal message such as "suggestion widget" on Mac for VoiceOver. Implementing audio cues for opening and closing suggestion widget will mitigate this issue.

meganrogge commented 1 year ago

@hediet @jrieken can you look into this soon? It's bad a SR user isn't informed when the suggest widget appears.

meganrogge commented 1 year ago

@jooyoungseo your original steps to repro involved inline suggestions. that is working for me currently. what does not currently have an audio cue is when the suggest widget appears.

jrieken commented 1 year ago

@hediet @jrieken can you look into this soon? It's bad a SR user isn't informed when the suggest widget appears.

Unlikely happening soon because this is feature work which needs some planning and prep, like getting the audio designer to work. Tho, if there is some kind of bug with the existing behaviour (like missing aria properties) we can maybe look into them, e.g treat them as bug

meganrogge commented 1 year ago

cc @AFre100 we have an inline suggestion audio cue already - this would be one that is slightly different, though related.

meganrogge commented 1 year ago

the aria labels are working fine for me, though I can imagine why users would want an audio cue so it's very clear where this suggestion is coming from

jooyoungseo commented 1 year ago

@meganrogge After hitting ctrl+space on Mac, it does not say something like "suggestion" immediately. I have to press DownArrow to hear the suggested items.

meganrogge commented 1 year ago

I was confused and had a different keybinding. When I use the keybinding, it reads the first suggestion to me. I'm still testing it to see if I can observe what you're mentioning.

meganrogge commented 1 year ago

Okay can you please try with Cmd+I? That is the command Trigger Suggest and is working for me.

When I try Ctrl+Space, nothing happens.

jooyoungseo commented 1 year ago

cmd+I is opening Copilot in-editor for me.

meganrogge commented 1 year ago

I have no custom keybindings. Perhaps you could send me yours?

meganrogge commented 1 year ago

@jrieken could you please advise here? any idea why ctrl+space is doing nothing for me while cmd+I works when they have the same when clauses? in the keybindings editor, I search trigger suggest. three entries for the same command appear - each when clause is identical

rperez030 commented 1 year ago

any idea why ctrl+space is doing nothing for me while cmd+I works when they have the same when clauses?

In macOS, I'm using Option +Escape to trigger the suggest widget because I use CTRL +Space to switch keyboard language, so it doesn't work for me.

When I trigger the widget manually on Mac, I hear the first suggestion followed by position information (E.G. Import os, 1 of 6). The same happens if the widget gets triggered as I type. That is enough for me, but I'm also someone who doesn't like to be bombarded as I type. I prefer to trigger the widget manually if I need it, but we all process information in different ways. I can understand that @jooyoungseo wants a more distinctive signal that the widget has opened / closed if that if something he relies upon. I wonder though whether this deserves a larger survey, especially if we want a new audio cue terming on by default for those of us who use screen readers. between screen reader announcements, audio cues and accessibility hints, the default experience is starting to feel slightly overwhelming. My hypothesis is that casual users are less likely to spend the time require to customize the editor, and a casual user who is annoyed by the experience to begin with is less likely to discover the many benefits that the editor has to offer in terms of accessibility and usability.

meganrogge commented 1 year ago

@isidorn could we do a survey to get some more opinions?

isidorn commented 1 year ago

any idea why ctrl+space is doing nothing for me while cmd+I works when they have the same when clauses?

You probably have something else binded to ctrl+space that is taking priority. I suggest to put a breakpoint around here https://github.com/microsoft/vscode/blob/fceb5c1f72ab8a70d302d7bf9f00dcf96f4b4e21/src/vs/workbench/services/commands/common/commandService.ts#L50 and investigate what command gets executed

could we do a survey to get some more opinions?

Can you clarify - survey about what? Thanks!

meganrogge commented 1 year ago

A survey about if we should add an audio cue when the suggest widget is shown

meganrogge commented 1 year ago

It does not hit that breakpoint - very odd

rperez030 commented 1 year ago

It does not hit that breakpoint - very odd

Could it be that you have something running in the background that grabs that keystroke before it can make it to VS Code?

AFre100 commented 1 year ago

@meganrogge @jooyoungseo I’m OOO for a bunch of august, but will queue this cue up! (It was already on my backlog after some conversations with JooYoung)

AFre100 commented 1 year ago

Jooyoung mentioned "Implementing audio cues for opening and closing suggestion widget will mitigate this issue."

Hi @meganrogge @jooyoungseo, will one suggestion widget sound provide all the needed feedback for SR users, or does it need to be two related sounds (1 "open" and 2 "closed" version)?

Can you please confirm this flow question -- the sound is only triggered when opening/closing the widget or clicking on it, right? It's not alerting the user the first moment that it's available, right?

Megan, might you send me a rough screen recording of this flow in action (even w/your phone, if it's faster), w/some indication of exactly where said sound(s) would potentially go, that'd be super helpful...I think that'd save time on rounds/corrections.

meganrogge commented 1 year ago

My computer is having trouble processing gifs ATM, so I cannot give you a time stamp for these, but we'd play the open sound immediately when the widget is shown. We'd play the close sound when it is hidden.

Note that this will be very noisy given it co-exists with inline suggestion audio cue. I'm not sure if there's a way around this.

Recording 2023-09-18 at 14 07 56 (2)

AFre100 commented 1 year ago

Thanks! Do you know if the inline suggestion cue happens in real time while you’re still on the line, or only when you click on a given line w/a suggestion? For me it was only when I clicked on one.

On Mon, Sep 18, 2023 at 5:12 PM Megan Rogge @.***> wrote:

My computer is having trouble processing gifs ATM, so I cannot give you a time stamp for these, but we'd play the open sound immediately when the widget is shown. We'd play the close sound when it is hidden.

Note that this will be very noisy given it co-exists with inline suggestion audio cue. I'm not sure if there's a way around this.

[image: Recording 2023-09-18 at 14 07 56 (2)] https://user-images.githubusercontent.com/29464607/268775542-839008a4-866f-48dc-86bd-f1ae66d818ea.gif

— Reply to this email directly, view it on GitHub https://github.com/microsoft/vscode/issues/147230#issuecomment-1724430680, or unsubscribe https://github.com/notifications/unsubscribe-auth/AZ3VLGWK5FXAKJRT6JWLFR3X3C2MHANCNFSM5TDSCCUQ . You are receiving this because you were mentioned.Message ID: @.***>

meganrogge commented 1 year ago

The in-line suggestion happens in real time