Skip to content

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