Skip to content

Commit 9c6fb17

Browse files
116404: Replaced ViewChild logic with a CSS selector
This change avoids triggering an additional change detection cycle (cherry picked from commit f7bb830)
1 parent 0f27ae1 commit 9c6fb17

4 files changed

Lines changed: 54 additions & 25 deletions

File tree

src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<div #expandableNavbarSectionContainer class="ds-menu-item-wrapper text-md-center"
1+
<div class="ds-menu-item-wrapper text-md-center"
22
[id]="'expandable-navbar-section-' + section.id"
33
(mouseenter)="onMouseEnter($event)"
44
(mouseleave)="onMouseLeave($event)"
@@ -23,7 +23,7 @@
2323
</a>
2424
<div *ngIf="(active$ | async).valueOf() === true" (click)="deactivateSection($event)"
2525
[id]="expandableNavbarSectionId()"
26-
[dsHoverOutsideOfElement]="expandableNavbarSection"
26+
[dsHoverOutsideOfParentSelector]="'#expandable-navbar-section-' + section.id"
2727
(dsHoverOutside)="deactivateSection($event, false)"
2828
role="menu"
2929
class="dropdown-menu show nav-dropdown-menu m-0 shadow-none border-top-0 px-3 px-md-0 pt-0 pt-md-1">

src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { HostWindowService } from '../../shared/host-window.service';
99
import { MenuService } from '../../shared/menu/menu.service';
1010
import { HostWindowServiceStub } from '../../shared/testing/host-window-service.stub';
1111
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
12-
import { VarDirective } from '../../shared/utils/var.directive';
12+
import { HoverOutsideDirective } from '../../shared/utils/hover-outside.directive';
1313

