Skip to content

Commit

Permalink
fix(cdk-experimental/menu): fix double state toggle in radio and chec…
Browse files Browse the repository at this point in the history
…kbox menu items (#20259)

Fixes an issue where clicking a CdkMenuItemRadio or CdkMenuItemCheckbox causes the click handler to
fire twice resulting in toggling state twice and an unchecked checkbox.
  • Loading branch information
andy9775 committed Aug 12, 2020
1 parent dedb799 commit c0499b8
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/cdk-experimental/menu/menu-item-checkbox.spec.ts
Expand Up @@ -53,7 +53,7 @@ describe('MenuItemCheckbox', () => {
it('should toggle the aria checked attribute', () => {
expect(checkboxElement.getAttribute('aria-checked')).toBeNull();

checkbox.trigger();
checkboxElement.click();
fixture.detectChanges();

expect(checkboxElement.getAttribute('aria-checked')).toBe('true');
Expand Down
8 changes: 2 additions & 6 deletions src/cdk-experimental/menu/menu-item-checkbox.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, HostListener} from '@angular/core';
import {Directive} from '@angular/core';
import {CdkMenuItemSelectable} from './menu-item-selectable';
import {CdkMenuItem} from './menu-item';

Expand All @@ -29,11 +29,7 @@ import {CdkMenuItem} from './menu-item';
],
})
export class CdkMenuItemCheckbox extends CdkMenuItemSelectable {
// In Ivy the `host` metadata will be merged, whereas in ViewEngine it is overridden. In order
// to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we
// can move this back into `host`.
// tslint:disable:no-host-decorator-in-concrete
@HostListener('click')
/** Toggle the checked state of the checkbox. */
trigger() {
super.trigger();

Expand Down
2 changes: 1 addition & 1 deletion src/cdk-experimental/menu/menu-item-radio.spec.ts
Expand Up @@ -51,7 +51,7 @@ describe('MenuItemRadio', () => {
it('should toggle the aria checked attribute', () => {
expect(radioElement.getAttribute('aria-checked')).toBeNull();

radioButton.trigger();
radioElement.click();
fixture.detectChanges();

expect(radioElement.getAttribute('aria-checked')).toBe('true');
Expand Down
16 changes: 1 addition & 15 deletions src/cdk-experimental/menu/menu-item-radio.ts
Expand Up @@ -6,16 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {UniqueSelectionDispatcher} from '@angular/cdk/collections';
import {
Directive,
OnDestroy,
ElementRef,
Self,
Optional,
Inject,
NgZone,
HostListener,
} from '@angular/core';
import {Directive, OnDestroy, ElementRef, Self, Optional, Inject, NgZone} from '@angular/core';
import {Directionality} from '@angular/cdk/bidi';
import {CdkMenuItemSelectable} from './menu-item-selectable';
import {CdkMenuItem} from './menu-item';
Expand Down Expand Up @@ -68,11 +59,6 @@ export class CdkMenuItemRadio extends CdkMenuItemSelectable implements OnDestroy
);
}

// In Ivy the `host` metadata will be merged, whereas in ViewEngine it is overridden. In order
// to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we
// can move this back into `host`.
// tslint:disable:no-host-decorator-in-concrete
@HostListener('click')
/** Toggles the checked state of the radio-button. */
trigger() {
super.trigger();
Expand Down

0 comments on commit c0499b8

Please sign in to comment.