Skip to content

Commit d10a20f

Browse files
crisbetowagnermaciel
crisbeto
authored and
wagnermaciel
committed
fix(material/list): emit selectionChange event when selecting with ctrl + a (#20667)
Fixes that we weren't emitting the `selectionChange` event when the user toggles all options with the ctrl + a shortcut. Also replaces `MatSelectionListChange.option` with `options` which is an array of all options that were changed by the current event. Fixes #15696. (cherry picked from commit 628b5e9)
1 parent 214bc7b commit d10a20f

File tree

5 files changed

+88
-36
lines changed

5 files changed

+88
-36
lines changed

‎src/material-experimental/mdc-list/selection-list.spec.ts

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,25 +64,25 @@ describe('MDC-based MatSelectionList without forms', () => {
6464
});
6565

6666
it('should not emit a selectionChange event if an option changed programmatically', () => {
67-
spyOn(fixture.componentInstance, 'onValueChange');
67+
spyOn(fixture.componentInstance, 'onSelectionChange');
6868

69-
expect(fixture.componentInstance.onValueChange).toHaveBeenCalledTimes(0);
69+
expect(fixture.componentInstance.onSelectionChange).toHaveBeenCalledTimes(0);
7070

7171
listOptions[2].componentInstance.toggle();
7272
fixture.detectChanges();
7373

74-
expect(fixture.componentInstance.onValueChange).toHaveBeenCalledTimes(0);
74+
expect(fixture.componentInstance.onSelectionChange).toHaveBeenCalledTimes(0);
7575
});
7676

7777
it('should emit a selectionChange event if an option got clicked', () => {
78-
spyOn(fixture.componentInstance, 'onValueChange');
78+
spyOn(fixture.componentInstance, 'onSelectionChange');
7979

80-
expect(fixture.componentInstance.onValueChange).toHaveBeenCalledTimes(0);
80+
expect(fixture.componentInstance.onSelectionChange).toHaveBeenCalledTimes(0);
8181

8282
dispatchMouseEvent(listOptions[2].nativeElement, 'click');
8383
fixture.detectChanges();
8484

85-
expect(fixture.componentInstance.onValueChange).toHaveBeenCalledTimes(1);
85+
expect(fixture.componentInstance.onSelectionChange).toHaveBeenCalledTimes(1);
8686
});
8787

8888
it('should be able to dispatch one selected item', () => {
@@ -518,6 +518,23 @@ describe('MDC-based MatSelectionList without forms', () => {
518518
expect(listOptions.every(option => option.componentInstance.selected)).toBe(false);
519519
});
520520

521+
// MDC does not support the common CTRL + A keyboard shortcut.
522+
// Tracked with: https://github.com/material-components/material-components-web/issues/6366
523+
// tslint:disable-next-line:ban
524+
xit('should dispatch the selectionChange event when selecting via ctrl + a', () => {
525+
const spy = spyOn(fixture.componentInstance, 'onSelectionChange');
526+
listOptions.forEach(option => option.componentInstance.disabled = false);
527+
const event = createKeyboardEvent('keydown', A, undefined, {control: true});
528+
529+
dispatchEvent(selectionList.nativeElement, event);
530+
fixture.detectChanges();
531+
532+
expect(spy).toHaveBeenCalledTimes(1);
533+
expect(spy).toHaveBeenCalledWith(jasmine.objectContaining({
534+
options: listOptions.map(option => option.componentInstance)
535+
}));
536+
});
537+
521538
it('should be able to jump focus down to an item by typing', fakeAsync(() => {
522539
const firstOption = listOptions[0].nativeElement;
523540

@@ -1426,7 +1443,7 @@ describe('MDC-based MatSelectionList with forms', () => {
14261443
@Component({template: `
14271444
<mat-selection-list
14281445
id="selection-list-1"
1429-
(selectionChange)="onValueChange($event)"
1446+
(selectionChange)="onSelectionChange($event)"
14301447
[disableRipple]="listRippleDisabled"
14311448
[color]="selectionListColor"
14321449
[multiple]="multiple">
@@ -1455,7 +1472,7 @@ class SelectionListWithListOptions {
14551472
selectionListColor: ThemePalette;
14561473
firstOptionColor: ThemePalette;
14571474

1458-
onValueChange(_change: MatSelectionListChange) {}
1475+
onSelectionChange(_change: MatSelectionListChange) {}
14591476
}
14601477

14611478
@Component({template: `

‎src/material-experimental/mdc-list/selection-list.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,16 @@ const MAT_SELECTION_LIST_VALUE_ACCESSOR: any = {
4444
/** Change event that is being fired whenever the selected state of an option changes. */
4545
export class MatSelectionListChange {
4646
constructor(
47-
/** Reference to the selection list that emitted the event. */
48-
public source: MatSelectionList,
49-
/** Reference to the option that has been changed. */
50-
public option: MatListOption) {}
47+
/** Reference to the selection list that emitted the event. */
48+
public source: MatSelectionList,
49+
/**
50+
* Reference to the option that has been changed.
51+
* @deprecated Use `options` instead, because some events may change more than one option.
52+
* @breaking-change 12.0.0
53+
*/
54+
public option: MatListOption,
55+
/** Reference to the options that have been changed. */
56+
public options: MatListOption[]) {}
5157
}
5258

5359
@Component({
@@ -209,8 +215,8 @@ export class MatSelectionList extends MatInteractiveListBase<MatListOption>
209215
}
210216

211217
/** Emits a change event if the selected state of an option changed. */
212-
_emitChangeEvent(option: MatListOption) {
213-
this.selectionChange.emit(new MatSelectionListChange(this, option));
218+
_emitChangeEvent(options: MatListOption[]) {
219+
this.selectionChange.emit(new MatSelectionListChange(this, options[0], options));
214220
}
215221

216222
/** Implemented as part of ControlValueAccessor. */
@@ -366,7 +372,7 @@ function getSelectionListAdapter(list: MatSelectionList): MDCListAdapter {
366372
baseAdapter.setAttributeForElementIndex(index, attribute, value);
367373
},
368374
notifyAction(index: number): void {
369-
list._emitChangeEvent(list._itemsArr[index]);
375+
list._emitChangeEvent([list._itemsArr[index]]);
370376
},
371377
};
372378
}

‎src/material/list/selection-list.spec.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,25 +74,25 @@ describe('MatSelectionList without forms', () => {
7474
});
7575

7676
it('should not emit a selectionChange event if an option changed programmatically', () => {
77-
spyOn(fixture.componentInstance, 'onValueChange');
77+
spyOn(fixture.componentInstance, 'onSelectionChange');
7878

79-
expect(fixture.componentInstance.onValueChange).toHaveBeenCalledTimes(0);
79+
expect(fixture.componentInstance.onSelectionChange).toHaveBeenCalledTimes(0);
8080

8181
listOptions[2].componentInstance.toggle();
8282
fixture.detectChanges();
8383

84-
expect(fixture.componentInstance.onValueChange).toHaveBeenCalledTimes(0);
84+
expect(fixture.componentInstance.onSelectionChange).toHaveBeenCalledTimes(0);
8585
});
8686

8787
it('should emit a selectionChange event if an option got clicked', () => {
88-
spyOn(fixture.componentInstance, 'onValueChange');
88+
spyOn(fixture.componentInstance, 'onSelectionChange');
8989

90-
expect(fixture.componentInstance.onValueChange).toHaveBeenCalledTimes(0);
90+
expect(fixture.componentInstance.onSelectionChange).toHaveBeenCalledTimes(0);
9191

9292
dispatchFakeEvent(listOptions[2].nativeElement, 'click');
9393
fixture.detectChanges();
9494

95-
expect(fixture.componentInstance.onValueChange).toHaveBeenCalledTimes(1);
95+
expect(fixture.componentInstance.onSelectionChange).toHaveBeenCalledTimes(1);
9696
});
9797

9898
it('should be able to dispatch one selected item', () => {
@@ -566,6 +566,20 @@ describe('MatSelectionList without forms', () => {
566566
expect(listOptions.every(option => option.componentInstance.selected)).toBe(false);
567567
});
568568

569+
it('should dispatch the selectionChange event when selecting via ctrl + a', () => {
570+
const spy = spyOn(fixture.componentInstance, 'onSelectionChange');
571+
listOptions.forEach(option => option.componentInstance.disabled = false);
572+
const event = createKeyboardEvent('keydown', A, undefined, {control: true});
573+
574+
dispatchEvent(selectionList.nativeElement, event);
575+
fixture.detectChanges();
576+
577+
expect(spy).toHaveBeenCalledTimes(1);
578+
expect(spy).toHaveBeenCalledWith(jasmine.objectContaining({
579+
options: listOptions.map(option => option.componentInstance)
580+
}));
581+
});
582+
569583
it('should be able to jump focus down to an item by typing', fakeAsync(() => {
570584
const listEl = selectionList.nativeElement;
571585
const manager = selectionList.componentInstance._keyManager;
@@ -1520,7 +1534,7 @@ describe('MatSelectionList with forms', () => {
15201534
@Component({template: `
15211535
<mat-selection-list
15221536
id="selection-list-1"
1523-
(selectionChange)="onValueChange($event)"
1537+
(selectionChange)="onSelectionChange($event)"
15241538
[disableRipple]="listRippleDisabled"
15251539
[color]="selectionListColor"
15261540
[multiple]="multiple">
@@ -1549,7 +1563,7 @@ class SelectionListWithListOptions {
15491563
selectionListColor: ThemePalette;
15501564
firstOptionColor: ThemePalette;
15511565

1552-
onValueChange(_change: MatSelectionListChange) {}
1566+
onSelectionChange(_change: MatSelectionListChange) {}
15531567
}
15541568

15551569
@Component({template: `

‎src/material/list/selection-list.ts

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,14 @@ export class MatSelectionListChange {
7272
constructor(
7373
/** Reference to the selection list that emitted the event. */
7474
public source: MatSelectionList,
75-
/** Reference to the option that has been changed. */
76-
public option: MatListOption) {}
75+
/**
76+
* Reference to the option that has been changed.
77+
* @deprecated Use `options` instead, because some events may change more than one option.
78+
* @breaking-change 12.0.0
79+
*/
80+
public option: MatListOption,
81+
/** Reference to the options that have been changed. */
82+
public options: MatListOption[]) {}
7783
}
7884

7985
/**
@@ -258,7 +264,7 @@ export class MatListOption extends _MatListOptionMixinBase implements AfterConte
258264
this.toggle();
259265

260266
// Emit a change event if the selected state of the option changed through user interaction.
261-
this.selectionList._emitChangeEvent(this);
267+
this.selectionList._emitChangeEvent([this]);
262268
}
263269
}
264270

@@ -560,7 +566,7 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements CanD
560566
if (keyCode === A && this.multiple && hasModifierKey(event, 'ctrlKey') &&
561567
!manager.isTyping()) {
562568
const shouldSelect = this.options.some(option => !option.disabled && !option.selected);
563-
this._setAllOptionsSelected(shouldSelect, true);
569+
this._setAllOptionsSelected(shouldSelect, true, true);
564570
event.preventDefault();
565571
} else {
566572
manager.onKeydown(event);
@@ -586,8 +592,8 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements CanD
586592
}
587593

588594
/** Emits a change event if the selected state of an option changed. */
589-
_emitChangeEvent(option: MatListOption) {
590-
this.selectionChange.emit(new MatSelectionListChange(this, option));
595+
_emitChangeEvent(options: MatListOption[]) {
596+
this.selectionChange.emit(new MatSelectionListChange(this, options[0], options));
591597
}
592598

593599
/** Implemented as part of ControlValueAccessor. */
@@ -648,7 +654,7 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements CanD
648654

649655
// Emit a change event because the focused option changed its state through user
650656
// interaction.
651-
this._emitChangeEvent(focusedOption);
657+
this._emitChangeEvent([focusedOption]);
652658
}
653659
}
654660
}
@@ -657,19 +663,26 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements CanD
657663
* Sets the selected state on all of the options
658664
* and emits an event if anything changed.
659665
*/
660-
private _setAllOptionsSelected(isSelected: boolean, skipDisabled?: boolean) {
666+
private _setAllOptionsSelected(
667+
isSelected: boolean,
668+
skipDisabled?: boolean,
669+
isUserInput?: boolean) {
661670
// Keep track of whether anything changed, because we only want to
662671
// emit the changed event when something actually changed.
663-
let hasChanged = false;
672+
const changedOptions: MatListOption[] = [];
664673

665674
this.options.forEach(option => {
666675
if ((!skipDisabled || !option.disabled) && option._setSelected(isSelected)) {
667-
hasChanged = true;
676+
changedOptions.push(option);
668677
}
669678
});
670679

671-
if (hasChanged) {
680+
if (changedOptions.length) {
672681
this._reportValueChange();
682+
683+
if (isUserInput) {
684+
this._emitChangeEvent(changedOptions);
685+
}
673686
}
674687
}
675688

‎tools/public_api_guard/material/list.d.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ export declare class MatSelectionList extends _MatSelectionListMixinBase impleme
116116
readonly selectionChange: EventEmitter<MatSelectionListChange>;
117117
tabIndex: number;
118118
constructor(_element: ElementRef<HTMLElement>, tabIndex: string, _changeDetector: ChangeDetectorRef, _focusMonitor?: FocusMonitor | undefined);
119-
_emitChangeEvent(option: MatListOption): void;
119+
_emitChangeEvent(options: MatListOption[]): void;
120120
_keydown(event: KeyboardEvent): void;
121121
_removeOptionFromList(option: MatListOption): MatListOption | null;
122122
_reportValueChange(): void;
@@ -140,8 +140,10 @@ export declare class MatSelectionList extends _MatSelectionListMixinBase impleme
140140

141141
export declare class MatSelectionListChange {
142142
option: MatListOption;
143+
options: MatListOption[];
143144
source: MatSelectionList;
144145
constructor(
145146
source: MatSelectionList,
146-
option: MatListOption);
147+
option: MatListOption,
148+
options: MatListOption[]);
147149
}

0 commit comments

Comments
 (0)