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

[Accessibility] Expose all available actions to command palette, such as review start and approve #5527

Closed
Tracked by #5975
jooyoungseo opened this issue Nov 30, 2023 · 5 comments · Fixed by #5980
Closed
Tracked by #5975
Assignees
Labels
accessibility feature-request Request for new features or functionality
Milestone

Comments

@jooyoungseo
Copy link

jooyoungseo commented Nov 30, 2023

Type: Feature Request

CC @meganrogge

For screen reader users, it looks like there is no way to start reviewing and approve PR using command palette without assigning keybindings like below:

[
  {
    "key": "ctrl+shift+g r",
    "command": "pr.startReview"
  },
  {
    "key": "ctrl+shift+g a",
    "command": "review.approve"
  }
]

Please expose as many relevant commands in the command palette as possible for keyboard users for better discoverability and navigability.

Extension version: 0.77.2023112911
VS Code version: Code - Insiders 1.85.0-insider (8762127fc97b8aaae5f3a0dd6a6d853d9a1b7574, 2023-11-29T17:38:23.440Z)
OS version: Windows_NT x64 10.0.22631
Modes:

@meganrogge meganrogge added this to the December 2023 milestone Nov 30, 2023
@meganrogge meganrogge added feature-request Request for new features or functionality accessibility labels Nov 30, 2023
@meganrogge meganrogge modified the milestones: December 2023, February 2024 Dec 6, 2023
@joaomoreno joaomoreno removed this from the February 2024 milestone Dec 11, 2023
@alexr00 alexr00 added this to the December / January 2024 milestone Dec 11, 2023
@meganrogge meganrogge removed their assignment Jan 22, 2024
@alexr00 alexr00 modified the milestones: December / January 2024, February 2024 Jan 23, 2024
@alexr00
Copy link
Member

alexr00 commented Feb 2, 2024

There's no need to run pr.startReview, that command is just an implementation detail so that the text on a button is correct when you add your first comment to a PR.

For approving and requesting changes, those buttons are available in the "Active Pull Request" view and pull request description. @jooyoungseo and @meganrogge is it fair to say that for approving or requesting changes it makes sense to keep them out of the command palette so that users can use the input boxes to add a comment when they approve or request changes?

@meganrogge
Copy link
Contributor

@jooyoungseo, could you pls help us understand your flow?

@alexr00 it can take a lot of effort for SR / keyboard only users to tab to various buttons. I am wondering if that is why JooYoung is assigning keybindings for these.

@jooyoungseo
Copy link
Author

jooyoungseo commented Feb 2, 2024 via email

@meganrogge
Copy link
Contributor

meganrogge commented Feb 2, 2024

Indeed, JooYoung showed that he is often in an editor when he wants to comment on & approve / request changes for a PR.

I suggest we add commands, with default keybindings when in screen reader mode for:

  1. focusing the input box of the PR's review view
  2. approving / requesting changes on the PR
  3. focusing the PR view
  4. focus issues

@alexr00 I will share the recording when JooYoung has sent it to me.

@alexr00
Copy link
Member

alexr00 commented Apr 30, 2024

  1. focusing the input box of the PR's review view
  2. approving / requesting changes on the PR
  3. focusing the PR view
  4. focus issues

1 will be addressed by #5980, which adds a command to focus the review input in the PR description when the PR description is visible.
For 2, I would not add commands for this as I think it makes sense to go through the PR description or focus view for these where you can add a review summary comment.
Commands already exist for 3 and 4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility feature-request Request for new features or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants