Skip to content

Commit 8af2daf

Browse files
committed
feat(button-toggle): general component cleanup and support value input in multiple mode
* 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.
1 parent c3d7cd9 commit 8af2daf

File tree

3 files changed

+225
-246
lines changed

3 files changed

+225
-246
lines changed

src/lib/button-toggle/button-toggle-module.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,14 @@
77
*/
88

99
import {NgModule} from '@angular/core';
10-
import {MatButtonToggleGroup, MatButtonToggleGroupMultiple, MatButtonToggle} from './button-toggle';
10+
import {MatButtonToggleGroup, MatButtonToggle} from './button-toggle';
1111
import {MatCommonModule} from '@angular/material/core';
12-
import {UNIQUE_SELECTION_DISPATCHER_PROVIDER} from '@angular/cdk/collections';
1312
import {A11yModule} from '@angular/cdk/a11y';
1413

1514

1615
@NgModule({
1716
imports: [MatCommonModule, A11yModule],
18-
exports: [
19-
MatButtonToggleGroup,
20-
MatButtonToggleGroupMultiple,
21-
MatButtonToggle,
22-
MatCommonModule,
23-
],
24-
declarations: [MatButtonToggleGroup, MatButtonToggleGroupMultiple, MatButtonToggle],
25-
providers: [UNIQUE_SELECTION_DISPATCHER_PROVIDER]
17+
exports: [MatCommonModule, MatButtonToggleGroup, MatButtonToggle],
18+
declarations: [MatButtonToggleGroup, MatButtonToggle],
2619
})
2720
export class MatButtonToggleModule {}

src/lib/button-toggle/button-toggle.spec.ts

Lines changed: 46 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,17 @@
1-
import {
2-
async,
3-
fakeAsync,
4-
tick,
5-
ComponentFixture,
6-
TestBed,
7-
} from '@angular/core/testing';
1+
import {fakeAsync, tick, ComponentFixture, TestBed} from '@angular/core/testing';
82
import {NgModel, FormsModule, ReactiveFormsModule, FormControl} from '@angular/forms';
93
import {Component, DebugElement} from '@angular/core';
104
import {By} from '@angular/platform-browser';
115
import {
126
MatButtonToggleGroup,
137
MatButtonToggle,
14-
MatButtonToggleGroupMultiple,
158
MatButtonToggleChange,
169
MatButtonToggleModule,
1710
} from './index';
1811