1414
describe('ExpandableNavbarSectionComponent', () => {
1515
let component: ExpandableNavbarSectionComponent;
@@ -20,7 +20,11 @@ describe('ExpandableNavbarSectionComponent', () => {
2020
beforeEach(waitForAsync(() => {
2121
TestBed.configureTestingModule({
2222
imports: [NoopAnimationsModule],
23-
declarations: [ExpandableNavbarSectionComponent, TestComponent, VarDirective],
23+
declarations: [
24+
ExpandableNavbarSectionComponent,
25+
HoverOutsideDirective,
26+
TestComponent,
27+
],
2428
providers: [
2529
{ provide: 'sectionDataProvider', useValue: {} },
2630
{ provide: MenuService, useValue: menuService },
@@ -43,10 +47,6 @@ describe('ExpandableNavbarSectionComponent', () => {
4347
fixture.detectChanges();
4448
});
4549

46-
it('should create', () => {
47-
expect(component).toBeTruthy();
48-
});
49-
5050
describe('when the mouse enters the section header (while inactive)', () => {
5151
beforeEach(() => {
5252
spyOn(component, 'onMouseEnter').and.callThrough();
@@ -187,7 +187,11 @@ describe('ExpandableNavbarSectionComponent', () => {
187187
beforeEach(waitForAsync(() => {
188188
TestBed.configureTestingModule({
189189
imports: [NoopAnimationsModule],
190-
declarations: [ExpandableNavbarSectionComponent, TestComponent, VarDirective],
190+
declarations: [
191+
ExpandableNavbarSectionComponent,
192+
HoverOutsideDirective,
193+
TestComponent,
194+
],
191195
providers: [
192196
{ provide: 'sectionDataProvider', useValue: {} },
193197
{ provide: MenuService, useValue: menuService },

src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Component, HostListener, Inject, Injector, OnInit, ViewChild, AfterViewChecked, ElementRef } from '@angular/core';
1+
import { Component, HostListener, Inject, Injector, OnInit, AfterViewChecked, OnDestroy } from '@angular/core';
22
import { NavbarSectionComponent } from '../navbar-section/navbar-section.component';
33
import { MenuService } from '../../shared/menu/menu.service';
44
import { slide } from '../../shared/animations/slide';
@@ -17,9 +17,7 @@ import { MenuSection } from '../../shared/menu/menu-section.model';
1717
styleUrls: ['./expandable-navbar-section.component.scss'],
1818
animations: [slide]
1919
})
20-
export class ExpandableNavbarSectionComponent extends NavbarSectionComponent implements AfterViewChecked, OnInit {
21-
22-
@ViewChild('expandableNavbarSectionContainer') expandableNavbarSection: ElementRef;
20+
export class ExpandableNavbarSectionComponent extends NavbarSectionComponent implements AfterViewChecked, OnInit, OnDestroy {
2321

2422
/**
2523
* This section resides in the Public Navbar
@@ -48,6 +46,11 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
4846
*/
4947
addArrowEventListeners = false;
5048

49+
/**
50+
* List of current dropdown items who have event listeners
51+
*/
52+
private dropdownItems: NodeListOf<HTMLElement>;
53+
5154
@HostListener('window:resize', ['$event'])
5255
onResize() {
5356
this.isMobile$.pipe(
@@ -77,23 +80,43 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
7780
this.subs.push(this.active$.subscribe((active: boolean) => {
7881
if (active === true) {
7982
this.addArrowEventListeners = true;
83+
} else {
84+
this.unsubscribeFromEventListeners();
8085
}
8186
}));
8287
}
8388

8489
ngAfterViewChecked(): void {
8590
if (this.addArrowEventListeners) {
86-
const dropdownItems: NodeListOf<HTMLElement> = document.querySelectorAll(`#${this.expandableNavbarSectionId()} *[role="menuitem"]`);
87-
dropdownItems.forEach((item: HTMLElement) => {
91+
this.dropdownItems = document.querySelectorAll(`#${this.expandableNavbarSectionId()} *[role="menuitem"]`);
92+
this.dropdownItems.forEach((item: HTMLElement) => {
8893
item.addEventListener('keydown', this.navigateDropdown.bind(this));
8994
});
90-
if (dropdownItems.length > 0) {
91-
dropdownItems.item(0).focus();
95+
if (this.dropdownItems.length > 0) {
96+
this.dropdownItems.item(0).focus();
9297
}
9398
this.addArrowEventListeners = false;
9499
}
95100
}
96101

102+
ngOnDestroy(): void {
103+
super.ngOnDestroy();
104+
this.unsubscribeFromEventListeners();
105+
}
106+
107+
/**
108+
* Removes all the current event listeners on the dropdown items (called when the menu is closed & on component
109+
* destruction)
110+
*/
111+
unsubscribeFromEventListeners(): void {
112+
if (this.dropdownItems) {
113+
this.dropdownItems.forEach((item: HTMLElement) => {
114+
item.removeEventListener('keydown', this.navigateDropdown.bind(this));
115+
});
116+
this.dropdownItems = undefined;
117+
}
118+
}
119+
97120
/**
98121
* When the mouse enters the section toggler activate the menu section
99122
* @param $event

src/app/shared/utils/hover-outside.directive.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@ import {
88
} from '@angular/core';
99

1010
/**
11-
* Directive to detect when the user hovers outside of the element the directive was put on
11+
* Directive to detect when the user hovers outside the element the directive was put on
1212
*
13-
* BEWARE: it's probably not good for performance to use this excessively (on {@link ExpandableNavbarSectionComponent}
14-
* for example, a workaround for this problem was to add an `*ngIf` to prevent this Directive from always being active)
13+
* **Performance Consideration**: it's probably not good for performance to use this excessively (on
14+
* {@link ExpandableNavbarSectionComponent} for example, a workaround for this problem was to add an `*ngIf` to prevent
15+
* this Directive from always being active)
1516
*/
1617
@Directive({
1718
selector: '[dsHoverOutside]',
@@ -25,22 +26,23 @@ export class HoverOutsideDirective {
2526
public dsHoverOutside = new EventEmitter();
2627

2728
/**
28-
* The {@link ElementRef} for which this directive should emit when the mouse leaves it. By default this will be the
29-
* element the directive was put on.
29+
* CSS selector for the parent element to monitor. If set, the directive will use this
30+
* selector to determine if the hover event originated within the selected parent element.
31+
* If left unset, the directive will monitor mouseover hover events for the element it is applied to.
3032
*/
3133
@Input()
32-
public dsHoverOutsideOfElement: ElementRef;
34+
public dsHoverOutsideOfParentSelector: string;
3335

3436
constructor(
3537
private elementRef: ElementRef,
3638
) {
37-
this.dsHoverOutsideOfElement = this.elementRef;
3839
}
3940

4041
@HostListener('document:mouseover', ['$event'])
4142
public onMouseOver(event: MouseEvent): void {
4243
const targetElement: HTMLElement = event.target as HTMLElement;
43-
const hoveredInside = this.dsHoverOutsideOfElement.nativeElement.contains(targetElement);
44+
const element: Element = document.querySelector(this.dsHoverOutsideOfParentSelector);
45+
const hoveredInside = (element ? new ElementRef(element) : this.elementRef).nativeElement.contains(targetElement);
4446

4547
if (!hoveredInside) {
4648
this.dsHoverOutside.emit(null);

0 commit comments

Comments
 (0)