fix(buttonGroup): Resolve the issue with the emission of buttonSelected on init (#13861)

PlamenD95 · web-flow · commit 9831fcc46d17 · 2024-02-08T09:25:18.000+02:00
diff --git a/projects/igniteui-angular/src/lib/buttonGroup/buttonGroup.component.ts b/projects/igniteui-angular/src/lib/buttonGroup/buttonGroup.component.ts
@@ -301,14 +301,20 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
     public selectedIndexes: number[] = [];
 
     protected buttonClickNotifier$ = new Subject<boolean>();
-    protected buttonSelectedNotifier$ = new Subject<boolean>();
     protected queryListNotifier$ = new Subject<boolean>();
 
     private _isVertical: boolean;
     private _itemContentCssClass: string;
     private _disabled = false;
     private _selectionMode: 'single' | 'singleRequired' | 'multi' = 'single';
 
+    private mutationObserver: MutationObserver;
+    private observerConfig: MutationObserverInit = {
+      attributeFilter: ["data-selected"],
+      childList: true,
+      subtree: true,
+    };
+
     constructor(
         private _cdr: ChangeDetectorRef,
         private _renderer: Renderer2,
@@ -350,6 +356,8 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
             return;
         }
 
+        this.updateSelected(index);
+
         const button = this.buttons[index];
         button.select();
     }
@@ -365,25 +373,21 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
             this.selectedIndexes.push(index);
         }
 
-        if (button.selected) {
-            this._renderer.setAttribute(button.nativeElement, 'aria-pressed', 'true');
-            this._renderer.addClass(button.nativeElement, 'igx-button-group__item--selected');
+        this._renderer.setAttribute(button.nativeElement, 'aria-pressed', 'true');
+        this._renderer.addClass(button.nativeElement, 'igx-button-group__item--selected');
 
-            const indexInViewButtons = this.viewButtons.toArray().indexOf(button);
-            if (indexInViewButtons !== -1) {
+        const indexInViewButtons = this.viewButtons.toArray().indexOf(button);
+        if (indexInViewButtons !== -1) {
             this.values[indexInViewButtons].selected = true;
-            }
+        }
 
-            // deselect other buttons if selectionMode is not multi
-            if (this.selectionMode !== 'multi' && this.selectedIndexes.length > 1) {
-                this.buttons.forEach((_, i) => {
-                    if (i !== index && this.selectedIndexes.indexOf(i) !== -1) {
-                        this.deselectButton(i);
-                    }
-                });
-            }
-        } else {
-            this.deselectButton(index);
+        // deselect other buttons if selectionMode is not multi
+        if (this.selectionMode !== 'multi' && this.selectedIndexes.length > 1) {
+            this.buttons.forEach((_, i) => {
+                if (i !== index && this.selectedIndexes.indexOf(i) !== -1) {
+                    this.deselectButton(i);
+                }
+            });
         }
     }
 
@@ -453,9 +457,6 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
                 }
 
                 button.buttonClick.pipe(takeUntil(this.buttonClickNotifier$)).subscribe((_) => this._clickHandler(index));
-                button.buttonSelected
-                    .pipe(takeUntil(this.buttonSelectedNotifier$))
-                    .subscribe((_) => this.updateSelected(index));
             });
         };
 
@@ -464,6 +465,10 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
         initButtons();
 
         this._cdr.detectChanges();
+
+        this.mutationObserver = this.setMutationsObserver();
+
+        this.mutationObserver.observe(this._el.nativeElement, this.observerConfig);
     }
 
     /**
@@ -473,17 +478,18 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
         this.buttonClickNotifier$.next();
         this.buttonClickNotifier$.complete();
 
-        this.buttonSelectedNotifier$.next();
-        this.buttonSelectedNotifier$.complete();
-
         this.queryListNotifier$.next();
         this.queryListNotifier$.complete();
+
+        this.mutationObserver.disconnect();
     }
 
     /**
      * @hidden
      */
     public _clickHandler(index: number) {
+        this.mutationObserver.disconnect();
+
         const button = this.buttons[index];
         const args: IButtonGroupEventArgs = { owner: this, button, index };
 
@@ -504,6 +510,54 @@ export class IgxButtonGroupComponent extends DisplayDensityBase implements After
                 this.deselected.emit(args);
             }
         }
