fix(core): clear recent styles after responsive deactivation (angular#927)

Fixes angular#697
Fixes angular#296

Author: CaerusKaru

src/lib/core/base/base2.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import {ElementRef, OnChanges, OnDestroy, SimpleChanges} from '@angular/core';
-import {Subject} from 'rxjs';
+import {Observable, Subject} from 'rxjs';
 
 import {StyleDefinition, StyleUtils} from '../style-utils/style-utils';
 import {StyleBuilder} from '../style-builder/style-builder';
@@ -17,6 +17,8 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy {
 
   protected DIRECTIVE_KEY = '';
   protected inputs: string[] = [];
+  /** The most recently used styles for the builder */
+  protected mru: StyleDefinition = {};
   protected destroySubject: Subject<void> = new Subject();
 
   /** Access to host element's parent DOM node */
@@ -64,6 +66,17 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy {
     this.marshal.releaseElement(this.nativeElement);
   }
 
+  /** Register with central marshaller service */
+  protected init(extraTriggers: Observable<any>[] = []): void {
+    this.marshal.init(
+      this.elementRef.nativeElement,
+      this.DIRECTIVE_KEY,
+      this.updateWithValue.bind(this),
+      this.clearStyles.bind(this),
+      extraTriggers
+    );
+  }
+
   /** Add styles to the element using predefined style builder */
   protected addStyles(input: string, parent?: Object) {
     const builder = this.styleBuilder;
@@ -78,10 +91,21 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy {
       }
     }
 
+    this.mru = {...genStyles};
     this.applyStyleToElement(genStyles);
     builder.sideEffect(input, genStyles, parent);
   }
 
+  /** Remove generated styles from an element using predefined style builder */
+  protected clearStyles() {
+    Object.keys(this.mru).forEach(k => {
+      this.mru[k] = '';
+    });
+    this.applyStyleToElement(this.mru);
+    this.mru = {};
+  }
+
+  /** Force trigger style updates on DOM element */
   protected triggerUpdate() {
     const val = this.marshal.getValue(this.nativeElement, this.DIRECTIVE_KEY);
     this.marshal.updateElement(this.nativeElement, this.DIRECTIVE_KEY, val);
@@ -119,4 +143,8 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy {
   protected setValue(val: any, bp: string): void {
     this.marshal.setValue(this.nativeElement, this.DIRECTIVE_KEY, val, bp);
   }
+
+  protected updateWithValue(input: string) {
+    this.addStyles(input);
+  }
 }

src/lib/core/media-marshaller/media-marshaller.spec.ts

@@ -66,7 +66,7 @@ describe('media-marshaller', () => {
     const builder = () => {
       triggered = true;
     };
-    mediaMarshaller.init(fakeElement, fakeKey, builder, [obs]);
+    mediaMarshaller.init(fakeElement, fakeKey, builder, () => {}, [obs]);
     subject.next();
     expect(triggered).toBeTruthy();
   });
@@ -119,7 +119,7 @@ describe('media-marshaller', () => {
     const builder = () => {
       triggered = true;
     };
-    mediaMarshaller.init(fakeElement, fakeKey, builder, [obs]);
+    mediaMarshaller.init(fakeElement, fakeKey, builder, () => {}, [obs]);
     mediaMarshaller.releaseElement(fakeElement);
     subject.next();
     expect(triggered).toBeFalsy();

src/lib/core/media-marshaller/media-marshaller.ts

@@ -16,9 +16,12 @@ import {MatchMedia} from '../match-media/match-media';
 import {MediaChange} from '../media-change';
 
 type Builder = Function;
+type ClearCallback = () => void;
+type UpdateCallback = (val: any) => void;
 type ValueMap = Map<string, string>;
 type BreakpointMap = Map<string, ValueMap>;
 type ElementMap = Map<HTMLElement, BreakpointMap>;
+type ElementKeyMap = WeakMap<HTMLElement, Set<string>>;
 type SubscriptionMap = Map<string, Subscription>;
 type WatcherMap = WeakMap<HTMLElement, SubscriptionMap>;
 type BuilderMap = WeakMap<HTMLElement, Map<string, Builder>>;
@@ -37,8 +40,11 @@ export interface ElementMatcher {
 export class MediaMarshaller {
   private activatedBreakpoints: BreakPoint[] = [];
   private elementMap: ElementMap = new Map();
+  private elementKeyMap: ElementKeyMap = new WeakMap();
+  // registry of special triggers to update elements
   private watcherMap: WatcherMap = new WeakMap();
   private builderMap: BuilderMap = new WeakMap();
+  private clearBuilderMap: BuilderMap = new WeakMap();
   private subject: Subject<ElementMatcher> = new Subject();
 
   get activatedBreakpoint(): string {
@@ -47,7 +53,9 @@ export class MediaMarshaller {
 
   constructor(protected matchMedia: MatchMedia,
               protected breakpoints: BreakPointRegistry) {
-    this.matchMedia.observe().subscribe(this.activate.bind(this));
+    this.matchMedia
+      .observe()
+      .subscribe(this.activate.bind(this));
     this.registerBreakpoints();
   }
 
@@ -71,36 +79,19 @@ export class MediaMarshaller {
    * initialize the marshaller with necessary elements for delegation on an element
    * @param element
    * @param key
-   * @param builder optional so that custom bp directives don't have to re-provide this
-   * @param observables
+   * @param updateFn optional callback so that custom bp directives don't have to re-provide this
+   * @param clearFn optional callback so that custom bp directives don't have to re-provide this
+   * @param extraTriggers other triggers to force style updates (e.g. layout, directionality, etc)
    */
   init(element: HTMLElement,
        key: string,
-       builder?: Builder,
-       observables: Observable<any>[] = []): void {
-    if (builder) {
-      let builders = this.builderMap.get(element);
-      if (!builders) {
-        builders = new Map();
-        this.builderMap.set(element, builders);
-      }
-      builders.set(key, builder);
-    }
-    if (observables) {
-      let watchers = this.watcherMap.get(element);
-      if (!watchers) {
-        watchers = new Map();
-        this.watcherMap.set(element, watchers);
-      }
-      const subscription = watchers.get(key);
-      if (!subscription) {
-        const newSubscription = merge(...observables).subscribe(() => {
-          const currentValue = this.getValue(element, key);
-          this.updateElement(element, key, currentValue);
-        });
-        watchers.set(key, newSubscription);
-      }
-    }
+       updateFn?: UpdateCallback,
+       clearFn?: ClearCallback,
+       extraTriggers: Observable<any>[] = []): void {
+    this.buildElementKeyMap(element, key);
+    initBuilderMap(this.builderMap, element, key, updateFn);
+    initBuilderMap(this.clearBuilderMap, element, key, clearFn);
+    this.watchExtraTriggers(element, key, extraTriggers);
   }
 
   /**
@@ -157,6 +148,7 @@ export class MediaMarshaller {
     this.updateElement(element, key, this.getValue(element, key));
   }
 
+  /** Track element value changes for a specific key */
   trackValue(element: HTMLElement, key: string): Observable<ElementMatcher> {
     return this.subject.asObservable()
       .pipe(filter(v => v.element === element && v.key === key));
@@ -166,12 +158,41 @@ export class MediaMarshaller {
   updateStyles(): void {
     this.elementMap.forEach((bpMap, el) => {
       const valueMap = this.getFallback(bpMap);
+      const keyMap = new Set(this.elementKeyMap.get(el)!);
       if (valueMap) {
-        valueMap.forEach((v, k) => this.updateElement(el, k, v));
+        valueMap.forEach((v, k) => {
+          this.updateElement(el, k, v);
+          keyMap.delete(k);
+        });
       }
+      keyMap.forEach(k => {
+        const fallbackMap = this.getFallback(bpMap, k);
+        if (fallbackMap) {
+          const value = fallbackMap.get(k);
+          this.updateElement(el, k, value);
+        } else {
+          this.clearElement(el, k);
+        }
+      });
     });
   }
 
+  /**
+   * clear the styles for a given element
+   * @param element
+   * @param key
+   */
+  clearElement(element: HTMLElement, key: string): void {
+    const builders = this.clearBuilderMap.get(element);
+    if (builders) {
+      const builder: Builder | undefined = builders.get(key);
+      if (builder) {
+        builder();
+        this.subject.next({element, key, value: ''});
+      }
+    }
+  }
+
   /**
    * update a given element with the activated values for a given key
    * @param element
@@ -206,6 +227,42 @@ export class MediaMarshaller {
     }
   }
 
+  /** Cross-reference for HTMLElement with directive key */
+  private buildElementKeyMap(element: HTMLElement, key: string) {
+    let keyMap = this.elementKeyMap.get(element);
+    if (!keyMap) {
+      keyMap = new Set();
+      this.elementKeyMap.set(element, keyMap);
+    }
+    keyMap.add(key);
+  }
+
+  /**
+   * Other triggers that should force style updates:
+   * - directionality
+   * - layout changes
+   * - mutationobserver updates
+   */
+  private watchExtraTriggers(element: HTMLElement,
+                             key: string,
+                             triggers: Observable<any>[]) {
+    if (triggers && triggers.length) {
+      let watchers = this.watcherMap.get(element);
+      if (!watchers) {
+        watchers = new Map();
+        this.watcherMap.set(element, watchers);
+      }
+      const subscription = watchers.get(key);
+      if (!subscription) {
+        const newSubscription = merge(...triggers).subscribe(() => {
+          const currentValue = this.getValue(element, key);
+          this.updateElement(element, key, currentValue);
+        });
+        watchers.set(key, newSubscription);
+      }
+    }
+  }
+
   /** Breakpoint locator by mediaQuery */
   private findByQuery(query: string) {
     return this.breakpoints.findByQuery(query);
@@ -234,3 +291,17 @@ export class MediaMarshaller {
     this.matchMedia.registerQuery(queries);
   }
 }
+
+function initBuilderMap(map: BuilderMap,
+                        element: HTMLElement,
+                        key: string,
+                        input?: UpdateCallback | ClearCallback): void {
+  if (input !== undefined) {
+    let oldMap = map.get(element);
+    if (!oldMap) {
+      oldMap = new Map();
+      map.set(element, oldMap);
+    }
+    oldMap.set(key, input);
+  }
+}

src/lib/extended/class/class.ts

@@ -48,7 +48,7 @@ export class ClassDirective extends BaseDirective2 implements DoCheck {
         this.iterableDiffers, this.keyValueDiffers, this.elementRef, this.renderer
       );
     }
-    this.marshal.init(this.nativeElement, this.DIRECTIVE_KEY, this.updateWithValue.bind(this));
+    this.init();
   }
 
   protected updateWithValue(value: any) {

src/lib/extended/img-src/img-src.ts

@@ -40,8 +40,7 @@ export class ImgSrcDirective extends BaseDirective2 {
               @Inject(PLATFORM_ID) protected platformId: Object,
               @Inject(SERVER_TOKEN) protected serverModuleLoaded: boolean) {
     super(elementRef, styleBuilder, styler, marshal);
-    this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY,
-      this.updateSrcFor.bind(this));
+    this.init();
     this.setValue('', this.nativeElement.getAttribute('src') || '');
     if (isPlatformServer(this.platformId) && this.serverModuleLoaded) {
       this.nativeElement.setAttribute('src', '');
@@ -56,7 +55,7 @@ export class ImgSrcDirective extends BaseDirective2 {
    * Do nothing to standard `<img src="">` usages, only when responsive
    * keys are present do we actually call `setAttribute()`
    */
-  protected updateSrcFor() {
+  protected updateWithValue() {
     let url = this.activatedValue || this.defaultSrc;
     if (isPlatformServer(this.platformId) && this.serverModuleLoaded) {
       this.addStyles(url);

src/lib/extended/show-hide/show-hide.ts

@@ -97,8 +97,7 @@ export class ShowHideDirective extends BaseDirective2 implements AfterViewInit,
       DISPLAY_MAP.set(this.nativeElement, this.display);
     }
 
-    this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY,
-      this.updateWithValue.bind(this));
+    this.init();
     // set the default to show unless explicitly overridden
     const defaultValue = this.marshal.getValue(this.nativeElement, this.DIRECTIVE_KEY, '');
     if (defaultValue === undefined || defaultValue === '') {

src/lib/extended/style/style.ts

@@ -49,7 +49,7 @@ export class StyleDirective extends BaseDirective2 implements DoCheck {
       // defined on the same host element; since the responsive variations may be defined...
       this.ngStyleInstance = new NgStyle(this.keyValueDiffers, this.elementRef, this.renderer);
     }
-    this.marshal.init(this.nativeElement, this.DIRECTIVE_KEY, this.updateWithValue.bind(this));
+    this.init();
     this.setValue(this.nativeElement.getAttribute('style') || '', '');
   }
 

src/lib/flex/flex-align/flex-align.ts

@@ -66,8 +66,7 @@ export class FlexAlignDirective extends BaseDirective2 {
               @Optional() protected styleBuilder: FlexAlignStyleBuilder,
               protected marshal: MediaMarshaller) {
     super(elRef, styleBuilder, styleUtils, marshal);
-    this.marshal.init(this.elRef.nativeElement, this.DIRECTIVE_KEY,
-      this.addStyles.bind(this));
+    this.init();
   }
 
   protected styleCache = flexAlignCache;

src/lib/flex/flex-offset/flex-offset.ts

@@ -76,10 +76,11 @@ export class FlexOffsetDirective extends BaseDirective2 implements OnChanges {
               protected marshal: MediaMarshaller,
               protected styler: StyleUtils) {
     super(elRef, styleBuilder, styler, marshal);
-    this.marshal.init(this.elRef.nativeElement, this.DIRECTIVE_KEY,
-      this.updateWithValue.bind(this), [this.directionality.change]);
+    this.init([this.directionality.change]);
+    // Parent DOM `layout-gap` with affect the nested child with `flex-offset`
     if (this.parentElement) {
-      this.marshal.trackValue(this.parentElement, 'layout-gap')
+      this.marshal
+        .trackValue(this.parentElement, 'layout-gap')
         .pipe(takeUntil(this.destroySubject))
         .subscribe(this.triggerUpdate.bind(this));
     }