From 7fb6c12ebbce541df0e6c25767e2df35a5c9c4b2 Mon Sep 17 00:00:00 2001 From: "Shlomi Assaf (shlassaf)" Date: Tue, 23 Aug 2016 22:54:53 +0300 Subject: [PATCH] refactor: move dynamic CSS/Style component to use direct element update (via renderer) instead of binding. Binding is bad for animation and since the whole purpose of this base class is to support animation direct access is needed. fix: fix backdrop "on show" flicker in bootstrap plugin --- .../components/base-dynamic-component.ts | 100 ++++++------------ .../angular2-modal/components/css-backdrop.ts | 14 +-- .../components/css-dialog-container.ts | 8 +- .../overlay/overlay.component.ts | 38 ++----- .../bootstrap/modal-container.component.ts | 8 +- .../angular2-modal/plugins/bootstrap/modal.ts | 21 +--- .../angular2-modal/plugins/vex/modal.ts | 9 -- 7 files changed, 60 insertions(+), 138 deletions(-) diff --git a/src/components/angular2-modal/components/base-dynamic-component.ts b/src/components/angular2-modal/components/base-dynamic-component.ts index f13b52ca..46beff34 100644 --- a/src/components/angular2-modal/components/base-dynamic-component.ts +++ b/src/components/angular2-modal/components/base-dynamic-component.ts @@ -4,13 +4,13 @@ import { ElementRef, ResolvedReflectiveProvider, OnDestroy, - ViewContainerRef + ViewContainerRef, + Renderer } from '@angular/core'; import { Observable } from 'rxjs/Observable'; import { Subject } from 'rxjs/Subject'; -import { DomSanitizationService, SafeStyle } from '@angular/platform-browser'; import { createComponent } from '../framework/createComponent'; const BROWSER_PREFIX = ['webkit', 'moz', 'MS', 'o', '']; @@ -22,7 +22,19 @@ function register(eventName, element, cb) { } /** - * A base class that expose customisation methods in derived components. + * A base class for supporting dynamic components. + * There are 3 main support areas: + * 1 - Easy wrapper for dynamic styling via CSS classes and inline styles. + * 2 - Easy wrapper for interception of transition/animation end events. + * 3 - Easy wrapper for component creation and injection. + * + * Dynamic css is done via direct element manipulation (via renderer), it does not use change detection + * or binding. This is to allow better control over animation. + * + * Animation support is limited, only transition/keyframes END even are notified. + * The animation support is needed since currently the angular animation module is limited as well and + * does not support CSS animation that are not pre-parsed and are not in the styles metadata of a component. + * * Capabilities: Add/Remove styls, Add/Remove classes, listen to animation/transition end event, * add components */ @@ -30,14 +42,9 @@ export class BaseDynamicComponent implements OnDestroy { animationEnd$: Observable<'transition' | 'animation'>; protected animationEnd: Subject<'transition' | 'animation'>; - protected style: { [prop: string]: string } = {} as any; - protected styleStr: SafeStyle = ''; - protected cssClass: SafeStyle = ''; - protected classArray: string[] = []; - private applyOnNextTurn: boolean; - - constructor(protected sanitizer: DomSanitizationService, - protected el: ElementRef) {} + + constructor(protected el: ElementRef, + protected renderer: Renderer) {} activateAnimationListener() { if (this.animationEnd) return; @@ -53,36 +60,24 @@ export class BaseDynamicComponent implements OnDestroy { * @returns {ModalOverlay} */ setStyle(prop: string, value: string): this { - if (this.style[prop] !== value) { - if (value === undefined) { - delete this.style[prop]; - } else { - this.style[prop] = value; - } - - this.applyStyle(); - } + this.renderer.setElementStyle(this.el.nativeElement, prop, value); return this; } - - /** - * Remove's all inline styles from the overlay host element. - */ - clearStyles(): void { - this.style = {} as any; - this.applyStyle(); + + forceReflow() { + this.el.nativeElement.offsetWidth; } - addClass(css: string, nextTurn: boolean = false): void { - if (typeof css === 'string') { - css.split(' ').forEach( c => this._addClass(c, nextTurn) ); - } + addClass(css: string, forceReflow: boolean = false): void { + css.split(' ') + .forEach( c => this.renderer.setElementClass(this.el.nativeElement, c, true) ); + if (forceReflow) this.forceReflow(); } - removeClass(css: string, nextTurn: boolean = false): void { - if (typeof css === 'string') { - css.split(' ').forEach( c => this._removeClass(c, nextTurn) ); - } + removeClass(css: string, forceReflow: boolean = false): void { + css.split(' ') + .forEach( c => this.renderer.setElementClass(this.el.nativeElement, c, false) ); + if (forceReflow) this.forceReflow(); } ngOnDestroy(): void { @@ -91,28 +86,6 @@ export class BaseDynamicComponent implements OnDestroy { } } - protected applyStyle(): void { - this.styleStr = this.sanitizer.bypassSecurityTrustStyle(JSON.stringify(this.style) - .replace('{', '') - .replace('}', '') - .replace(/,/g, ';') - .replace(/"/g, '')); - } - - protected applyClasses(nextTurn: boolean) { - if (nextTurn === true) { - if (!this.applyOnNextTurn) { - this.applyOnNextTurn = true; - setTimeout(() => { - this.applyOnNextTurn = false; - this.applyClasses(false); - }); - } - } else { - this.cssClass = this.classArray.join(' '); - } - } - /** * Add a component, supply a view container ref. * Note: The components vcRef will result in a sibling. @@ -145,17 +118,4 @@ export class BaseDynamicComponent implements OnDestroy { } } - private _addClass(css: string, nextTurn: boolean = false): void { - if (this.classArray.indexOf(css) > -1) return; - this.classArray.push(css); - this.applyClasses(nextTurn); - } - - private _removeClass(css: string, nextTurn: boolean = false): void { - const idx = this.classArray.indexOf(css); - if (idx > -1) { - this.classArray.splice(idx, 1); - this.applyClasses(nextTurn); - } - } } diff --git a/src/components/angular2-modal/components/css-backdrop.ts b/src/components/angular2-modal/components/css-backdrop.ts index e46a5e8e..d22ce734 100644 --- a/src/components/angular2-modal/components/css-backdrop.ts +++ b/src/components/angular2-modal/components/css-backdrop.ts @@ -1,9 +1,9 @@ import { Component, ElementRef, - ViewEncapsulation + ViewEncapsulation, + Renderer } from '@angular/core'; -import { DomSanitizationService } from '@angular/platform-browser'; import { BaseDynamicComponent } from './base-dynamic-component'; @@ -21,17 +21,17 @@ import { BaseDynamicComponent } from './base-dynamic-component'; }) export class CSSBackdrop extends BaseDynamicComponent { - constructor(el: ElementRef, sanitizer: DomSanitizationService) { - super(sanitizer, el); + constructor(el: ElementRef, renderer: Renderer) { + super(el, renderer); this.activateAnimationListener(); - this.style = { + const style = { position: 'absolute', top: 0, left: 0, width: '100%', height: '100%' - } as any; - this.applyStyle(); + }; + Object.keys(style).forEach( k => this.setStyle(k, style[k]) ); } } diff --git a/src/components/angular2-modal/components/css-dialog-container.ts b/src/components/angular2-modal/components/css-dialog-container.ts index 89207b1d..aaa6c8bd 100644 --- a/src/components/angular2-modal/components/css-dialog-container.ts +++ b/src/components/angular2-modal/components/css-dialog-container.ts @@ -5,9 +5,9 @@ import { ResolvedReflectiveProvider, ViewChild, ViewEncapsulation, - ElementRef + ElementRef, + Renderer } from '@angular/core'; -import { DomSanitizationService } from '@angular/platform-browser'; import { BaseDynamicComponent } from './base-dynamic-component'; import { DialogRef } from '../models/dialog-ref'; @@ -34,8 +34,8 @@ export class CSSDialogContainer extends BaseDynamicComponent { @ViewChild('modalDialog', {read: ViewContainerRef}) private vcRef: ViewContainerRef; constructor(public dialog: DialogRef, - el: ElementRef, sanitizer: DomSanitizationService) { - super(sanitizer, el); + el: ElementRef, renderer: Renderer) { + super(el, renderer); this.activateAnimationListener(); } diff --git a/src/components/angular2-modal/overlay/overlay.component.ts b/src/components/angular2-modal/overlay/overlay.component.ts index e3ef90c3..a88bfa75 100644 --- a/src/components/angular2-modal/overlay/overlay.component.ts +++ b/src/components/angular2-modal/overlay/overlay.component.ts @@ -8,11 +8,10 @@ import { ResolvedReflectiveProvider, ViewChild, ViewContainerRef, - ViewEncapsulation + ViewEncapsulation, + Renderer } from '@angular/core'; -import { DomSanitizationService } from '@angular/platform-browser'; - import { PromiseCompleter, supportsKey } from '../framework/utils'; import { DialogRef } from '../models/dialog-ref'; import { BaseDynamicComponent } from '../components/index'; @@ -24,8 +23,6 @@ import { BaseDynamicComponent } from '../components/index'; @Component({ selector: 'modal-overlay', host: { - '[attr.style]': 'styleStr', - '[attr.class]': 'cssClass', '(body:keydown)': 'documentKeypress($event)' }, encapsulation: ViewEncapsulation.None, @@ -38,45 +35,30 @@ export class ModalOverlay extends BaseDynamicComponent { constructor(private dialogRef: DialogRef, private appRef: ApplicationRef, el: ElementRef, - sanitizer: DomSanitizationService) { - super(sanitizer, el); + renderer: Renderer) { + super(el, renderer); this.activateAnimationListener(); } - /** - * Performs an ApplicationRef.tick - * - */ - tick(): void { - // this.cdr.markForCheck(); - // this.cdr.detectChanges(); - this.appRef.tick(); - - // TODO: - // Change detection doesn't run after doing some operations in plugins. - // this function is a workaround for those situations. (see bootstrap/vex modal implementations) - // strange enough, only ApplicationRef.tick() works, the CDR does not... probably due to - // the need to trigger from a higher change detector, needs investigation. - } addComponent(type: any, bindings?: ResolvedReflectiveProvider[]): ComponentRef { return super._addComponent(type, this.vcRef, bindings); } fullscreen(): void { - this.style = { + const style = { position: 'fixed', top: 0, left: 0, bottom: 0, right: 0, 'z-index': 1500 - } as any; - this.applyStyle(); + }; + Object.keys(style).forEach( k => this.setStyle(k, style[k]) ); } insideElement(): void { - this.style = { + const style = { position: 'absolute', width: '100%', height: '100%', @@ -84,8 +66,8 @@ export class ModalOverlay extends BaseDynamicComponent { left: 0, bottom: 0, right: 0 - } as any; - this.applyStyle(); + }; + Object.keys(style).forEach( k => this.setStyle(k, style[k]) ); } /** diff --git a/src/components/angular2-modal/plugins/bootstrap/modal-container.component.ts b/src/components/angular2-modal/plugins/bootstrap/modal-container.component.ts index 47b07d65..3859eb90 100644 --- a/src/components/angular2-modal/plugins/bootstrap/modal-container.component.ts +++ b/src/components/angular2-modal/plugins/bootstrap/modal-container.component.ts @@ -5,9 +5,9 @@ import { ViewContainerRef, ViewEncapsulation, ResolvedReflectiveProvider, - ComponentRef + ComponentRef, + Renderer } from '@angular/core'; -import { DomSanitizationService } from '@angular/platform-browser'; import { BaseDynamicComponent, DialogRef } from '../../../../components/angular2-modal'; @@ -29,8 +29,8 @@ export class BSModalContainer extends BaseDynamicComponent { @ViewChild('dlg', {read: ViewContainerRef}) private vcRef: ViewContainerRef; constructor(public dialog: DialogRef, - el: ElementRef, sanitizer: DomSanitizationService) { - super(sanitizer, el); + el: ElementRef, renderer: Renderer) { + super(el, renderer); } addComponent(type: any, bindings?: ResolvedReflectiveProvider[]): ComponentRef { diff --git a/src/components/angular2-modal/plugins/bootstrap/modal.ts b/src/components/angular2-modal/plugins/bootstrap/modal.ts index 68775296..82d39027 100644 --- a/src/components/angular2-modal/plugins/bootstrap/modal.ts +++ b/src/components/angular2-modal/plugins/bootstrap/modal.ts @@ -61,16 +61,14 @@ export class Modal extends Modal_ { document.body.classList.add('modal-open'); } - // on removal, remove if last. - // dialogRef.onDestroy - // .subscribe(() => this.overlay.groupStackLength(dialogRef) === 0 && document.body.classList.remove('modal-open')); - backdrop.addClass('modal-backdrop fade'); - backdrop.addClass('in', true); - container.addClass('modal fade'); + backdrop.addClass('modal-backdrop fade', true); container.setStyle('position', 'absolute'); container.setStyle('display', 'block'); - container.addClass('in', true); + container.addClass('modal fade', true); + + backdrop.addClass('in'); + container.addClass('in'); if (refs.containerRef.location.nativeElement) { refs.containerRef.location.nativeElement.focus(); @@ -78,17 +76,8 @@ export class Modal extends Modal_ { overlay.beforeDestroy(() => { const completer = new PromiseCompleter(); - backdrop.removeClass('in'); container.removeClass('in'); - // TODO: - // Change detection doesn't run after removing these classes, not even in 'nextTurn' - // e.g: backdrop.removeClass('in', true); - // the only solution is to change immediately and tick the change detection. - // this only happen when clicking outside of the bounds (overlayDialogBoundary). - // oddly using ChangeDetectorRef.detectChanges() doesn't work... ??? - // running inside zone didn't help. - overlay.tick(); backdrop.animationEnd$.first().subscribe(type => { this.overlay.groupStackLength(dialogRef) === 1 && document.body.classList.remove('modal-open'); diff --git a/src/components/angular2-modal/plugins/vex/modal.ts b/src/components/angular2-modal/plugins/vex/modal.ts index 332ed8f7..4e4a130b 100644 --- a/src/components/angular2-modal/plugins/vex/modal.ts +++ b/src/components/angular2-modal/plugins/vex/modal.ts @@ -74,15 +74,6 @@ export class Modal extends Modal_ { overlay.beforeDestroy(() => { overlay.addClass('vex-closing'); - // TODO: - // Change detection doesn't run after removing these classes, not even in 'nextTurn' - // e.g: backdrop.removeClass('in', true); - // the only solution is to change immediately and tick the change detection. - // This happen for every click (unlike bootstrap plugin). - // oddly using ChangeDetectorRef.detectChanges() doesn't work... ??? - // running inside zone didn't help. - overlay.tick(); - const completer = new PromiseCompleter(); container.animationEnd$.first().subscribe(type => { // on removal, remove if last.