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 aria-description support for selectbox #152251

Merged
merged 2 commits into from
Jun 16, 2022

Conversation

alanrenmsft
Copy link
Contributor

while working on an accessibility issue for Azure Data Studio (a fork of vscode), we realized that this option is missing, thought it could be useful for VSCode too, so created this PR.

@@ -126,6 +126,10 @@ export class SelectBoxList extends Disposable implements ISelectBoxDelegate, ILi
this.selectElement.setAttribute('aria-label', this.selectBoxOptions.ariaLabel);
}

if (this.selectBoxOptions.ariaDescription) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably best to keep the check similar to the other ones

typeof this.selectBoxOptions.ariaDescription === 'string'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -35,6 +35,10 @@ export class SelectBoxNative extends Disposable implements ISelectBoxDelegate {
this.selectElement.setAttribute('aria-label', this.selectBoxOptions.ariaLabel);
}

if (this.selectBoxOptions.ariaDescription) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here - typeof this.selectBoxOptions.ariaDescription === 'string'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@meganrogge meganrogge assigned bpasero and unassigned meganrogge Jun 15, 2022
@bpasero bpasero assigned isidorn and unassigned bpasero Jun 16, 2022
@isidorn
Copy link
Contributor

isidorn commented Jun 16, 2022

@alanrenmsft thanks for the PR. I think this makes sense, even though aria-description is not yet supported by all screen readers to my knowledge.
Though makes sense for the future, and it is a small change. So merging in. Thank you!

@isidorn isidorn merged commit c3424ba into microsoft:main Jun 16, 2022
@isidorn isidorn added this to the June 2022 milestone Jun 16, 2022
justschen pushed a commit to justschen/vscode that referenced this pull request Jun 16, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2022
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