Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(select): make aria compliant, read value in screen readers #6000

Closed
wants to merge 1 commit into from
Closed

fix(select): make aria compliant, read value in screen readers #6000

wants to merge 1 commit into from

Conversation

rschmukler
Copy link
Contributor

closes #3891, closes #4914, closes #4977, closes #3859

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

jelbourn
jelbourn reviewed Dec 1, 2015
View reviewed changes
src/components/select/select.js
selectEl.data('$mdSelectController', mdSelectCtrl);
selectScope = scope.$new();
$mdTheming.inherit(selectContainer, element);
function getSelect() {
Copy link
Member

Choose a reason for hiding this comment

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

How about getNativeSelect ?

Copy link
Member

Choose a reason for hiding this comment

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

It's weird to have a function that starts with get that doesn't return anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not a native select. It's just finding the element inside itself that represents the select menu. Maybe getSelectMenu

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I was thrown off by the comment. Is it still accurate?

@jelbourn
Copy link
Member

jelbourn commented Dec 1, 2015

Overall LGTM aside from minor naming nitpicks.

@rschmukler rschmukler closed this in f73e503 Dec 1, 2015
@rschmukler rschmukler deleted the select-accessability branch December 1, 2015 01:15
@Splaktar Splaktar added the a11y This issue is related to accessibility label Jul 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
a11y This issue is related to accessibility
Projects
None yet
Milestone
No milestone
4 participants
@rschmukler @googlebot @jelbourn @Splaktar