-
Notifications
You must be signed in to change notification settings - Fork 599
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
fix: wc menu keyboarding and aria-expanded #3496
Conversation
@attr({ attribute: "expanded" }) | ||
public expanded: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be aria-expanded
or AT will not recognize it as expanded. The attr should stay the same here. I think we you'll want is to keep the boolean attribute converter but change "reflect" to "fromView".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We write to the "aria-expanded" attribute in the template, just like we do for disabled:
aria-expanded="${x => x.expanded}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's undefined we don't render a value, which fixes the bug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gotcha, thanks for the additional context.
f66985f
to
cbb26c0
Compare
06d3991
to
970422e
Compare
Description
A timing issue was causing the menu to check the role attribute of children to determine focusability before those children had a chance to update their roles. Waiting to check on the value change callback resolved that. Also, we were always setting an "aria-expanded" attribute on menu items which would cause menu items to announce as not expanded even when they had no expanded state to begin with.
Motivation & context
partially fixes #3481
Issue type checklist
Is this a breaking change?
Adding or modifying component(s) in
@microsoft/fast-components
checklistProcess & policy checklist