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

add accessibility help extension contributions pt #210116

Merged
merged 37 commits into from Apr 18, 2024

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Apr 10, 2024

This includes:

  • an accessibility help dialog implementation for each view with accessibleHelpContent
  • an accessibility help hint that is applied to the view's aria label until a user opens the accessibility help dialog for that view. this required adding a new AcessibleViewInformationService to workbench/services so the view service can understand if an accessible view has been shown already in that layer
  • resolves <keybinding:commandId> to the keybinding, if any, or a message and linked command to configure the keybinding

Screenshot 2024-04-15 at 2 09 28 PM

@meganrogge meganrogge marked this pull request as draft April 10, 2024 23:26
@meganrogge meganrogge self-assigned this Apr 10, 2024
@meganrogge meganrogge added this to the April 2024 milestone Apr 10, 2024
@meganrogge meganrogge marked this pull request as ready for review April 11, 2024 17:03
@meganrogge meganrogge requested a review from mjbvz April 11, 2024 17:04
Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

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

Very cool! In addtion to my inline comments, why is the accessibilityHelpContent contribution only a string and not also a markdown string?

@meganrogge
Copy link
Contributor Author

meganrogge commented Apr 12, 2024

why is the accessibilityHelpContent contribution only a string and not also a markdown string?

done

@meganrogge meganrogge enabled auto-merge (squash) April 15, 2024 16:22
@meganrogge meganrogge enabled auto-merge (squash) April 15, 2024 16:26
Copy link
Contributor

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

Let's discuss how keybindings should be expressed at an API sync before merging this

@meganrogge
Copy link
Contributor Author

meganrogge commented Apr 15, 2024

To discuss in API sync:

  1. Do we want to have syntax which allows just a command ID to be provided and resolved as a the command title, its keybinding (if any) or the link to configure the keybinding
  2. Can we render the markdown such that a screen reader user doesn't have to navigate through [Configure a Keybinding](commandLink)?

@alexr00 alexr00 disabled auto-merge April 16, 2024 09:55
alexr00
alexr00 previously approved these changes Apr 16, 2024
Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

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

You have my approval, but I unchecked auto-merge since Matt wants to discuss at the API sync.

@meganrogge
Copy link
Contributor Author

meganrogge commented Apr 18, 2024

@mjbvz discussed with Jo at the API sync and he has signed off on what I'm doing for keybindings.

I will get feedback on and defer this part #210665:

Can we render the markdown such that a screen reader user doesn't have to navigate through Configure a Keybinding?

@meganrogge meganrogge merged commit 48bbfa7 into main Apr 18, 2024
9 checks passed
@meganrogge meganrogge deleted the merogge/accessibility-help-extension branch April 18, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants