Skip to content

Commit

Permalink
116404: Replaced ViewChild logic with a CSS selector
Browse files Browse the repository at this point in the history
This change avoids triggering an additional change detection cycle
  • Loading branch information
alexandrevryghem committed Oct 27, 2024
1 parent 84df67c commit bc083a1
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -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)"
Expand All @@ -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">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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 },
Expand All @@ -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();
Expand Down Expand Up @@ -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 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';

/**
Expand All @@ -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
Expand Down Expand Up @@ -123,6 +118,14 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
}
}

ngOnDestroy(): void {
super.ngOnDestroy();
const dropdownItems: NodeListOf<HTMLElement> = document.querySelectorAll(`#${this.expandableNavbarSectionId()} *[role="menuitem"]`);
dropdownItems.forEach((item: HTMLElement) => {
item.removeEventListener('keydown', this.navigateDropdown.bind(this));
});
}

/**
* When the mouse enters the section toggler activate the menu section
* @param $event
Expand Down
18 changes: 10 additions & 8 deletions src/app/shared/utils/hover-outside.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]',
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -27,7 +26,6 @@ import { VarDirective } from '../../../../../app/shared/utils/var.directive';
NgFor,
NgIf,
RouterLinkActive,
VarDirective,
],
})
export class ExpandableNavbarSectionComponent extends BaseComponent {
Expand Down

0 comments on commit bc083a1

Please sign in to comment.