Skip to content

Commit

Permalink
Merge pull request #193 from TAMULib/sprint9-staging-change-detection
Browse files Browse the repository at this point in the history
remove manual change detection
  • Loading branch information
wwelling committed Aug 13, 2020
2 parents 33624ed + ca8b28e commit f8dcf0c
Show file tree
Hide file tree
Showing 12 changed files with 30 additions and 55 deletions.
25 changes: 11 additions & 14 deletions projects/wvr-elements/src/lib/core/wvr-animation.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,19 @@ import { WvrBaseComponent } from '../shared/wvr-base.component';
})
export class WvrAnimationService {

private readonly recieversRegistry = new Map<string, Array<WvrBaseComponent>>();
private readonly _animationTargetsRegistry = new Map<string, Array<WvrBaseComponent>>();

private readonly animationStates = new Map<number, Map<string, boolean>>();

constructor(private readonly builder: AnimationBuilder) { }

registerAnimationTargets(recieverName: string, component: WvrBaseComponent): void {
let recievers = this.recieversRegistry.get(recieverName);
if (!recievers) {
recievers = new Array<WvrBaseComponent>();
this.recieversRegistry.set(recieverName, recievers);
registerAnimationTargets(targetName: string, component: WvrBaseComponent): void {
let targets = this._animationTargetsRegistry.get(targetName);
if (!targets) {
targets = new Array<WvrBaseComponent>();
this._animationTargetsRegistry.set(targetName, targets);
}
recievers.push(component);
targets.push(component);
}

registerAnimationStates(): number {
Expand All @@ -32,10 +32,10 @@ export class WvrAnimationService {
return id;
}

triggerAnimationReciever(recieverName: string): void {
const recievers = this.recieversRegistry.get(recieverName);
if (recievers) {
recievers.forEach(r => {
triggerAnimationTarget(targetName: string): void {
const targets = this._animationTargetsRegistry.get(targetName);
if (targets) {
targets.forEach(r => {
r.triggerAnimations('animationTrigger');
});
}
Expand All @@ -54,7 +54,6 @@ export class WvrAnimationService {
}

playAnimation(stateId: number, animationName: string, animationConfig: {}, animationRoot: HTMLElement): AnimationPlayer {

const timing = animationConfig[animationName] ?
animationConfig[animationName].timing :
wvrAnimationDefaults[animationName].timing;
Expand All @@ -77,7 +76,6 @@ export class WvrAnimationService {

private selectAnimation(stateId: number, animationName: string, timing: string,
to: string, from: string, animationRoot: HTMLElement): AnimationReferenceMetadata {

const animationInput: AnimationMetadata | Array<AnimationMetadata> =
this.compileAnimation(stateId, animationName, animationRoot);

Expand All @@ -91,7 +89,6 @@ export class WvrAnimationService {
}

private compileAnimation(stateId, animationName, value): AnimationMetadata | Array<AnimationMetadata> {

const a = wvrAnimations[animationName];

if (!a) {
Expand Down
12 changes: 4 additions & 8 deletions projects/wvr-elements/src/lib/shared/wvr-base.component.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { AfterContentInit, ChangeDetectorRef, Directive, ElementRef, EventEmitter, Injector, Input, OnInit, Output, ViewChild } from '@angular/core';
import { AfterContentInit, Directive, ElementRef, EventEmitter, Injector, Input, OnInit, Output, ViewChild } from '@angular/core';
import { DomSanitizer } from '@angular/platform-browser';
import * as JSON5 from 'json5';
import { fromEvent, Observable } from 'rxjs';
import { debounceTime, map } from 'rxjs/operators';
import { WvrAnimationService } from '../core/wvr-animation.service';
import * as JSON5 from 'json5';

@Directive()
// tslint:disable-next-line:directive-class-suffix
Expand All @@ -25,7 +25,7 @@ export abstract class WvrBaseComponent implements AfterContentInit, OnInit {

@Input() animateTarget: string;

@ViewChild('animationRoot') private animationRootElem: ElementRef;
@ViewChild('animationRoot') animationRootElem: ElementRef;

get isMobileAgent(): boolean {
const agent = navigator.userAgent || navigator.vendor || (window as any).opera;
Expand All @@ -44,14 +44,11 @@ export abstract class WvrBaseComponent implements AfterContentInit, OnInit {

protected readonly _eRef: ElementRef;

protected readonly _cdRef: ChangeDetectorRef;

@Output() protected readonly animationEventTrigger = new EventEmitter<Event>();

constructor(injector: Injector) {

this._animationService = injector.get(WvrAnimationService);
this._cdRef = injector.get(ChangeDetectorRef);
this._domSanitizer = injector.get(DomSanitizer);
this._eRef = injector.get(ElementRef);

Expand All @@ -75,7 +72,6 @@ export abstract class WvrBaseComponent implements AfterContentInit, OnInit {
}
this.screenSizeChanged$.subscribe(iml => {
this.isMobileLayout = iml;
this._cdRef.detectChanges();
});
this.isMobileLayout = this.checkScreenSize();
}
Expand All @@ -93,7 +89,7 @@ export abstract class WvrBaseComponent implements AfterContentInit, OnInit {
: [this._animationSettings[animationTriggerType]];
animations.forEach(an => {
if (an === 'animationTrigger') {
this._animationService.triggerAnimationReciever(this.animateTarget);
this._animationService.triggerAnimationTarget(this.animateTarget);
} else {
this._animationService
.playAnimation(this.animationStateId, an, this._animationConfig, this.animationRootElem.nativeElement);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div #WvrDropdown class="wvr-components wvr-dropdown d-inline-flex align-items-center">
<div ngbDropdown display="dynamic" class="d-inline-block" (openChange)="detectChanges()">
<div ngbDropdown display="dynamic" class="d-inline-block">
<wvr-button-element (click)="clickOpen($event)"
[background]="btnBackground"
[backgroundActive]="btnBackgroundActive"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
--wvr-dropdown-menu-padding: 25px;
--wvr-dropdown-menu-width: fit-content;
--wvr-dropdown-menu-x-offset: 0px;
--wvr-dropdown-menu-y-offset: -1px;
--wvr-dropdown-menu-y-offset: -6px;
--wvr-dropdown-menu-item-margin: 0 0 10px 0;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ChangeDetectionStrategy, CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
import { async, ComponentFixture, TestBed } from '@angular/core/testing';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
import { NgbDropdown } from '@ng-bootstrap/ng-bootstrap';
Expand All @@ -17,9 +17,6 @@ describe('WvrDropdownComponent', () => {
],
schemas: [CUSTOM_ELEMENTS_SCHEMA]
})
.overrideComponent(WvrDropdownComponent, {
set: { changeDetection: ChangeDetectionStrategy.Default }
})
.compileComponents();
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ export class WvrDropdownComponent extends WvrBaseComponent {
* Binds the input from `menu-y-offset` to the css variable `--wvr-dropdown-y-offset`.
* This css variable is applied by `margin-top` css rule to the menu.
*/
@HostBinding('style.--wvr-dropdown-menu-y-offset') @Input() menuYOffset = '34px';
@HostBinding('style.--wvr-dropdown-menu-y-offset') @Input() menuYOffset;

/**
* Binds the input from `item-margin` to the css variable `--wvr-dropdown-item-margin`.
Expand Down Expand Up @@ -357,11 +357,6 @@ export class WvrDropdownComponent extends WvrBaseComponent {
config.autoClose = false;
}

/** A utility method for manually detecting state changes */
detectChanges(): void {
this._cdRef.detectChanges();
}

/** An access method to expose the `isOpen` utility method from `NgbDropdown` */
isOpen(): boolean {
return this.dropdown ? this.dropdown.isOpen() : false;
Expand Down Expand Up @@ -415,7 +410,6 @@ export class WvrDropdownComponent extends WvrBaseComponent {
.click();
setTimeout(() => {
this.open = true;
this._cdRef.detectChanges();
this.dropdown.open();
}, this._animationSpeedMili);
}
Expand All @@ -425,7 +419,6 @@ export class WvrDropdownComponent extends WvrBaseComponent {
private closeDropdown(): void {
this.closing = true;
this.open = false;
this._cdRef.detectChanges();
setTimeout(() => {
this.dropdown.close();
this.closing = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export class WvrFooterComponent extends WvrBaseComponent implements OnInit {
const newIsSticky = this.parentElement.clientHeight <= compareHeight;
if (this.isSticky !== newIsSticky) {
this.isSticky = newIsSticky;
this._cdRef.detectChanges();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,17 @@ describe('WvrHeaderComponent', () => {
const bottomNavElement = fixture.nativeElement.querySelector('.bottom-nav') as HTMLElement;

component.displayBottomNav = 'true';
fixture.detectChanges();
expect(bottomNavElement.hasAttribute('hidden'))
.toEqual(false);

component.displayBottomNav = 'false';
fixture.detectChanges();
expect(bottomNavElement.hasAttribute('hidden'))
.toEqual(true);

component.displayBottomNav = undefined;
fixture.detectChanges();
expect(bottomNavElement.hasAttribute('hidden'))
.toEqual(true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ export class WvrHeaderComponent extends WvrBaseComponent implements OnInit, Afte
@Input() set displayBottomNav(value: 'true' | 'false') {
this._displayBottomNav = value;
this.checkBottomNavHasChildren();
this._cdRef.detectChanges();
}

get displayBottomNav(): 'true' | 'false' {
Expand All @@ -88,19 +87,16 @@ export class WvrHeaderComponent extends WvrBaseComponent implements OnInit, Afte
ngOnInit(): void {
super.ngOnInit();
this.checkBottomNavHasChildren();
this._cdRef.detectChanges();
}

ngAfterContentChecked(): void {
this.checkBottomNavHasChildren();
this._cdRef.detectChanges();
}

/** Determines if the bottom nav list has children in order to display bottom nav section. */
private checkBottomNavHasChildren(): void {
const bottomNavListElement = (this._eRef.nativeElement as HTMLElement).querySelector('.bottom-nav wvr-nav-li, .bottom-nav wvr-nav-li-element');
this.isBottomNavHidden = !(this.displayBottomNav === 'true' || (this.displayBottomNav === undefined && !!bottomNavListElement));
this._cdRef.detectChanges();
}

}
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
<wvr-icon-wrapper #animationRoot [innerHTML]="iconSvg">
<wvr-icon-wrapper #animationRoot [innerHTML]="iconSvg | async">
</wvr-icon-wrapper>
12 changes: 5 additions & 7 deletions projects/wvr-elements/src/lib/wvr-icon/wvr-icon.component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { AfterViewInit, Component, HostBinding, Injector, Input } from '@angular/core';
import { SafeHtml } from '@angular/platform-browser';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
import { IconService } from '../core/icon.service';
import { WvrBaseComponent } from '../shared/wvr-base.component';

Expand All @@ -18,19 +20,15 @@ export class WvrIconComponent extends WvrBaseComponent implements AfterViewInit

@HostBinding('style.--wvr-icon-size') @Input() size = '24px';

iconSvg: SafeHtml;
iconSvg: Observable<SafeHtml>;

constructor(injector: Injector, private readonly iconService: IconService) {
super(injector);
}

ngAfterViewInit(): void {
this.iconService.getIcon(this.set, this.name)
.toPromise()
.then(svg => {
this.iconSvg = this._domSanitizer.bypassSecurityTrustHtml(svg);
this._cdRef.detectChanges();
});
this.iconSvg = this.iconService.getIcon(this.set, this.name)
.pipe(map(svg => this._domSanitizer.bypassSecurityTrustHtml(svg)));
}

}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AfterViewInit, Component, Injector, Input, OnInit } from '@angular/core';
import { Component, Injector, Input, OnInit } from '@angular/core';
import { Theme } from '../../shared/theme.type';
import { WvrBaseComponent } from '../../shared/wvr-base.component';

Expand All @@ -7,7 +7,7 @@ import { WvrBaseComponent } from '../../shared/wvr-base.component';
templateUrl: './wvr-list-item.component.html',
styleUrls: ['./wvr-list-item.component.scss']
})
export class WvrListItemComponent extends WvrBaseComponent implements OnInit, AfterViewInit {
export class WvrListItemComponent extends WvrBaseComponent implements OnInit {

private _parent: HTMLElement;

Expand Down Expand Up @@ -37,8 +37,4 @@ export class WvrListItemComponent extends WvrBaseComponent implements OnInit, Af
this.context = contextAttribute ? contextAttribute : undefined;
}

ngAfterViewInit(): void {
this._cdRef.detectChanges();
}

}

0 comments on commit f8dcf0c

Please sign in to comment.