fix(button): prevent click bindings from firing on disabled anchor bu…

…ttons (#963) Co-authored-by: Alexander Sawtschuk <[email protected]>
allianz · Jul 10, 2023 · 3d2ef2d · 3d2ef2d
1 parent 2928f7a
commit 3d2ef2d
Show file tree

Hide file tree

Showing 10 changed files with 143 additions and 137 deletions.
diff --git a/projects/ng-aquila/src/button/anchor-button.component.spec.ts b/projects/ng-aquila/src/button/anchor-button.component.spec.ts
@@ -1,6 +1,5 @@
 import { Component, Directive, Type, ViewChild } from '@angular/core';
-import { ComponentFixture, fakeAsync, TestBed, waitForAsync } from '@angular/core/testing';
-import { NxIconModule } from '@aposin/ng-aquila/icon';
+import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
 
 import { NxButtonModule } from '.';
 import { NxAnchorButtonComponent } from './anchor-button.component';
@@ -11,48 +10,55 @@ abstract class AnchorButtonTest {
 }
 
 @Component({
-    template: `<a nxButton #button href="#" class="some-arbitray-class-name">Hello Anchor Button</a>`,
+    template: `<a nxButton #button href="#" class="some-arbitrary-class-name">Hello Anchor Button</a>`,
 })
-class AnchorButton extends AnchorButtonTest {}
+class TestInstance extends AnchorButtonTest {
+    clickBindingSpy = jasmine.createSpy('clickSpy');
+}
 
 describe('NxAnchorButtonComponent', () => {
-    let fixture: ComponentFixture<AnchorButton>;
-    let testInstance: AnchorButton;
-    let buttonInstance: NxAnchorButtonComponent;
-    let buttonNativeElement: HTMLAnchorElement;
-
     beforeEach(waitForAsync(() => {
         TestBed.configureTestingModule({
-            declarations: [AnchorButton],
-            imports: [NxIconModule, NxButtonModule],
+            declarations: [TestInstance],
+            imports: [NxButtonModule],
         }).compileComponents();
     }));
 
-    function createTestComponent(component: Type<AnchorButton>) {
+    let fixture: ComponentFixture<TestInstance>;
+    let testInstance: TestInstance;
+    let buttonInstance: NxAnchorButtonComponent;
+    let buttonNativeElement: HTMLAnchorElement;
+
+    function createTestComponent(component: Type<TestInstance>) {
         fixture = TestBed.createComponent(component);
         fixture.detectChanges();
         testInstance = fixture.componentInstance;
-        buttonInstance = testInstance.buttonInstance;
+        buttonInstance = fixture.componentInstance.buttonInstance;
         buttonNativeElement = fixture.nativeElement.querySelector('a') as HTMLAnchorElement;
     }
 
     it('creates the button', waitForAsync(() => {
-        createTestComponent(AnchorButton);
+        createTestComponent(TestInstance);
         expect(buttonInstance).toBeTruthy();
     }));
 
-    it('prevents default when the anchor button is disabled', fakeAsync(() => {
-        createTestComponent(AnchorButton);
-        const clickSpy = jasmine.createSpy('clickSpy');
-        buttonNativeElement.addEventListener('click', clickSpy);
+    it('has correct base class', waitForAsync(() => {
+        createTestComponent(TestInstance);
+        expect(buttonNativeElement).toHaveClass('nx-button');
+    }));
+
+    it('disabled state prevents click binding on host element from firing', () => {
+        createTestComponent(TestInstance);
+        fixture.detectChanges();
         buttonInstance.disabled = true;
         buttonNativeElement.click();
-        expect(clickSpy).toHaveBeenCalledTimes(0);
-    }));
+
+        expect(testInstance.clickBindingSpy).not.toHaveBeenCalled();
+    });
 
     describe('a11y', () => {
         it('has no accessibility violations', async () => {
-            createTestComponent(AnchorButton);
+            createTestComponent(TestInstance);
             await expectAsync(fixture.nativeElement).toBeAccessible();
         });
     });

diff --git a/projects/ng-aquila/src/button/anchor-button.component.ts b/projects/ng-aquila/src/button/anchor-button.component.ts
@@ -1,28 +1,16 @@
-import { FocusMonitor } from '@angular/cdk/a11y';
-import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, HostListener } from '@angular/core';
+import { ChangeDetectionStrategy, Component } from '@angular/core';
 import { NxTriggerButton } from '@aposin/ng-aquila/overlay';
-
-import { NxButtonComponent } from './button.component';
+import { NxAnchorButtonBase } from './button-base';
 
 @Component({
     templateUrl: './button.html',
     styleUrls: ['button.scss'],
     selector: 'a[nxButton]',
-    inputs: ['classNames:nxButton'],
     changeDetection: ChangeDetectionStrategy.OnPush,
+    inputs: ['classNames:nxButton'],
     providers: [{ provide: NxTriggerButton, useExisting: NxAnchorButtonComponent }],
+    host: {
+        class: 'nx-button',
+    },
 })
-export class NxAnchorButtonComponent extends NxButtonComponent {
-    /** @docs-private */
-    @HostListener('click', ['$event'])
-    _checkEventsDisabled(event: Event) {
-        if (this.disabled) {
-            event.preventDefault();
-            event.stopImmediatePropagation();
-        }
-    }
-
-    constructor(_cdr: ChangeDetectorRef, elementRef: ElementRef, focusMonitor: FocusMonitor) {
-        super(_cdr, elementRef, focusMonitor);
-    }
-}
+export class NxAnchorButtonComponent extends NxAnchorButtonBase {}
diff --git a/projects/ng-aquila/src/button/anchor-icon-button.component.spec.ts b/projects/ng-aquila/src/button/anchor-icon-button.component.spec.ts
@@ -7,53 +7,60 @@ import { NxAnchorButtonComponent } from './anchor-button.component';
 import { NxAnchorIconButtonComponent } from './anchor-icon-button.component';
 
 @Directive()
-abstract class AnchorIconButtonTest {
-    @ViewChild('button') buttonInstance!: NxAnchorIconButtonComponent;
+abstract class AnchorButtonTest {
+    @ViewChild('button') buttonInstance!: NxAnchorButtonComponent;
 }
 
 @Component({
-    template: `<a nxIconButton #button href="#" class="some-arbitray-class-name">Hello Anchor Button</a>`,
+    template: `<a nxIconButton #button href="#" class="some-arbitrary-class-name" aria-label="Link Text"><nx-icon name="info"></nx-icon></a>`,
 })
-class AnchorButton extends AnchorIconButtonTest {}
+class TestInstance extends AnchorButtonTest {
+    clickBindingSpy = jasmine.createSpy('clickSpy');
+}
 
 describe('NxAnchorIconButtonComponent', () => {
-    let fixture: ComponentFixture<AnchorButton>;
-    let testInstance: AnchorButton;
-    let buttonInstance: NxAnchorButtonComponent;
-    let buttonNativeElement: HTMLAnchorElement;
-
     beforeEach(waitForAsync(() => {
         TestBed.configureTestingModule({
-            declarations: [AnchorButton],
+            declarations: [TestInstance],
             imports: [NxIconModule, NxButtonModule],
         }).compileComponents();
     }));
 
-    function createTestComponent(component: Type<AnchorButton>) {
+    let fixture: ComponentFixture<TestInstance>;
+    let testInstance: TestInstance;
+    let buttonInstance: NxAnchorButtonComponent;
+    let buttonNativeElement: HTMLAnchorElement;
+
+    function createTestComponent(component: Type<TestInstance>) {
         fixture = TestBed.createComponent(component);
         fixture.detectChanges();
         testInstance = fixture.componentInstance;
-        buttonInstance = testInstance.buttonInstance;
+        buttonInstance = fixture.componentInstance.buttonInstance;
         buttonNativeElement = fixture.nativeElement.querySelector('a') as HTMLAnchorElement;
     }
 
     it('creates the button', waitForAsync(() => {
-        createTestComponent(AnchorButton);
+        createTestComponent(TestInstance);
         expect(buttonInstance).toBeTruthy();
     }));
 
-    it('prevents default when the anchor button is disabled', fakeAsync(() => {
-        createTestComponent(AnchorButton);
-        const clickSpy = jasmine.createSpy('clickSpy');
-        buttonNativeElement.addEventListener('click', clickSpy);
+    it('has correct base class', waitForAsync(() => {
+        createTestComponent(TestInstance);
+        expect(buttonNativeElement).toHaveClass('nx-icon-button');
+    }));
+
+    it('disabled state prevents click binding on host element from firing', () => {
+        createTestComponent(TestInstance);
+        fixture.detectChanges();
         buttonInstance.disabled = true;
         buttonNativeElement.click();
-        expect(clickSpy).toHaveBeenCalledTimes(0);
-    }));
+
+        expect(testInstance.clickBindingSpy).not.toHaveBeenCalled();
+    });
 
     describe('a11y', () => {
         it('has no accessibility violations', async () => {
-            createTestComponent(AnchorButton);
+            createTestComponent(TestInstance);
             await expectAsync(fixture.nativeElement).toBeAccessible();
         });
     });

diff --git a/projects/ng-aquila/src/button/anchor-icon-button.component.ts b/projects/ng-aquila/src/button/anchor-icon-button.component.ts
@@ -1,28 +1,16 @@
-import { FocusMonitor } from '@angular/cdk/a11y';
-import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, HostListener } from '@angular/core';
+import { ChangeDetectionStrategy, Component } from '@angular/core';
 import { NxTriggerButton } from '@aposin/ng-aquila/overlay';
-
-import { NxIconButtonComponent } from './icon-button.component';
+import { NxAnchorButtonBase } from './button-base';
 
 @Component({
     templateUrl: './button.html',
     styleUrls: ['button.scss'],
     selector: 'a[nxIconButton]',
-    inputs: ['classNames:nxIconButton'],
     changeDetection: ChangeDetectionStrategy.OnPush,
+    inputs: ['classNames:nxIconButton'],
     providers: [{ provide: NxTriggerButton, useExisting: NxAnchorIconButtonComponent }],
+    host: {
+        class: 'nx-icon-button',
+    },
 })
-export class NxAnchorIconButtonComponent extends NxIconButtonComponent {
-    /** @docs-private */
-    @HostListener('click', ['$event'])
-    _checkEventsDisabled(event: Event) {
-        if (this.disabled) {
-            event.preventDefault();
-            event.stopImmediatePropagation();
-        }
-    }
-
-    constructor(_cdr: ChangeDetectorRef, elementRef: ElementRef, focusMonitor: FocusMonitor) {
-        super(_cdr, elementRef, focusMonitor);
-    }
-}
+export class NxAnchorIconButtonComponent extends NxAnchorButtonBase {}
diff --git a/projects/ng-aquila/src/button/anchor-plain-button.component.spec.ts b/projects/ng-aquila/src/button/anchor-plain-button.component.spec.ts
@@ -1,59 +1,64 @@
 import { Component, Directive, Type, ViewChild } from '@angular/core';
-import { ComponentFixture, fakeAsync, TestBed, waitForAsync } from '@angular/core/testing';
+import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
 
-import { NxAnchorPlainButtonComponent } from './anchor-plain-button.component';
-import { NxButtonModule } from './button.module';
+import { NxAnchorPlainButtonComponent, NxButtonModule } from '.';
+import { NxAnchorButtonComponent } from './anchor-button.component';
 
 @Directive()
-abstract class ButtonTest {
-    @ViewChild('button') buttonInstance!: NxAnchorPlainButtonComponent;
-
-    disabled = false;
+abstract class AnchorButtonTest {
+    @ViewChild('button') buttonInstance!: NxAnchorButtonComponent;
 }
 
 @Component({
-    template: `<a [nxPlainButton]="classNames" #button>Hello Button</a>`,
+    template: `<a nxPlainButton #button href="#" class="some-arbitrary-class-name">Link Text</a>`,
 })
-class BasicButton extends ButtonTest {}
+class TestInstance extends AnchorButtonTest {
+    clickBindingSpy = jasmine.createSpy('clickSpy');
+}
 
-describe('NxAnchorPlainButtonComponent', () => {
-    let fixture: ComponentFixture<ButtonTest>;
-    let testInstance: ButtonTest;
-    let buttonElement: HTMLAnchorElement;
+describe('NxAnchorButtonComponent', () => {
+    beforeEach(waitForAsync(() => {
+        TestBed.configureTestingModule({
+            declarations: [TestInstance],
+            imports: [NxButtonModule],
+        }).compileComponents();
+    }));
 
-    function createTestComponent(component: Type<ButtonTest>) {
+    let fixture: ComponentFixture<TestInstance>;
+    let testInstance: TestInstance;
+    let buttonInstance: NxAnchorPlainButtonComponent;
+    let buttonNativeElement: HTMLAnchorElement;
+
+    function createTestComponent(component: Type<TestInstance>) {
         fixture = TestBed.createComponent(component);
         fixture.detectChanges();
         testInstance = fixture.componentInstance;
-        buttonElement = fixture.nativeElement.querySelector('a') as HTMLAnchorElement;
+        buttonInstance = fixture.componentInstance.buttonInstance;
+        buttonNativeElement = fixture.nativeElement.querySelector('a') as HTMLAnchorElement;
     }
 
-    beforeEach(waitForAsync(() => {
-        TestBed.configureTestingModule({
-            imports: [NxButtonModule],
-            declarations: [BasicButton],
-        }).compileComponents();
+    it('creates the button', waitForAsync(() => {
+        createTestComponent(TestInstance);
+        expect(buttonInstance).toBeTruthy();
     }));
 
-    it('should create the button component', () => {
-        createTestComponent(BasicButton);
-        expect(testInstance.buttonInstance).toBeTruthy();
-        expect(buttonElement).toBeTruthy();
-        expect(buttonElement.textContent).toBe('Hello Button');
-    });
-
-    it('prevents default when the anchor button is disabled', fakeAsync(() => {
-        createTestComponent(BasicButton);
-        const clickSpy = jasmine.createSpy('clickSpy');
-        buttonElement.addEventListener('click', clickSpy);
-        testInstance.buttonInstance.disabled = true;
-        buttonElement.click();
-        expect(clickSpy).toHaveBeenCalledTimes(0);
+    it('has correct base class', waitForAsync(() => {
+        createTestComponent(TestInstance);
+        expect(buttonNativeElement).toHaveClass('nx-plain-button');
     }));
 
+    it('disabled state prevents click binding on host element from firing', () => {
+        createTestComponent(TestInstance);
+        fixture.detectChanges();
+        buttonInstance.disabled = true;
+        buttonNativeElement.click();
+
+        expect(testInstance.clickBindingSpy).not.toHaveBeenCalled();
+    });
+
     describe('a11y', () => {
         it('has no accessibility violations', async () => {
-            createTestComponent(BasicButton);
+            createTestComponent(TestInstance);
             await expectAsync(fixture.nativeElement).toBeAccessible();
         });
     });

diff --git a/projects/ng-aquila/src/button/anchor-plain-button.component.ts b/projects/ng-aquila/src/button/anchor-plain-button.component.ts
@@ -1,28 +1,16 @@
-import { FocusMonitor } from '@angular/cdk/a11y';
-import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, HostListener } from '@angular/core';
+import { ChangeDetectionStrategy, Component } from '@angular/core';
 import { NxTriggerButton } from '@aposin/ng-aquila/overlay';
-
-import { NxPlainButtonComponent } from './plain-button.component';
+import { NxAnchorButtonBase } from './button-base';
 
 @Component({
     templateUrl: './plain-button.component.html',
     styleUrls: ['plain-button.component.scss'],
     selector: 'a[nxPlainButton]',
-    inputs: ['classNames:nxPlainButton'],
     changeDetection: ChangeDetectionStrategy.OnPush,
+    inputs: ['classNames:nxPlainButton'],
     providers: [{ provide: NxTriggerButton, useExisting: NxAnchorPlainButtonComponent }],
+    host: {
+        class: 'nx-plain-button',
+    },
 })
-export class NxAnchorPlainButtonComponent extends NxPlainButtonComponent {
-    /** @docs-private */
-    @HostListener('click', ['$event'])
-    _checkEventsDisabled(event: Event) {
-        if (this.disabled) {
-            event.preventDefault();
-            event.stopImmediatePropagation();
-        }
-    }
-
-    constructor(_cdr: ChangeDetectorRef, elementRef: ElementRef, focusMonitor: FocusMonitor) {
-        super(_cdr, elementRef, focusMonitor);
-    }
-}
+export class NxAnchorPlainButtonComponent extends NxAnchorButtonBase {}