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

provide screen reader with inline suggestions #175823

Merged
merged 10 commits into from
Mar 7, 2023
Merged

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Mar 2, 2023

@meganrogge meganrogge marked this pull request as draft March 2, 2023 00:46
@meganrogge meganrogge self-assigned this Mar 2, 2023
@meganrogge meganrogge added this to the March 2023 milestone Mar 2, 2023
@jooyoungseo
Copy link

@meganrogge -- The best way for me to test this feature would be after it is released to insider because I don't know how to trigger inline suggestions like copilot in dev version. I usually test the inline suggestion with copilot extension, but as you know, I cannot use any extensions in dev version.

@meganrogge
Copy link
Contributor Author

You can load the extension by downloading and loading the .vsix file, but I am able to test this so can proceed based on that

@meganrogge meganrogge marked this pull request as ready for review March 6, 2023 19:54
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're probably best off getting a review from someone familiar with monaco for this one

@meganrogge meganrogge requested a review from hediet March 6, 2023 20:40
@blaise80
Copy link

blaise80 commented Mar 6, 2023 via email

@meganrogge
Copy link
Contributor Author

@hediet when I tried this, I didn't see an issue with audio cues/ debouncing. Could you pls suggest the repro steps for the issue you mentioned in the other PR and/or what you'd suggest?

@meganrogge meganrogge requested a review from hediet March 7, 2023 15:51
@meganrogge meganrogge enabled auto-merge (squash) March 7, 2023 19:13
@meganrogge meganrogge dismissed Tyriar’s stale review March 7, 2023 19:14

discussed with Henning and JooYoung the points of concern

@meganrogge meganrogge merged commit dcebcb6 into main Mar 7, 2023
@meganrogge meganrogge deleted the merogge/ariaAlert2 branch March 7, 2023 19:14
@meganrogge
Copy link
Contributor Author

meganrogge commented Mar 8, 2023

@jooyoungseo this is available for testing in insider's with editor.screenReaderAnnounceInlineSuggestion: true - be aware if you update at the moment though, there are some issues with the terminal accessible buffer

#176219
#176197

@jooyoungseo
Copy link

jooyoungseo commented Mar 8, 2023

@meganrogge -- Thanks! I have tested this morning and have found some issues, but hard to explain. I will take some time next week to file an issue. To cut To the chase, the suggestion live event is read aloud twice (tested with NVDA).

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

Successfully merging this pull request may close these issues.

6 participants