+
+        this.mutationObserver.observe(this._el.nativeElement, this.observerConfig);
+    }
+
+    private setMutationsObserver() {
+        return new MutationObserver((records, observer) => {
+            // Stop observing while handling changes
+            observer.disconnect();
+
+            const updatedButtons = this.getUpdatedButtons(records);
+
+            if (updatedButtons.length > 0) {
+                updatedButtons.forEach((button) => {
+                    const index = this.buttons.map((b) => b.nativeElement).indexOf(button);
+                    const args: IButtonGroupEventArgs = { owner: this, button: this.buttons[index], index };
+
+                    this.updateButtonSelectionState(index, args);
+                });
+            }
+
+            // Watch for changes again
+            observer.observe(this._el.nativeElement, this.observerConfig);
+        });
+    }
+
+    private getUpdatedButtons(records: MutationRecord[]) {
+        const updated: HTMLButtonElement[] = [];
+
+        records
+          .filter((x) => x.type === 'attributes')
+          .reduce((prev, curr) => {
+            prev.push(
+              curr.target as HTMLButtonElement
+            );
+            return prev;
+          }, updated);
+
+        return updated;
+    }
+
+    private updateButtonSelectionState(index: number, args: IButtonGroupEventArgs) {
+        if (this.selectedIndexes.indexOf(index) === -1) {
+            this.selectButton(index);
+            this.selected.emit(args);
+        } else {
+            this.deselectButton(index);
+            this.deselected.emit(args);
+        }
     }
 }
 
diff --git a/projects/igniteui-angular/src/lib/buttonGroup/buttongroup.component.spec.ts b/projects/igniteui-angular/src/lib/buttonGroup/buttongroup.component.spec.ts
@@ -1,5 +1,5 @@
 import { Component, OnInit, ViewChild } from '@angular/core';
-import { TestBed, waitForAsync } from '@angular/core/testing';
+import { TestBed, fakeAsync, flushMicrotasks, waitForAsync } from '@angular/core/testing';
 import { ButtonGroupAlignment, IgxButtonGroupComponent } from './buttonGroup.component';
 import { configureTestSuite } from '../test-utils/configure-suite';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
@@ -115,7 +115,11 @@ describe('IgxButtonGroup', () => {
 
         const button = fixture.debugElement.nativeElement.querySelector('button');
         button.click();
+        // The first button is already selected, so it should not fire the selected event, but the deselected one.
+        expect(btnGroupInstance.selected.emit).not.toHaveBeenCalled();
 
+        const unselectedButton = fixture.debugElement.nativeElement.querySelector('#unselected');
+        unselectedButton.click();
         expect(btnGroupInstance.selected.emit).toHaveBeenCalled();
     });
 
@@ -360,7 +364,7 @@ describe('IgxButtonGroup', () => {
         }
     });
 
-    it('should style the corresponding button as deselected when the value bound to the selected input changes', () => {
+    it('should style the corresponding button as deselected when the value bound to the selected input changes', fakeAsync(() => {
         const fixture = TestBed.createComponent(ButtonGroupButtonWithBoundSelectedOutputComponent);
         fixture.detectChanges();
 
@@ -370,11 +374,13 @@ describe('IgxButtonGroup', () => {
         expect(btnGroupInstance.buttons[1].selected).toBe(true);
 
         fixture.componentInstance.selectedValue = 100;
+        flushMicrotasks();
         fixture.detectChanges();
 
-        expect(btnGroupInstance.selectedButtons.length).toBe(0);
-        expect(btnGroupInstance.buttons[1].selected).toBe(false);
-    });
+        btnGroupInstance.buttons.forEach((button) => {
+            expect(button.selected).toBe(false);
+        });
+    }));
 
 });
 
