Skip to content

Commit c0b34e0

Browse files
PlamenD95hanastasov
PlamenD95
and
hanastasov
authored
fix(button): Resolve the issue with buttonSelected being called on init (#13816)
* fix(button): Resolve the issue with buttonSelected being called on init * feat(button-group): Use MutationObserver to watch for changes of selected * chore(button-group): Remove commented code and console log * fix(buttonGroup): Resolve build error after the merge with master * fix(buttonGroup): Resolve the failing selected event firing unit test * fix(buttonGroup): Resolve failing button group unit tests * fix(CHANGELOG): The buttonSelected event no longer emits on init * fix(README): Update the button readme with data for buttonSelected * fix(button): Add a unit test for the emission of the buttonSelected event * fix(button): Manually call the onInit hook of the button in the unit test * fix(buttonGroup): Use a custom attribute for the filter of the MutationObserver * fix(changelog): Fix typo in CHANGELOG.md Co-authored-by: Hristo <[email protected]> * Update CHANGELOG.md --------- Co-authored-by: Hristo <[email protected]>
1 parent e3c852a commit c0b34e0

File tree

6 files changed

+123
-39
lines changed

6 files changed

+123
-39
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ All notable changes for each version of this project will be documented in this
1212
- `IPivotDimension` interface now exposes a property called `displayName` similar to the one in the `IPivotValue` interface. This property is optional and will be displayed inside the chips for rows and columns in the `IgxPivotGrid`. If the `displayName` proeprty is not set, `memberName` will be used as a fallback.
1313
- New directive - `igxIconButton` directive that provides a way to use an icon as a fully functional button has been added. The new `igxIconButton` comes in three types - flat, outlined and contained (default). All `igxButton`'s with type `icon` will be automatically migrated to the new `igxIconButton`'s with `ng update`.
1414
- `IgxButton`
15-
- **Behavioral Change** `buttonSelected` event is now emitted not only when a button gets selected, but also when it gets deselected. A benefit of that is when updating the value bound to the `selected` input of an `IgxButton`, the button group in which the button resides is now able to update the styling of the button from selected to deselected. If this event was used in a scenario where it is assumed that the button gets selected, it's a good idea the logic to be branched now based on `eventArgs.selected` condition.
15+
- **Behavioral Change** `buttonSelected` event is now emitted not only when a button gets selected, but also when it gets deselected. However, the event is no longer being emitted on initialization. If this event was used in a scenario where it is assumed that the button gets selected, it's a good idea the logic to be branched now based on `eventArgs.selected` condition.
1616

1717
### General
1818
- `igxButton`:

‎projects/igniteui-angular/src/lib/buttonGroup/buttonGroup.component.ts‎

Lines changed: 77 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -302,14 +302,20 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
302302
public selectedIndexes: number[] = [];
303303

304304
protected buttonClickNotifier$ = new Subject<boolean>();
305-
protected buttonSelectedNotifier$ = new Subject<boolean>();
306305
protected queryListNotifier$ = new Subject<boolean>();
307306

308307
private _isVertical: boolean;
309308
private _itemContentCssClass: string;
310309
private _disabled = false;
311310
private _selectionMode: 'single' | 'singleRequired' | 'multi' = 'single';
312311

312+
private mutationObserver: MutationObserver;
313+
private observerConfig: MutationObserverInit = {
314+
attributeFilter: ["data-selected"],
315+
childList: true,
316+
subtree: true,
317+
};
318+
313319
constructor(
314320
private _cdr: ChangeDetectorRef,
315321
private _renderer: Renderer2,
@@ -351,6 +357,8 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
351357
return;
352358
}
353359

360+
this.updateSelected(index);
361+
354362
const button = this.buttons[index];
355363
button.select();
356364
}
@@ -366,25 +374,21 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
366374
this.selectedIndexes.push(index);
367375
}
368376

369-
if (button.selected) {
370-
this._renderer.setAttribute(button.nativeElement, 'aria-pressed', 'true');
371-
this._renderer.addClass(button.nativeElement, 'igx-button-group__item--selected');
377+
this._renderer.setAttribute(button.nativeElement, 'aria-pressed', 'true');
378+
this._renderer.addClass(button.nativeElement, 'igx-button-group__item--selected');
372379

373-
const indexInViewButtons = this.viewButtons.toArray().indexOf(button);
374-
if (indexInViewButtons !== -1) {
380+
const indexInViewButtons = this.viewButtons.toArray().indexOf(button);
381+
if (indexInViewButtons !== -1) {
375382
this.values[indexInViewButtons].selected = true;
376-
}
383+
}
377384

378-
// deselect other buttons if selectionMode is not multi
379-
if (this.selectionMode !== 'multi' && this.selectedIndexes.length > 1) {
380-
this.buttons.forEach((_, i) => {
381-
if (i !== index && this.selectedIndexes.indexOf(i) !== -1) {
382-
this.deselectButton(i);
383-
}
384-
});
385-
}
386-
} else {
387-
this.deselectButton(index);
385+
// deselect other buttons if selectionMode is not multi
386+
if (this.selectionMode !== 'multi' && this.selectedIndexes.length > 1) {
387+
this.buttons.forEach((_, i) => {
388+
if (i !== index && this.selectedIndexes.indexOf(i) !== -1) {
389+
this.deselectButton(i);
390+
}
391+
});
388392
}
389393

390394
}
@@ -455,9 +459,6 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
455459
}
456460

