refactor: move dynamic CSS/Style component to use direct element upda…

…te (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
shlomiassaf · Aug 23, 2016 · 7fb6c12 · 7fb6c12
1 parent ab4d51a
commit 7fb6c12
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 138 deletions.
diff --git 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,22 +22,29 @@ 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
  */
 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
@@ -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
@@ -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<any>,
-              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
@@ -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,54 +35,39 @@ export class ModalOverlay extends BaseDynamicComponent {
   constructor(private dialogRef: DialogRef<any>,
               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<T>(type: any, bindings?: ResolvedReflectiveProvider[]): ComponentRef<T> {
     return super._addComponent<T>(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%',
       top: 0,
       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
@@ -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<MessageModalPreset>,
-              el: ElementRef, sanitizer: DomSanitizationService) {
-    super(sanitizer, el);
+              el: ElementRef, renderer: Renderer) {
+    super(el, renderer);
   }
 
   addComponent<T>(type: any, bindings?: ResolvedReflectiveProvider[]): ComponentRef<T> {

diff --git a/src/components/angular2-modal/plugins/bootstrap/modal.ts b/src/components/angular2-modal/plugins/bootstrap/modal.ts
@@ -61,34 +61,23 @@ 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();
     }
 
     overlay.beforeDestroy(() => {
       const completer = new PromiseCompleter<void>();
-
       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
@@ -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<void>();
       container.animationEnd$.first().subscribe(type => {
         // on removal, remove if last.