feat(button-toggle): general component cleanup and support value input in multiple mode

crisbeto · crisbeto · commit 6ff097d33ff8 · 2018-02-25T21:23:14.000+01:00
* Reworks the `MatButtonToggleGroup` component to remove the need for the `MatButtonToggleGroupMultiple` component and to avoid having to implement features in two places. * Reworks the `MatButtonToggleGroup` to use the `SelectionModel` for managing its state. * Switches all of the `async` button toggle tests to run in `fakeAsync`. As a side-effect of the above-mentioned changes, the toggle group in multiple mode now supports the same inputs as the one in single-selection mode (`value` input/output, `name` input, `change` event etc.). Note: this is along the same lines as #2773, however #2773 got left behind for too long and it would need a lot more work to fit in our current setup. Fixes #9058.
diff --git a/src/lib/button-toggle/button-toggle-module.ts b/src/lib/button-toggle/button-toggle-module.ts
@@ -7,21 +7,14 @@
  */
 
 import {NgModule} from '@angular/core';
-import {MatButtonToggleGroup, MatButtonToggleGroupMultiple, MatButtonToggle} from './button-toggle';
+import {MatButtonToggleGroup, MatButtonToggle} from './button-toggle';
 import {MatCommonModule, MatRippleModule} from '@angular/material/core';
-import {UNIQUE_SELECTION_DISPATCHER_PROVIDER} from '@angular/cdk/collections';
 import {A11yModule} from '@angular/cdk/a11y';
 
 
 @NgModule({
   imports: [MatCommonModule, MatRippleModule, A11yModule],
-  exports: [
-    MatButtonToggleGroup,
-    MatButtonToggleGroupMultiple,
-    MatButtonToggle,
-    MatCommonModule,
-  ],
-  declarations: [MatButtonToggleGroup, MatButtonToggleGroupMultiple, MatButtonToggle],
-  providers: [UNIQUE_SELECTION_DISPATCHER_PROVIDER]
+  exports: [MatCommonModule, MatButtonToggleGroup, MatButtonToggle],
+  declarations: [MatButtonToggleGroup, MatButtonToggle],
 })
 export class MatButtonToggleModule {}
diff --git a/src/lib/button-toggle/button-toggle.spec.ts b/src/lib/button-toggle/button-toggle.spec.ts
@@ -1,26 +1,19 @@
-import {
-  async,
-  fakeAsync,
-  tick,
-  ComponentFixture,
-  TestBed,
-} from '@angular/core/testing';
+import {fakeAsync, tick, ComponentFixture, TestBed} from '@angular/core/testing';
 import {dispatchMouseEvent} from '@angular/cdk/testing';
 import {NgModel, FormsModule, ReactiveFormsModule, FormControl} from '@angular/forms';
 import {Component, DebugElement} from '@angular/core';
 import {By} from '@angular/platform-browser';
 import {
   MatButtonToggleGroup,
-  MatButtonToggle,
   MatButtonToggleGroupMultiple,
+  MatButtonToggle,
   MatButtonToggleChange,
   MatButtonToggleModule,
 } from './index';
 
