Skip to content

Commit

Permalink
fix(menu): not closing sibling sub-menus when hovering over disabled …
Browse files Browse the repository at this point in the history
…items (#10396)

* Fixes sub-menus not being closed when the user hovers over a disabled sibling menu item.
* Fixes the sub-menu arrow not being greyed out for disabled items.
  • Loading branch information
crisbeto authored and mmalerba committed Apr 19, 2018
1 parent 0e21443 commit 3b7fe64
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 7 deletions.
4 changes: 3 additions & 1 deletion src/lib/menu/_menu-theme.scss
Expand Up @@ -16,7 +16,9 @@
color: mat-color($foreground, 'text');

&[disabled] {
color: mat-color($foreground, 'disabled');
&, &::after {
color: mat-color($foreground, 'disabled');
}
}
}

Expand Down
8 changes: 3 additions & 5 deletions src/lib/menu/menu-item.ts
Expand Up @@ -47,7 +47,7 @@ export const _MatMenuItemMixinBase = mixinDisableRipple(mixinDisabled(MatMenuIte
'[attr.aria-disabled]': 'disabled.toString()',
'[attr.disabled]': 'disabled || null',
'(click)': '_checkDisabled($event)',
'(mouseenter)': '_emitHoverEvent()',
'(mouseenter)': '_handleMouseEnter()',
},
changeDetection: ChangeDetectionStrategy.OnPush,
encapsulation: ViewEncapsulation.None,
Expand Down Expand Up @@ -121,10 +121,8 @@ export class MatMenuItem extends _MatMenuItemMixinBase
}

/** Emits to the hover stream. */
_emitHoverEvent() {
if (!this.disabled) {
this._hovered.next(this);
}
_handleMouseEnter() {
this._hovered.next(this);
}

/** Gets the label to be used when determining whether the option should be focused. */
Expand Down
2 changes: 1 addition & 1 deletion src/lib/menu/menu-trigger.ts
Expand Up @@ -152,7 +152,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
if (this.triggersSubmenu()) {
// Subscribe to changes in the hovered item in order to toggle the panel.
this._hoverSubscription = this._parentMenu._hovered()
.pipe(filter(active => active === this._menuItemInstance))
.pipe(filter(active => active === this._menuItemInstance && !active.disabled))
.subscribe(() => {
this._openedByMouse = true;
this.openMenu();
Expand Down
51 changes: 51 additions & 0 deletions src/lib/menu/menu.spec.ts
Expand Up @@ -910,6 +910,57 @@ describe('MatMenu', () => {
.toBe(1, 'Expected one open menu');
}));

it('should close submenu when hovering over disabled sibling item', fakeAsync(() => {
compileTestComponent();
instance.rootTriggerEl.nativeElement.click();
fixture.detectChanges();
tick(500);

const items = fixture.debugElement.queryAll(By.directive(MatMenuItem));

dispatchFakeEvent(items[0].nativeElement, 'mouseenter');
fixture.detectChanges();
tick(500);

expect(overlay.querySelectorAll('.mat-menu-panel').length)
.toBe(2, 'Expected two open menus');

items[1].componentInstance.disabled = true;
fixture.detectChanges();

// Invoke the handler directly since the fake events are flaky on disabled elements.
items[1].componentInstance._handleMouseEnter();
fixture.detectChanges();
tick(500);

expect(overlay.querySelectorAll('.mat-menu-panel').length)
.toBe(1, 'Expected one open menu');
}));

it('should not open submenu when hovering over disabled trigger', fakeAsync(() => {
compileTestComponent();
instance.rootTriggerEl.nativeElement.click();
fixture.detectChanges();
tick(500);

expect(overlay.querySelectorAll('.mat-menu-panel').length)
.toBe(1, 'Expected one open menu');

const item = fixture.debugElement.query(By.directive(MatMenuItem));

item.componentInstance.disabled = true;
fixture.detectChanges();

// Invoke the handler directly since the fake events are flaky on disabled elements.
item.componentInstance._handleMouseEnter();
fixture.detectChanges();
tick(500);

expect(overlay.querySelectorAll('.mat-menu-panel').length)
.toBe(1, 'Expected to remain at one open menu');
}));


it('should open a nested menu when its trigger is clicked', () => {
compileTestComponent();
instance.rootTriggerEl.nativeElement.click();
Expand Down

0 comments on commit 3b7fe64

Please sign in to comment.