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

Support change the aria role of menu or menuitem. #115

Closed
wants to merge 1 commit into from

Conversation

fatelei
Copy link

@fatelei fatelei commented Jan 2, 2018

When menu is used with select support search mode

  • the role of menu should be listbox
  • the role of menuitem should be option

so that, the screen reader like nvda can read the item in option.

Remove the aria-activedescendant

This attribute has no meaning in menu.

Ref

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 84.8% when pulling c4a48e9 on fatelei:aria-issue into 5a5d6c8 on react-component:master.

@fatelei fatelei closed this Jan 2, 2018
@fatelei fatelei reopened this Jan 2, 2018
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 84.8% when pulling c4a48e9 on fatelei:aria-issue into 5a5d6c8 on react-component:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 84.77% when pulling b35ad16 on fatelei:aria-issue into 5a5d6c8 on react-component:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 84.77% when pulling 847b3bb on fatelei:aria-issue into 5a5d6c8 on react-component:master.

@fatelei
Copy link
Author

fatelei commented Jan 2, 2018

@afc163

const domProps = {
className,
role: 'menu',
'aria-activedescendant': '',
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Member

Choose a reason for hiding this comment

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

@fatelei Why do you remove this line?

Copy link
Contributor

Choose a reason for hiding this comment

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

aria-activedescendant="" is an invalid value

@picodoth
Copy link
Contributor

@fatelei woops, I did a similar fix here: #137. Sorry, didn't realize this one. But I believe #137 covers more invalid aria-* values. Do you mind to close this one?

@fatelei
Copy link
Author

fatelei commented Apr 28, 2018

@picodoth I will close this pr

@fatelei fatelei closed this Apr 28, 2018
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.

5 participants