-
 describe('MatButtonToggle with forms', () => {
 
-  beforeEach(async(() => {
+  beforeEach(fakeAsync(() => {
     TestBed.configureTestingModule({
       imports: [MatButtonToggleModule, FormsModule, ReactiveFormsModule],
       declarations: [
@@ -38,7 +31,7 @@ describe('MatButtonToggle with forms', () => {
     let groupInstance: MatButtonToggleGroup;
     let testComponent: ButtonToggleGroupWithFormControl;
 
-    beforeEach(async(() => {
+    beforeEach(fakeAsync(() => {
       fixture = TestBed.createComponent(ButtonToggleGroupWithFormControl);
       fixture.detectChanges();
 
@@ -88,7 +81,7 @@ describe('MatButtonToggle with forms', () => {
     let groupNgModel: NgModel;
     let buttonToggleLabels: HTMLElement[];
 
-    beforeEach(async(() => {
+    beforeEach(fakeAsync(() => {
       fixture = TestBed.createComponent(ButtonToggleGroupWithNgModel);
       fixture.detectChanges();
       testComponent = fixture.debugElement.componentInstance;
@@ -139,7 +132,10 @@ describe('MatButtonToggle with forms', () => {
       for (let buttonToggle of buttonToggleInstances) {
         expect(buttonToggle.checked).toBe(groupInstance.value === buttonToggle.value);
       }
-      expect(groupInstance.selected!.value).toBe(groupInstance.value);
+
+      const selected = groupInstance.selected as MatButtonToggle;
+
+      expect(selected.value).toBe(groupInstance.value);
     });
 
     it('should have the correct FormControl state initially and after interaction',
@@ -203,7 +199,7 @@ describe('MatButtonToggle with forms', () => {
 
 describe('MatButtonToggle without forms', () => {
 
-  beforeEach(async(() => {
+  beforeEach(fakeAsync(() => {
     TestBed.configureTestingModule({
       imports: [MatButtonToggleModule],
       declarations: [
@@ -338,7 +334,6 @@ describe('MatButtonToggle without forms', () => {
       let changeSpy = jasmine.createSpy('button-toggle change listener');
       buttonToggleInstances[0].change.subscribe(changeSpy);
 
-
       buttonToggleLabelElements[0].click();
       fixture.detectChanges();
       tick();
@@ -412,14 +407,16 @@ describe('MatButtonToggle without forms', () => {
 
       fixture.detectChanges();
 
+      // Note that we cast to a boolean, because the event has some circular references
+      // which will crash the runner when Jasmine attempts to stringify them.
+      expect(!!testComponent.lastEvent).toBe(false);
       expect(groupInstance.value).toBe('red');
-      expect(testComponent.lastEvent).toBeFalsy();
 
       groupInstance.value = 'green';
       fixture.detectChanges();
 
+      expect(!!testComponent.lastEvent).toBe(false);
       expect(groupInstance.value).toBe('green');
-      expect(testComponent.lastEvent).toBeFalsy();
     });
 
   });
@@ -431,20 +428,19 @@ describe('MatButtonToggle without forms', () => {
     let buttonToggleDebugElements: DebugElement[];
     let buttonToggleNativeElements: HTMLElement[];
     let buttonToggleLabelElements: HTMLLabelElement[];
-    let groupInstance: MatButtonToggleGroupMultiple;
+    let groupInstance: MatButtonToggleGroup;
     let buttonToggleInstances: MatButtonToggle[];
     let testComponent: ButtonTogglesInsideButtonToggleGroupMultiple;
 
-    beforeEach(async(() => {
+    beforeEach(fakeAsync(() => {
       fixture = TestBed.createComponent(ButtonTogglesInsideButtonToggleGroupMultiple);
       fixture.detectChanges();
 
       testComponent = fixture.debugElement.componentInstance;
 
-      groupDebugElement = fixture.debugElement.query(By.directive(MatButtonToggleGroupMultiple));
+      groupDebugElement = fixture.debugElement.query(By.directive(MatButtonToggleGroup));
       groupNativeElement = groupDebugElement.nativeElement;
-      groupInstance = groupDebugElement.injector.get<MatButtonToggleGroupMultiple>(
-          MatButtonToggleGroupMultiple);
+      groupInstance = groupDebugElement.injector.get<MatButtonToggleGroup>(MatButtonToggleGroup);
 
       buttonToggleDebugElements = fixture.debugElement.queryAll(By.directive(MatButtonToggle));
       buttonToggleNativeElements = buttonToggleDebugElements
@@ -468,16 +464,22 @@ describe('MatButtonToggle without forms', () => {
       let nativeCheckboxLabel = buttonToggleDebugElements[0].query(By.css('label')).nativeElement;
 
       nativeCheckboxLabel.click();
+
+      expect(groupInstance.value).toEqual(['eggs']);
       expect(buttonToggleInstances[0].checked).toBe(true);
     });
 
     it('should allow for multiple toggles to be selected', () => {
       buttonToggleInstances[0].checked = true;
       fixture.detectChanges();
+
+      expect(groupInstance.value).toEqual(['eggs']);
       expect(buttonToggleInstances[0].checked).toBe(true);
 
       buttonToggleInstances[1].checked = true;
       fixture.detectChanges();
+
+      expect(groupInstance.value).toEqual(['eggs', 'flour']);
       expect(buttonToggleInstances[1].checked).toBe(true);
       expect(buttonToggleInstances[0].checked).toBe(true);
     });
@@ -488,6 +490,7 @@ describe('MatButtonToggle without forms', () => {
       nativeCheckboxInput.click();
       fixture.detectChanges();
 
+      expect(groupInstance.value).toEqual(['eggs']);
       expect(buttonToggleInstances[0].checked).toBe(true);
     });
 
@@ -500,18 +503,23 @@ describe('MatButtonToggle without forms', () => {
       expect(groupNativeElement.classList).toContain('mat-button-toggle-vertical');
     });
 
-    it('should deselect a button toggle when selected twice', () => {
-      buttonToggleNativeElements[0].click();
+    it('should deselect a button toggle when selected twice', fakeAsync(() => {
+      buttonToggleLabelElements[0].click();
       fixture.detectChanges();
+      tick();
 
-      buttonToggleNativeElements[0].click();
+      expect(buttonToggleInstances[0].checked).toBe(true);
+      expect(groupInstance.value).toEqual(['eggs']);
+
+      buttonToggleLabelElements[0].click();
       fixture.detectChanges();
+      tick();
 
+      expect(groupInstance.value).toEqual([]);
       expect(buttonToggleInstances[0].checked).toBe(false);
-    });
+    }));
 
     it('should emit a change event for state changes', fakeAsync(() => {
-
       expect(buttonToggleInstances[0].checked).toBe(false);
 
       let changeSpy = jasmine.createSpy('button-toggle change listener');
@@ -521,17 +529,29 @@ describe('MatButtonToggle without forms', () => {
       fixture.detectChanges();
       tick();
       expect(changeSpy).toHaveBeenCalled();
+      expect(groupInstance.value).toEqual(['eggs']);
 
       buttonToggleLabelElements[0].click();
       fixture.detectChanges();
       tick();
+      expect(groupInstance.value).toEqual([]);
 
       // The default browser behavior is to emit an event, when the value was set
       // to false. That's because the current input type is set to `checkbox` when
       // using the multiple mode.
       expect(changeSpy).toHaveBeenCalledTimes(2);
     }));
 
+    it('should throw when attempting to assign a non-array value', () => {
+      expect(() => {
+        groupInstance.value = 'not-an-array';
+      }).toThrowError(/Value must be an array/);
+    });
+
+    it('should be able to query for the deprecated `MatButtonToggleGroupMultiple`', () => {
+      expect(fixture.debugElement.query(By.directive(MatButtonToggleGroupMultiple))).toBeTruthy();
+    });
+
   });
 
   describe('as standalone', () => {
@@ -541,7 +561,7 @@ describe('MatButtonToggle without forms', () => {
     let buttonToggleLabelElement: HTMLLabelElement;
     let buttonToggleInstance: MatButtonToggle;
 
-    beforeEach(async(() => {
+    beforeEach(fakeAsync(() => {
       fixture = TestBed.createComponent(StandaloneButtonToggle);
       fixture.detectChanges();
 
@@ -551,7 +571,7 @@ describe('MatButtonToggle without forms', () => {
       buttonToggleInstance = buttonToggleDebugElement.componentInstance;
     }));
 
-    it('should toggle when clicked', () => {
+    it('should toggle when clicked', fakeAsync(() => {
       buttonToggleLabelElement.click();
 
       fixture.detectChanges();
@@ -562,7 +582,7 @@ describe('MatButtonToggle without forms', () => {
       fixture.detectChanges();
 
       expect(buttonToggleInstance.checked).toBe(false);
-    });
+    }));
 
     it('should emit a change event for state changes', fakeAsync(() => {
 
diff --git a/src/lib/button-toggle/button-toggle.ts b/src/lib/button-toggle/button-toggle.ts