@@ -492,7 +498,7 @@ class TemplatedButtonGroupDesplayDensityComponent {
     template: `
     <igx-buttongroup>
         <button igxButton [selected]="true">Button 0</button>
-        <button igxButton>Button 1</button>
+        <button igxButton id="unselected">Button 1</button>
         <button igxButton>Button 2</button>
     </igx-buttongroup>
     `,
diff --git a/projects/igniteui-angular/src/lib/directives/button/README.md b/projects/igniteui-angular/src/lib/directives/button/README.md
@@ -31,6 +31,8 @@ this.button.displayDensity = "compact";
 | `igxButtonColor` |    string   |   Set the button text color. You can pass any CSS valid color value. |
 | `igxButtonBackground` | string | Set the button background color. You can pass any CSS valid color value. |
 | `displayDensity` | DisplayDensity | Determines the display density of the button. |
+| `buttonSelected` | EventEmitter<IButtonEventArgs> | Emitted only when a button gets selected, or deselected, and not on initialization. |
+| `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. |
 
 # Button types
 | Name   | Description |
diff --git a/projects/igniteui-angular/src/lib/directives/button/button.directive.spec.ts b/projects/igniteui-angular/src/lib/directives/button/button.directive.spec.ts
@@ -146,6 +146,24 @@ describe('IgxButton', () => {
         expect(theButtonNativeEl.classList.length).toEqual(2);
         expect(theButtonNativeEl.classList).toContain(classes.flat);
     });
+
+    it('Should emit the buttonSelected event only on user interaction, not on initialization', () => {
+        const fixture = TestBed.createComponent(InitButtonComponent);
+        fixture.detectChanges();
+        const button = fixture.componentInstance.button;
+        spyOn(button.buttonSelected, 'emit');
+
+        button.ngOnInit();
+        expect(button.buttonSelected.emit).not.toHaveBeenCalled();
+
+        button.nativeElement.click();
+        fixture.detectChanges();
+        expect(button.buttonSelected.emit).toHaveBeenCalledTimes(1);
+
+        button.nativeElement.click();
+        fixture.detectChanges();
+        expect(button.buttonSelected.emit).toHaveBeenCalledTimes(2);
+    });
 });
 
 @Component({
diff --git a/projects/igniteui-angular/src/lib/directives/button/button.directive.ts b/projects/igniteui-angular/src/lib/directives/button/button.directive.ts
@@ -8,7 +8,8 @@ import {
     Renderer2,
     HostListener,
     Optional,
-    Inject
+    Inject,
+    AfterContentInit
 } from '@angular/core';
 import { DisplayDensityBase, DisplayDensityToken, IDisplayDensityOptions } from '../../core/density';
 import { mkenum } from '../../core/utils';
@@ -50,7 +51,7 @@ export type IgxButtonType = typeof IgxButtonType[keyof typeof IgxButtonType];
     selector: '[igxButton]',
     standalone: true
 })
-export class IgxButtonDirective extends DisplayDensityBase {
+export class IgxButtonDirective extends DisplayDensityBase implements AfterContentInit {
     private static ngAcceptInputType_type: IgxButtonType | '';
     private static ngAcceptInputType_disabled: boolean | '';
 
@@ -135,9 +136,7 @@ export class IgxButtonDirective extends DisplayDensityBase {
         if(this._selected !== value) {
             this._selected = value;
 
-            this.buttonSelected.emit({
-                button: this
-            });
+            this._renderer.setAttribute(this.nativeElement, 'data-selected', value.toString());
         }
     }
 
@@ -153,6 +152,14 @@ export class IgxButtonDirective extends DisplayDensityBase {
         super(_displayDensityOptions, element);
     }
 
+    public ngAfterContentInit() {
+        this.nativeElement.addEventListener('click', () => {
+            this.buttonSelected.emit({
+                button: this
+            });
+        });
+    }
+
     /**
      * @hidden
      * @internal
@@ -329,7 +336,7 @@ export class IgxButtonDirective extends DisplayDensityBase {
      * @internal
      */
     public deselect() {
-        this._selected = false;
+        this.selected = false;
     }
 }
 