19-
2012
describe('MatButtonToggle with forms', () => {
2113

22-
beforeEach(async(() => {
14+
beforeEach(fakeAsync(() => {
2315
TestBed.configureTestingModule({
2416
imports: [MatButtonToggleModule, FormsModule, ReactiveFormsModule],
2517
declarations: [
@@ -37,7 +29,7 @@ describe('MatButtonToggle with forms', () => {
3729
let groupInstance: MatButtonToggleGroup;
3830
let testComponent: ButtonToggleGroupWithFormControl;
3931

40-
beforeEach(async(() => {
32+
beforeEach(fakeAsync(() => {
4133
fixture = TestBed.createComponent(ButtonToggleGroupWithFormControl);
4234
fixture.detectChanges();
4335

@@ -89,7 +81,7 @@ describe('MatButtonToggle with forms', () => {
8981
let groupNgModel: NgModel;
9082
let buttonToggleLabels: HTMLElement[];
9183

92-
beforeEach(async(() => {
84+
beforeEach(fakeAsync(() => {
9385
fixture = TestBed.createComponent(ButtonToggleGroupWithNgModel);
9486
fixture.detectChanges();
9587
testComponent = fixture.debugElement.componentInstance;
@@ -143,7 +135,10 @@ describe('MatButtonToggle with forms', () => {
143135
for (let buttonToggle of buttonToggleInstances) {
144136
expect(buttonToggle.checked).toBe(groupInstance.value === buttonToggle.value);
145137
}
146-
expect(groupInstance.selected!.value).toBe(groupInstance.value);
138+
139+
const selected = groupInstance.selected as MatButtonToggle;
140+
141+
expect(selected.value).toBe(groupInstance.value);
147142
});
148143

149144
it('should have the correct FormControl state initially and after interaction',
@@ -183,7 +178,7 @@ describe('MatButtonToggle with forms', () => {
183178

184179
describe('MatButtonToggle without forms', () => {
185180

186-
beforeEach(async(() => {
181+
beforeEach(fakeAsync(() => {
187182
TestBed.configureTestingModule({
188183
imports: [MatButtonToggleModule],
189184
declarations: [
@@ -318,7 +313,6 @@ describe('MatButtonToggle without forms', () => {
318313
let changeSpy = jasmine.createSpy('button-toggle change listener');
319314
buttonToggleInstances[0].change.subscribe(changeSpy);
320315

321-
322316
buttonToggleLabelElements[0].click();
323317
fixture.detectChanges();
324318
tick();
@@ -392,14 +386,16 @@ describe('MatButtonToggle without forms', () => {
392386

393387
fixture.detectChanges();
394388

389+
// Note that we cast to a boolean, because the event has some circular references
390+
// which will crash the runner when Jasmine attempts to stringify them.
391+
expect(!!testComponent.lastEvent).toBe(false);
395392
expect(groupInstance.value).toBe('red');
396-
expect(testComponent.lastEvent).toBeFalsy();
397393

398394
groupInstance.value = 'green';
399395
fixture.detectChanges();
400396

397+
expect(!!testComponent.lastEvent).toBe(false);
401398
expect(groupInstance.value).toBe('green');
402-
expect(testComponent.lastEvent).toBeFalsy();
403399
});
404400

405401
});
@@ -411,20 +407,19 @@ describe('MatButtonToggle without forms', () => {
411407
let buttonToggleDebugElements: DebugElement[];
412408
let buttonToggleNativeElements: HTMLElement[];
413409
let buttonToggleLabelElements: HTMLLabelElement[];
414-
let groupInstance: MatButtonToggleGroupMultiple;
410+
let groupInstance: MatButtonToggleGroup;
415411
let buttonToggleInstances: MatButtonToggle[];
416412
let testComponent: ButtonTogglesInsideButtonToggleGroupMultiple;
417413

418-
beforeEach(async(() => {
414+
beforeEach(fakeAsync(() => {
419415
fixture = TestBed.createComponent(ButtonTogglesInsideButtonToggleGroupMultiple);
420416
fixture.detectChanges();
421417

422418
testComponent = fixture.debugElement.componentInstance;
423419

424-
groupDebugElement = fixture.debugElement.query(By.directive(MatButtonToggleGroupMultiple));
420+
groupDebugElement = fixture.debugElement.query(By.directive(MatButtonToggleGroup));
425421
groupNativeElement = groupDebugElement.nativeElement;
426-
groupInstance = groupDebugElement.injector.get<MatButtonToggleGroupMultiple>(
427-
MatButtonToggleGroupMultiple);
422+
groupInstance = groupDebugElement.injector.get<MatButtonToggleGroup>(MatButtonToggleGroup);
428423

429424
buttonToggleDebugElements = fixture.debugElement.queryAll(By.directive(MatButtonToggle));
430425
buttonToggleNativeElements = buttonToggleDebugElements
@@ -448,16 +443,22 @@ describe('MatButtonToggle without forms', () => {
448443
let nativeCheckboxLabel = buttonToggleDebugElements[0].query(By.css('label')).nativeElement;
449444

450445
nativeCheckboxLabel.click();
446+
447+
expect(groupInstance.value).toEqual(['eggs']);
451448
expect(buttonToggleInstances[0].checked).toBe(true);
452449
});
453450

454451
it('should allow for multiple toggles to be selected', () => {
455452
buttonToggleInstances[0].checked = true;
456453
fixture.detectChanges();
454+
455+
expect(groupInstance.value).toEqual(['eggs']);
457456
expect(buttonToggleInstances[0].checked).toBe(true);
458457

459458
buttonToggleInstances[1].checked = true;
460459
fixture.detectChanges();
460+
461+
expect(groupInstance.value).toEqual(['eggs', 'flour']);
461462
expect(buttonToggleInstances[1].checked).toBe(true);
462463
expect(buttonToggleInstances[0].checked).toBe(true);
463464
});
@@ -468,6 +469,7 @@ describe('MatButtonToggle without forms', () => {
468469
nativeCheckboxInput.click();
469470
fixture.detectChanges();
470471

472+
expect(groupInstance.value).toEqual(['eggs']);
471473
expect(buttonToggleInstances[0].checked).toBe(true);
472474
});
473475

@@ -480,18 +482,23 @@ describe('MatButtonToggle without forms', () => {
480482
expect(groupNativeElement.classList).toContain('mat-button-toggle-vertical');
481483
});
482484

483-
it('should deselect a button toggle when selected twice', () => {
484-
buttonToggleNativeElements[0].click();
485+
it('should deselect a button toggle when selected twice', fakeAsync(() => {
486+
buttonToggleLabelElements[0].click();
485487
fixture.detectChanges();
488+
tick();
486489

487-
buttonToggleNativeElements[0].click();
490+
expect(buttonToggleInstances[0].checked).toBe(true);
491+
expect(groupInstance.value).toEqual(['eggs']);
492+
493+
buttonToggleLabelElements[0].click();
488494
fixture.detectChanges();
495+
tick();
489496

497+
expect(groupInstance.value).toEqual([]);
490498
expect(buttonToggleInstances[0].checked).toBe(false);
491-
});
499+
}));
492500

493501
it('should emit a change event for state changes', fakeAsync(() => {
494-
495502
expect(buttonToggleInstances[0].checked).toBe(false);
496503

497504
let changeSpy = jasmine.createSpy('button-toggle change listener');
@@ -501,17 +508,25 @@ describe('MatButtonToggle without forms', () => {
501508
fixture.detectChanges();
502509
tick();
503510
expect(changeSpy).toHaveBeenCalled();
511+
expect(groupInstance.value).toEqual(['eggs']);
504512

505513
buttonToggleLabelElements[0].click();
506514
fixture.detectChanges();
507515
tick();
516+
expect(groupInstance.value).toEqual([]);
508517

509518
// The default browser behavior is to emit an event, when the value was set
510519
// to false. That's because the current input type is set to `checkbox` when
511520
// using the multiple mode.
512521
expect(changeSpy).toHaveBeenCalledTimes(2);
513522
}));
514523

524+
it('should throw when attempting to assign a non-array value', () => {
525+
expect(() => {
526+
groupInstance.value = 'not-an-array';
527+
}).toThrowError(/Value must be an array/);
528+
});
529+
515530
});
516531

517532
describe('as standalone', () => {
@@ -522,19 +537,18 @@ describe('MatButtonToggle without forms', () => {
522537
let buttonToggleInstance: MatButtonToggle;
523538
let testComponent: StandaloneButtonToggle;
524539

525-
beforeEach(async(() => {
540+
beforeEach(fakeAsync(() => {
526541
fixture = TestBed.createComponent(StandaloneButtonToggle);
527542
fixture.detectChanges();
528543

529-
testComponent = fixture.debugElement.componentInstance;
530-
544+
testComponent = fixture.componentInstance;
531545
buttonToggleDebugElement = fixture.debugElement.query(By.directive(MatButtonToggle));
532546
buttonToggleNativeElement = buttonToggleDebugElement.nativeElement;
533547
buttonToggleLabelElement = fixture.debugElement.query(By.css('label')).nativeElement;
534548
buttonToggleInstance = buttonToggleDebugElement.componentInstance;
535549
}));
536550

537-
it('should toggle when clicked', () => {
551+
it('should toggle when clicked', fakeAsync(() => {
538552
buttonToggleLabelElement.click();
539553

540554
fixture.detectChanges();
@@ -545,7 +559,7 @@ describe('MatButtonToggle without forms', () => {
545559
fixture.detectChanges();
546560

547561
expect(buttonToggleInstance.checked).toBe(false);
548-
});
562+
}));
549563

550564
it('should emit a change event for state changes', fakeAsync(() => {
551565

0 commit comments

Comments
 (0)