From 9c6fb1794f1329870092433a78bdcbdd0b5f5729 Mon Sep 17 00:00:00 2001
From: Alexandre Vryghem <alexandre@atmire.com>
Date: Sun, 27 Oct 2024 14:11:05 +0100
Subject: [PATCH] 116404: Replaced ViewChild logic with a CSS selector

This change avoids triggering an additional change detection cycle

(cherry picked from commit f7bb83013a7b6c49fee3695668c66129b4078f5c)
---
 .../expandable-navbar-section.component.html  |  4 +-
 ...xpandable-navbar-section.component.spec.ts | 18 +++++----
 .../expandable-navbar-section.component.ts    | 39 +++++++++++++++----
 .../shared/utils/hover-outside.directive.ts   | 18 +++++----
 4 files changed, 54 insertions(+), 25 deletions(-)

diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html
index f8facaa82a..0af5c706b7 100644
--- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html
+++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html
@@ -1,4 +1,4 @@
-<div #expandableNavbarSectionContainer class="ds-menu-item-wrapper text-md-center"
+<div class="ds-menu-item-wrapper text-md-center"
      [id]="'expandable-navbar-section-' + section.id"
      (mouseenter)="onMouseEnter($event)"
      (mouseleave)="onMouseLeave($event)"
@@ -23,7 +23,7 @@
   </a>
   <div *ngIf="(active$ | async).valueOf() === true" (click)="deactivateSection($event)"
        [id]="expandableNavbarSectionId()"
-       [dsHoverOutsideOfElement]="expandableNavbarSection"
+       [dsHoverOutsideOfParentSelector]="'#expandable-navbar-section-' + section.id"
        (dsHoverOutside)="deactivateSection($event, false)"
        role="menu"
        class="dropdown-menu show nav-dropdown-menu m-0 shadow-none border-top-0 px-3 px-md-0 pt-0 pt-md-1">
diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts
index 986dca86eb..2b7c665258 100644
--- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts
+++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts
@@ -9,7 +9,7 @@ import { HostWindowService } from '../../shared/host-window.service';
 import { MenuService } from '../../shared/menu/menu.service';
 import { HostWindowServiceStub } from '../../shared/testing/host-window-service.stub';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
-import { VarDirective } from '../../shared/utils/var.directive';
+import { HoverOutsideDirective } from '../../shared/utils/hover-outside.directive';
 
 describe('ExpandableNavbarSectionComponent', () => {
   let component: ExpandableNavbarSectionComponent;
@@ -20,7 +20,11 @@ describe('ExpandableNavbarSectionComponent', () => {
     beforeEach(waitForAsync(() => {
       TestBed.configureTestingModule({
         imports: [NoopAnimationsModule],
-        declarations: [ExpandableNavbarSectionComponent, TestComponent, VarDirective],
+        declarations: [
+          ExpandableNavbarSectionComponent,
+          HoverOutsideDirective,
+          TestComponent,
+        ],
         providers: [
           { provide: 'sectionDataProvider', useValue: {} },
           { provide: MenuService, useValue: menuService },
@@ -43,10 +47,6 @@ describe('ExpandableNavbarSectionComponent', () => {
       fixture.detectChanges();
     });
 
-    it('should create', () => {
-      expect(component).toBeTruthy();
-    });
-
     describe('when the mouse enters the section header (while inactive)', () => {
       beforeEach(() => {
         spyOn(component, 'onMouseEnter').and.callThrough();
@@ -187,7 +187,11 @@ describe('ExpandableNavbarSectionComponent', () => {
     beforeEach(waitForAsync(() => {
       TestBed.configureTestingModule({
         imports: [NoopAnimationsModule],
-        declarations: [ExpandableNavbarSectionComponent, TestComponent, VarDirective],
+        declarations: [
+          ExpandableNavbarSectionComponent,
+          HoverOutsideDirective,
+          TestComponent,
+        ],
         providers: [
           { provide: 'sectionDataProvider', useValue: {} },
           { provide: MenuService, useValue: menuService },
diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts
index 72e33b1f18..063b080c62 100644
--- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts
+++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts
@@ -1,4 +1,4 @@
-import { Component, HostListener, Inject, Injector, OnInit, ViewChild, AfterViewChecked, ElementRef } from '@angular/core';
+import { Component, HostListener, Inject, Injector, OnInit, AfterViewChecked, OnDestroy } from '@angular/core';
 import { NavbarSectionComponent } from '../navbar-section/navbar-section.component';
 import { MenuService } from '../../shared/menu/menu.service';
 import { slide } from '../../shared/animations/slide';
@@ -17,9 +17,7 @@ import { MenuSection } from '../../shared/menu/menu-section.model';
   styleUrls: ['./expandable-navbar-section.component.scss'],
   animations: [slide]
 })
-export class ExpandableNavbarSectionComponent extends NavbarSectionComponent implements AfterViewChecked, OnInit {
-
-  @ViewChild('expandableNavbarSectionContainer') expandableNavbarSection: ElementRef;
+export class ExpandableNavbarSectionComponent extends NavbarSectionComponent implements AfterViewChecked, OnInit, OnDestroy {
 
   /**
    * This section resides in the Public Navbar
@@ -48,6 +46,11 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
    */
   addArrowEventListeners = false;
 
+  /**
+   * List of current dropdown items who have event listeners
+   */
+  private dropdownItems: NodeListOf<HTMLElement>;
+
   @HostListener('window:resize', ['$event'])
   onResize() {
     this.isMobile$.pipe(
@@ -77,23 +80,43 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
     this.subs.push(this.active$.subscribe((active: boolean) => {
       if (active === true) {
         this.addArrowEventListeners = true;
+      } else {
+        this.unsubscribeFromEventListeners();
       }
     }));
   }
 
   ngAfterViewChecked(): void {
     if (this.addArrowEventListeners) {
-      const dropdownItems: NodeListOf<HTMLElement> = document.querySelectorAll(`#${this.expandableNavbarSectionId()} *[role="menuitem"]`);
-      dropdownItems.forEach((item: HTMLElement) => {
+      this.dropdownItems = document.querySelectorAll(`#${this.expandableNavbarSectionId()} *[role="menuitem"]`);
+      this.dropdownItems.forEach((item: HTMLElement) => {
         item.addEventListener('keydown', this.navigateDropdown.bind(this));
       });
-      if (dropdownItems.length > 0) {
-        dropdownItems.item(0).focus();
+      if (this.dropdownItems.length > 0) {
+        this.dropdownItems.item(0).focus();
       }
       this.addArrowEventListeners = false;
     }
   }
 
+  ngOnDestroy(): void {
+    super.ngOnDestroy();
+    this.unsubscribeFromEventListeners();
+  }
+
+  /**
+   * Removes all the current event listeners on the dropdown items (called when the menu is closed & on component
+   * destruction)
+   */
+  unsubscribeFromEventListeners(): void {
+    if (this.dropdownItems) {
+      this.dropdownItems.forEach((item: HTMLElement) => {
+        item.removeEventListener('keydown', this.navigateDropdown.bind(this));
+      });
+      this.dropdownItems = undefined;
+    }
+  }
+
   /**
    * When the mouse enters the section toggler activate the menu section
    * @param $event
diff --git a/src/app/shared/utils/hover-outside.directive.ts b/src/app/shared/utils/hover-outside.directive.ts
index 7a4bd58540..fd6676cbe7 100644
--- a/src/app/shared/utils/hover-outside.directive.ts
+++ b/src/app/shared/utils/hover-outside.directive.ts
@@ -8,10 +8,11 @@ import {
 } from '@angular/core';
 
 /**
- * Directive to detect when the user hovers outside of the element the directive was put on
+ * Directive to detect when the user hovers outside the element the directive was put on
  *
- * BEWARE: it's probably not good for performance to use this excessively (on {@link ExpandableNavbarSectionComponent}
- * for example, a workaround for this problem was to add an `*ngIf` to prevent this Directive from always being active)
+ * **Performance Consideration**: it's probably not good for performance to use this excessively (on
+ * {@link ExpandableNavbarSectionComponent} for example, a workaround for this problem was to add an `*ngIf` to prevent
+ * this Directive from always being active)
  */
 @Directive({
   selector: '[dsHoverOutside]',
@@ -25,22 +26,23 @@ export class HoverOutsideDirective {
   public dsHoverOutside = new EventEmitter();
 
   /**
-   * The {@link ElementRef} for which this directive should emit when the mouse leaves it. By default this will be the
-   * element the directive was put on.
+   * CSS selector for the parent element to monitor. If set, the directive will use this
+   * selector to determine if the hover event originated within the selected parent element.
+   * If left unset, the directive will monitor mouseover hover events for the element it is applied to.
    */
   @Input()
-  public dsHoverOutsideOfElement: ElementRef;
+  public dsHoverOutsideOfParentSelector: string;
 
   constructor(
     private elementRef: ElementRef,
   ) {
-    this.dsHoverOutsideOfElement = this.elementRef;
   }
 
   @HostListener('document:mouseover', ['$event'])
   public onMouseOver(event: MouseEvent): void {
     const targetElement: HTMLElement = event.target as HTMLElement;
-    const hoveredInside = this.dsHoverOutsideOfElement.nativeElement.contains(targetElement);
+    const element: Element = document.querySelector(this.dsHoverOutsideOfParentSelector);
+    const hoveredInside = (element ? new ElementRef(element) : this.elementRef).nativeElement.contains(targetElement);
 
     if (!hoveredInside) {
       this.dsHoverOutside.emit(null);