457461
button.buttonClick.pipe(takeUntil(this.buttonClickNotifier$)).subscribe((_) => this._clickHandler(index));
458-
button.buttonSelected
459-
.pipe(takeUntil(this.buttonSelectedNotifier$))
460-
.subscribe((_) => this.updateSelected(index));
461462
});
462463
};
463464

@@ -466,6 +467,10 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
466467
initButtons();
467468

468469
this._cdr.detectChanges();
470+
471+
this.mutationObserver = this.setMutationsObserver();
472+
473+
this.mutationObserver.observe(this._el.nativeElement, this.observerConfig);
469474
}
470475

471476
/**
@@ -475,17 +480,18 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
475480
this.buttonClickNotifier$.next();
476481
this.buttonClickNotifier$.complete();
477482

478-
this.buttonSelectedNotifier$.next();
479-
this.buttonSelectedNotifier$.complete();
480-
481483
this.queryListNotifier$.next();
482484
this.queryListNotifier$.complete();
485+
486+
this.mutationObserver.disconnect();
483487
}
484488

485489
/**
486490
* @hidden
487491
*/
488492
public _clickHandler(index: number) {
493+
this.mutationObserver.disconnect();
494+
489495
const button = this.buttons[index];
490496
const args: IButtonGroupEventArgs = { owner: this, button, index };
491497

@@ -506,6 +512,54 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
506512
this.deselected.emit(args);
507513
}
508514
}
515+
516+
this.mutationObserver.observe(this._el.nativeElement, this.observerConfig);
517+
}
518+
519+
private setMutationsObserver() {
520+
return new MutationObserver((records, observer) => {
521+
// Stop observing while handling changes
522+
observer.disconnect();
523+
524+
const updatedButtons = this.getUpdatedButtons(records);
525+
526+
if (updatedButtons.length > 0) {
527+
updatedButtons.forEach((button) => {
528+
const index = this.buttons.map((b) => b.nativeElement).indexOf(button);
529+
const args: IButtonGroupEventArgs = { owner: this, button: this.buttons[index], index };
530+
531+
this.updateButtonSelectionState(index, args);
532+
});
533+
}
534+
535+
// Watch for changes again
536+
observer.observe(this._el.nativeElement, this.observerConfig);
537+
});
538+
}
539+
540+
private getUpdatedButtons(records: MutationRecord[]) {
541+
const updated: HTMLButtonElement[] = [];
542+
543+
records
544+
.filter((x) => x.type === 'attributes')
545+
.reduce((prev, curr) => {
546+
prev.push(
547+
curr.target as HTMLButtonElement
548+
);
549+
return prev;
550+
}, updated);
551+
552+
return updated;
553+
}
554+
555+
private updateButtonSelectionState(index: number, args: IButtonGroupEventArgs) {
556+
if (this.selectedIndexes.indexOf(index) === -1) {
557+
this.selectButton(index);
558+
this.selected.emit(args);
559+
} else {
560+
this.deselectButton(index);
561+
this.deselected.emit(args);
562+
}
509563
}
510564
}
511565

‎projects/igniteui-angular/src/lib/buttonGroup/buttongroup.component.spec.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Component, OnInit, ViewChild } from '@angular/core';
2-
import { TestBed, waitForAsync } from '@angular/core/testing';
2+
import { TestBed, fakeAsync, flushMicrotasks, waitForAsync } from '@angular/core/testing';
33
import { ButtonGroupAlignment, IgxButtonGroupComponent } from './buttonGroup.component';
44
import { configureTestSuite } from '../test-utils/configure-suite';
55
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
@@ -115,7 +115,11 @@ describe('IgxButtonGroup', () => {
115115

116116
const button = fixture.debugElement.nativeElement.querySelector('button');
117117
button.click();
118+
// The first button is already selected, so it should not fire the selected event, but the deselected one.
119+
expect(btnGroupInstance.selected.emit).not.toHaveBeenCalled();
118120

121+
const unselectedButton = fixture.debugElement.nativeElement.querySelector('#unselected');
122+
unselectedButton.click();
119123
expect(btnGroupInstance.selected.emit).toHaveBeenCalled();
120124
});
121125

@@ -360,7 +364,7 @@ describe('IgxButtonGroup', () => {
360364
}
361365
});
362366

