-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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(menu): role being set on the wrong element #5191
Conversation
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.
LGTM
src/lib/menu/menu.spec.ts
Outdated
fixture.componentInstance.trigger.openMenu(); | ||
fixture.detectChanges(); | ||
|
||
expect(overlayContainerElement.querySelector('[role="menu"]')) |
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.
Any reason why we're not using getAttribute
here to check directly?
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 was a little shorter. I'll switch to a querySelector
and getAttribute
.
Currently, the `menu` role is set on the `md-menu` directive, however the it is inert and won't get rendered inside the overlay. These changes remove the `menu` role from the inert element and set it on the element that will get rendered inside the overlay.
6c05288
to
c292dd7
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Currently, the
menu
role is set on themd-menu
directive, however the it is inert and won't get rendered inside the overlay. These changes remove themenu
role from the inert element and set it on the element that will get rendered inside the overlay.