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 comments accessible: Dealing with comments and suggestions in the inline editor #110408

Closed
daniel-montalvo opened this issue Apr 23, 2020 · 9 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues bug Issue identified by VS Code Team member as probable bug comments Comments Provider/Widget/Panel issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Milestone

Comments

@daniel-montalvo
Copy link

Thanks for the very good work you are doing with this extension!

There seems not to be an apparent way to deal with comments in the inline editor for screen reader users (or I am totally missing it). There is a tree view for changes but not so for comments.

  1. Is there an implemented workflow for inserting, replying, and suggesting using the VSCode inline editor that takes into account screen reading technologies? If so, please point me to some guidance on how to accomplish it.
  2. If not, I would really appreciate you could work on this, as adding, managing, replying, and suggesting could be much easier from an inline local editor than it actually is from within the GitHub web interface for reviews.
@alexr00 alexr00 transferred this issue from microsoft/vscode-pull-request-github Nov 11, 2020
@alexr00 alexr00 changed the title Dealing with comments and suggestions in the inline editor Make comments accessible: Dealing with comments and suggestions in the inline editor Nov 11, 2020
@alexr00 alexr00 added accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues comments Comments Provider/Widget/Panel issues labels Nov 11, 2020
@rebornix
Copy link
Member

rebornix commented Aug 9, 2021

@isidorn will give it a go through and provide feedback and suggestions for how we can move forward.

@rebornix rebornix added this to the August 2021 milestone Aug 9, 2021
@isidorn
Copy link
Contributor

isidorn commented Aug 9, 2021

I just gave it a quick spin. Here are things which I think we should improve:

  • Similar to the F8, Shift + F8 error experience (user can press F8 to navigate through each error), we need to introduce a command with a keybinding to focus onto the next and previous comment in the file. Triggering this command would move focus to the comment box. I see that there is Go To Next Comment Thread command but that one does not move focus inside the comment box. And there is no Go to Previous.
  • When the focus is in the comment box Up and Down navigation should intuitively move focus

Once we have that ideally @daniel-montalvo can try it out and provide more feedback
Also let me know if you think this proposal makes sense

fyi @alexr00 for thoughts

@venkateshpotluri
Copy link

Hi, while my experience with the comment functionality that is being discussed here is limited, it is exciting to see this experience potentially improving. Are the comments in question here the discussion on a PR or the code review comments? I like the idea of a similar experience as errors. However, if this experience is for in-line code review comments, I found the part of the proposal

Triggering this command would move focus to the comment box.
slightly confusing as I was not sure if moving the focus to the comment box (assuming the user invokes the command from the code) would be desirable behavior as this may require additional steps if the user wishes to go back to code and jump between comments. Here is a potential proposed approach that replicates the errors experience that would not move the focus of the user's cursor from the code:

  1. user goes through code in the editor.
  2. user presses the key-binding for this command.
  3. similar to the errors experience, The cursor moves to the next comment, and announces the author, line number or a range of line numbers(if the comment is a multi-line comment), the number of comments in the thread, and the actual comment.
  4. user can invoke the jump to comments view command (this can be bound to a different keystroke) to go to the comment's node in the comment view (this could be a view similar to the problems view). user can tab to find different options relevant to the comment (this depends on the functionality supported).
  5. For threaded comments, the first comment in the comments view could be an expandable treeview node (similar to each file in the problems view) with the comments in the thread as it's child.

@daniel-montalvo
Copy link
Author

Thanks for that detailed proposal @venkateshpotluri

I think that makes a lot of sense. As it is consistent with other user interactions.

@isidorn Happy to try this out as you develop it, and also very happy that you are giving this a thought.

@isidorn
Copy link
Contributor

isidorn commented Aug 10, 2021

@venkateshpotluri @daniel-montalvo great, thanks a lot for the feedback. We seem to be aligned here, and I agree that once the comment is focused all those details you mentioned should be read out (author, line number, range, content).
Focusing the comments view should be already possible, and the threaded comments interaction makes sense.

@rebornix we could achieve this by having a nice aria-label on the comment box that contains all these details, and once user presses tab or down we would focus on the input which contains the actual comment and then that would be read.
Anyways let me know when you start looking into this and I can provide more feedback and ideas if needed.

@venkateshpotluri
Copy link

Great! happy to try this out and provide further input as appropriate.

@rebornix rebornix modified the milestones: August 2021, September 2021 Aug 20, 2021
@rebornix rebornix modified the milestones: September 2021, October 2021 Sep 29, 2021
@rebornix rebornix added the bug Issue identified by VS Code Team member as probable bug label Oct 12, 2021
@rebornix rebornix modified the milestones: October 2021, On Deck Oct 26, 2021
@egamma egamma modified the milestones: On Deck, March 2022 Mar 7, 2022
alexr00 added a commit that referenced this issue Mar 18, 2022
@alexr00
Copy link
Member

alexr00 commented Mar 18, 2022

@venkateshpotluri and @daniel-montalvo I've made several changes to the comments widget accessibility:

  • Added a command for creating a comment if you're in a commentable range
  • Added an aria label for comment threads, which includes the number of comments and the label of the thread
  • Updated the "Go to Next Comment Thread" command to focus into the comment thread widget
  • Added a "Go to Previous Comment Thread" command
  • Added keyboard shortcuts for both commands: alt + F9 and shift + alt + F9

All changes should be available in VS Code insiders on Monday, and it would be great to get your feedback. I'm closing this issue so that it gets included in our release testing, not because I'm sure we've nailed the accessibility support of comments. We can always open more issues for further improvement!

@isidorn
Copy link
Contributor

isidorn commented Mar 18, 2022

@alexr00 thanks a lot for doing this 👏

fyi @neeleshb since I know you are looking into accessibility and comments flow is important for your team.

@daniel-montalvo
Copy link
Author

That's great news @alexr00

Thanks a lot!

I will keep an eye on the Insiders channel and will get back to you with feedback.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues bug Issue identified by VS Code Team member as probable bug comments Comments Provider/Widget/Panel issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

8 participants
@egamma @rebornix @isidorn @Tyriar @venkateshpotluri @alexr00 @daniel-montalvo and others