From f7bb83013a7b6c49fee3695668c66129b4078f5c 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
---
 .../expandable-navbar-section.component.html  |  4 +-
 ...xpandable-navbar-section.component.spec.ts | 20 +++++----
 .../expandable-navbar-section.component.ts    | 42 ++++++++++++++-----
 .../shared/utils/hover-outside.directive.ts   | 18 ++++----
 .../expandable-navbar-section.component.ts    |  2 -
 5 files changed, 56 insertions(+), 30 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 f8facaa82a7..0af5c706b76 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 d03c8d89eb1..91d5a7f1eda 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
@@ -12,7 +12,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 { MenuServiceStub } from '../../shared/testing/menu-service.stub';
-import { VarDirective } from '../../shared/utils/var.directive';
+import { HoverOutsideDirective } from '../../shared/utils/hover-outside.directive';
 import { ExpandableNavbarSectionComponent } from './expandable-navbar-section.component';
 
 describe('ExpandableNavbarSectionComponent', () => {
@@ -23,7 +23,12 @@ describe('ExpandableNavbarSectionComponent', () => {
   describe('on larger screens', () => {
     beforeEach(waitForAsync(() => {
       TestBed.configureTestingModule({
-        imports: [NoopAnimationsModule, ExpandableNavbarSectionComponent, TestComponent, VarDirective],
+        imports: [
+          ExpandableNavbarSectionComponent,
+          HoverOutsideDirective,
+          NoopAnimationsModule,
+          TestComponent,
+        ],
         providers: [
           { provide: 'sectionDataProvider', useValue: {} },
           { provide: MenuService, useValue: menuService },
@@ -41,10 +46,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();
@@ -184,7 +185,12 @@ describe('ExpandableNavbarSectionComponent', () => {
   describe('on smaller, mobile screens', () => {
     beforeEach(waitForAsync(() => {
       TestBed.configureTestingModule({
-        imports: [NoopAnimationsModule, ExpandableNavbarSectionComponent, TestComponent, VarDirective],
+        imports: [
+          ExpandableNavbarSectionComponent,
+          HoverOutsideDirective,
+          NoopAnimationsModule,
+          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 e546585c07b..b28aa258048 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
@@ -7,12 +7,11 @@ import {
 import {
   AfterViewChecked,
   Component,
-  ElementRef,
   HostListener,
   Inject,
   Injector,
+  OnDestroy,
   OnInit,
-  ViewChild,
 } from '@angular/core';
 import { RouterLinkActive } from '@angular/router';
 import { Observable } from 'rxjs';
@@ -24,7 +23,6 @@ import { MenuService } from '../../shared/menu/menu.service';
 import { MenuID } from '../../shared/menu/menu-id.model';
 import { MenuSection } from '../../shared/menu/menu-section.model';
 import { HoverOutsideDirective } from '../../shared/utils/hover-outside.directive';
-import { VarDirective } from '../../shared/utils/var.directive';
 import { NavbarSectionComponent } from '../navbar-section/navbar-section.component';
 
 /**
@@ -43,12 +41,9 @@ import { NavbarSectionComponent } from '../navbar-section/navbar-section.compone
     NgFor,
     NgIf,
     RouterLinkActive,
-    VarDirective,
   ],
 })
-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
@@ -77,6 +72,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(
@@ -106,23 +106,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 a0abb8bb63e..e046d3a0878 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]',
@@ -26,22 +27,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);
diff --git a/src/themes/custom/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts b/src/themes/custom/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts
index 167b805fd58..b0b57a75aae 100644
--- a/src/themes/custom/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts
+++ b/src/themes/custom/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts
@@ -10,7 +10,6 @@ import { RouterLinkActive } from '@angular/router';
 import { ExpandableNavbarSectionComponent as BaseComponent } from '../../../../../app/navbar/expandable-navbar-section/expandable-navbar-section.component';
 import { slide } from '../../../../../app/shared/animations/slide';
 import { HoverOutsideDirective } from '../../../../../app/shared/utils/hover-outside.directive';
-import { VarDirective } from '../../../../../app/shared/utils/var.directive';
 
 @Component({
   selector: 'ds-themed-expandable-navbar-section',
@@ -27,7 +26,6 @@ import { VarDirective } from '../../../../../app/shared/utils/var.directive';
     NgFor,
     NgIf,
     RouterLinkActive,
-    VarDirective,
   ],
 })
 export class ExpandableNavbarSectionComponent extends BaseComponent {