Skip to content

Commit a984957

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 daf2f50 commit a984957

File tree

5 files changed

+56
-30
lines changed

5 files changed

+56
-30
lines changed

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: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { HostWindowService } from '../../shared/host-window.service';
1212
import { MenuService } from '../../shared/menu/menu.service';
1313
import { HostWindowServiceStub } from '../../shared/testing/host-window-service.stub';
1414
import { MenuServiceStub } from '../../shared/testing/menu-service.stub';
15-
import { VarDirective } from '../../shared/utils/var.directive';
15+
import { HoverOutsideDirective } from '../../shared/utils/hover-outside.directive';
1616
import { ExpandableNavbarSectionComponent } from './expandable-navbar-section.component';
1717

1818
describe('ExpandableNavbarSectionComponent', () => {
@@ -23,7 +23,12 @@ describe('ExpandableNavbarSectionComponent', () => {
2323
describe('on larger screens', () => {
2424
beforeEach(waitForAsync(() => {
2525
TestBed.configureTestingModule({
26-
imports: [NoopAnimationsModule, ExpandableNavbarSectionComponent, TestComponent, VarDirective],
26+
imports: [
27+
ExpandableNavbarSectionComponent,
28+
HoverOutsideDirective,
29+
NoopAnimationsModule,
30+
TestComponent,
31+
],
2732
providers: [
2833
{ provide: 'sectionDataProvider', useValue: {} },
2934
{ provide: MenuService, useValue: menuService },
@@ -41,10 +46,6 @@ describe('ExpandableNavbarSectionComponent', () => {
4146
fixture.detectChanges();
4247
});
4348

44-
it('should create', () => {
45-
expect(component).toBeTruthy();
46-
});
47-
4849
describe('when the mouse enters the section header (while inactive)', () => {
4950
beforeEach(() => {
5051
spyOn(component, 'onMouseEnter').and.callThrough();
@@ -184,7 +185,12 @@ describe('ExpandableNavbarSectionComponent', () => {
184185
describe('on smaller, mobile screens', () => {
185186
beforeEach(waitForAsync(() => {
186187
TestBed.configureTestingModule({
187-
imports: [NoopAnimationsModule, ExpandableNavbarSectionComponent, TestComponent, VarDirective],
188+
imports: [
189+
ExpandableNavbarSectionComponent,
190+
HoverOutsideDirective,
191+
NoopAnimationsModule,
192+
TestComponent,
193+
],
188194
providers: [
189195
{ provide: 'sectionDataProvider', useValue: {} },
190196
{ provide: MenuService, useValue: menuService },

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

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@ import {
77
import {
88
AfterViewChecked,
99
Component,
10-
ElementRef,
1110
HostListener,
1211
Inject,
1312
Injector,
13+
OnDestroy,
1414
OnInit,
15-
ViewChild,
1615
} from '@angular/core';
1716
import { RouterLinkActive } from '@angular/router';
1817
import { Observable } from 'rxjs';
@@ -24,7 +23,6 @@ import { MenuService } from '../../shared/menu/menu.service';
2423
import { MenuID } from '../../shared/menu/menu-id.model';
2524
import { MenuSection } from '../../shared/menu/menu-section.model';
2625
import { HoverOutsideDirective } from '../../shared/utils/hover-outside.directive';
27-
import { VarDirective } from '../../shared/utils/var.directive';
2826
import { NavbarSectionComponent } from '../navbar-section/navbar-section.component';
2927

3028
/**
@@ -43,12 +41,9 @@ import { NavbarSectionComponent } from '../navbar-section/navbar-section.compone
4341
NgFor,
4442
NgIf,
4543
RouterLinkActive,
46-
VarDirective,
4744
],
4845
})
49-
export class ExpandableNavbarSectionComponent extends NavbarSectionComponent implements AfterViewChecked, OnInit {
50-
51-
@ViewChild('expandableNavbarSectionContainer') expandableNavbarSection: ElementRef;
46+
export class ExpandableNavbarSectionComponent extends NavbarSectionComponent implements AfterViewChecked, OnInit, OnDestroy {
5247

5348
/**
5449
* This section resides in the Public Navbar
@@ -77,6 +72,11 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
7772
*/
7873
addArrowEventListeners = false;
7974

75+
/**
76+
* List of current dropdown items who have event listeners
77+
*/
78+
private dropdownItems: NodeListOf<HTMLElement>;
79+
8080
@HostListener('window:resize', ['$event'])
8181
onResize() {
8282
this.isMobile$.pipe(
@@ -106,23 +106,43 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
106106
this.subs.push(this.active$.subscribe((active: boolean) => {
107107
if (active === true) {
108108
this.addArrowEventListeners = true;
109+
} else {
110+
this.unsubscribeFromEventListeners();
109111
}
110112
}));
111113
}
112114

113115
ngAfterViewChecked(): void {
114116
if (this.addArrowEventListeners) {
115-
const dropdownItems: NodeListOf<HTMLElement> = document.querySelectorAll(`#${this.expandableNavbarSectionId()} *[role="menuitem"]`);
116-
dropdownItems.forEach((item: HTMLElement) => {
117+
this.dropdownItems = document.querySelectorAll(`#${this.expandableNavbarSectionId()} *[role="menuitem"]`);
118+
this.dropdownItems.forEach((item: HTMLElement) => {
117119
item.addEventListener('keydown', this.navigateDropdown.bind(this));
118120
});
119-
if (dropdownItems.length > 0) {
120-
dropdownItems.item(0).focus();
121+
if (this.dropdownItems.length > 0) {
122+
this.dropdownItems.item(0).focus();
121123
}
122124
this.addArrowEventListeners = false;
123125
}
124126
}
125127

128+
ngOnDestroy(): void {
129+
super.ngOnDestroy();
130+
this.unsubscribeFromEventListeners();
131+
}
132+
133+
/**
134+
* Removes all the current event listeners on the dropdown items (called when the menu is closed & on component
135+
* destruction)
136+
*/
137+
unsubscribeFromEventListeners(): void {
138+
if (this.dropdownItems) {
139+
this.dropdownItems.forEach((item: HTMLElement) => {
140+
item.removeEventListener('keydown', this.navigateDropdown.bind(this));
141+
});
142+
this.dropdownItems = undefined;
143+
}
144+
}
145+
126146
/**
127147
* When the mouse enters the section toggler activate the menu section
128148
* @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]',
@@ -26,22 +27,23 @@ export class HoverOutsideDirective {
2627
public dsHoverOutside = new EventEmitter();
2728

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

3537
constructor(
3638
private elementRef: ElementRef,
3739
) {
38-
this.dsHoverOutsideOfElement = this.elementRef;
3940
}
4041

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

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

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { RouterLinkActive } from '@angular/router';
1010
import { ExpandableNavbarSectionComponent as BaseComponent } from '../../../../../app/navbar/expandable-navbar-section/expandable-navbar-section.component';
1111
import { slide } from '../../../../../app/shared/animations/slide';
1212
import { HoverOutsideDirective } from '../../../../../app/shared/utils/hover-outside.directive';
13-
import { VarDirective } from '../../../../../app/shared/utils/var.directive';
1413

1514
@Component({
1615
selector: 'ds-themed-expandable-navbar-section',
@@ -27,7 +26,6 @@ import { VarDirective } from '../../../../../app/shared/utils/var.directive';
2726
NgFor,
2827
NgIf,
2928
RouterLinkActive,
30-
VarDirective,
3129
],
3230
})
3331
export class ExpandableNavbarSectionComponent extends BaseComponent {

0 commit comments

Comments
 (0)