363-
it('should style the corresponding button as deselected when the value bound to the selected input changes', () => {
367+
it('should style the corresponding button as deselected when the value bound to the selected input changes', fakeAsync(() => {
364368
const fixture = TestBed.createComponent(ButtonGroupButtonWithBoundSelectedOutputComponent);
365369
fixture.detectChanges();
366370

@@ -370,11 +374,13 @@ describe('IgxButtonGroup', () => {
370374
expect(btnGroupInstance.buttons[1].selected).toBe(true);
371375

372376
fixture.componentInstance.selectedValue = 100;
377+
flushMicrotasks();
373378
fixture.detectChanges();
374379

375-
expect(btnGroupInstance.selectedButtons.length).toBe(0);
376-
expect(btnGroupInstance.buttons[1].selected).toBe(false);
377-
});
380+
btnGroupInstance.buttons.forEach((button) => {
381+
expect(button.selected).toBe(false);
382+
});
383+
}));
378384

379385
});
380386

@@ -492,7 +498,7 @@ class TemplatedButtonGroupDesplayDensityComponent {
492498
template: `
493499
<igx-buttongroup>
494500
<button igxButton [selected]="true">Button 0</button>
495-
<button igxButton>Button 1</button>
501+
<button igxButton id="unselected">Button 1</button>
496502
<button igxButton>Button 2</button>
497503
</igx-buttongroup>
498504
`,

‎projects/igniteui-angular/src/lib/directives/button/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ this.button.displayDensity = "compact";
3131
| `igxButtonColor` | string | Set the button text color. You can pass any CSS valid color value. |
3232
| `igxButtonBackground` | string | Set the button background color. You can pass any CSS valid color value. |
3333
| `displayDensity` | DisplayDensity | Determines the display density of the button. |
34+
| `buttonSelected` | EventEmitter<IButtonEventArgs> | Emitted only when a button gets selected, or deselected, and not on initialization. |
35+
| `selected` | boolean | Gets or sets whether the button is selected. Mainly used in the IgxButtonGroup component and it will have no effect if set separately. |
3436

3537
# Button types
3638
| Name | Description |

‎projects/igniteui-angular/src/lib/directives/button/button.directive.spec.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,24 @@ describe('IgxButton', () => {
134134
expect(theButtonNativeEl.classList.length).toEqual(2);
135135
expect(theButtonNativeEl.classList).toContain(classes.flat);
136136
});
137+
138+
it('Should emit the buttonSelected event only on user interaction, not on initialization', () => {
139+
const fixture = TestBed.createComponent(InitButtonComponent);
140+
fixture.detectChanges();
141+
const button = fixture.componentInstance.button;
142+
spyOn(button.buttonSelected, 'emit');
143+
144+
button.ngOnInit();
145+
expect(button.buttonSelected.emit).not.toHaveBeenCalled();
146+
147+
button.nativeElement.click();
148+
fixture.detectChanges();
149+
expect(button.buttonSelected.emit).toHaveBeenCalledTimes(1);
150+
151+
button.nativeElement.click();
152+
fixture.detectChanges();
153+
expect(button.buttonSelected.emit).toHaveBeenCalledTimes(2);
154+
});
137155
});
138156

139157
@Component({

‎projects/igniteui-angular/src/lib/directives/button/button.directive.ts‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import {
88
Renderer2,
99
Optional,
1010
Inject,
11-
booleanAttribute
11+
booleanAttribute,
12+
AfterContentInit
1213
} from '@angular/core';
1314
import { DisplayDensityToken, IDisplayDensityOptions } from '../../core/density';
1415
import { mkenum } from '../../core/utils';
@@ -48,7 +49,7 @@ export type IgxButtonType = typeof IgxButtonType[keyof typeof IgxButtonType];
4849
selector: '[igxButton]',
4950
standalone: true
5051
})
51-
export class IgxButtonDirective extends IgxButtonBaseDirective {
52+
export class IgxButtonDirective extends IgxButtonBaseDirective implements AfterContentInit {
5253
private static ngAcceptInputType_type: IgxButtonType | '';
5354

5455
/**
@@ -106,13 +107,8 @@ export class IgxButtonDirective extends IgxButtonBaseDirective {
106107
@Input({ transform: booleanAttribute })
107108
public set selected(value: boolean) {
108109
if (this._selected !== value) {
109-
110110
this._selected = value;
111-
112-
this.buttonSelected.emit({
113-
button: this
114-
});
115-
111+
this._renderer.setAttribute(this.nativeElement, 'data-selected', value.toString());
116112
}
117113
}
118114

@@ -129,6 +125,14 @@ export class IgxButtonDirective extends IgxButtonBaseDirective {
129125
super(element, _displayDensityOptions);
130126
}
131127

128+
public ngAfterContentInit() {
129+
this.nativeElement.addEventListener('click', () => {
130+
this.buttonSelected.emit({
131+
button: this
132+
});
133+
});
134+
}
135+
132136
/**
133137
* Sets the type of the button.
134138
*
@@ -247,7 +251,7 @@ export class IgxButtonDirective extends IgxButtonBaseDirective {
247251
* @internal
248252
*/
249253
public deselect() {
250-
this._selected = false;
254+
this.selected = false;
251255
}
252256
}
253257

0 commit comments

Comments
 (0)