From fee3bf213d885fe5ca42ed43803bff0deb618e1b Mon Sep 17 00:00:00 2001 From: Adam Plumer Date: Sat, 15 Dec 2018 00:13:46 -0600 Subject: [PATCH 1/4] fix(fxFlexOrder): remove order after responsive deactivation --- src/lib/flex/flex-order/flex-order.spec.ts | 87 ++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 src/lib/flex/flex-order/flex-order.spec.ts diff --git a/src/lib/flex/flex-order/flex-order.spec.ts b/src/lib/flex/flex-order/flex-order.spec.ts new file mode 100644 index 000000000..a619626b5 --- /dev/null +++ b/src/lib/flex/flex-order/flex-order.spec.ts @@ -0,0 +1,87 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import {Component} from '@angular/core'; +import {ComponentFixture, inject, TestBed} from '@angular/core/testing'; +import {CommonModule} from '@angular/common'; +import { + MatchMedia, + MockMatchMedia, + MockMatchMediaProvider, + SERVER_TOKEN, + StyleUtils, +} from '@angular/flex-layout/core'; + +import {customMatchers} from '../../utils/testing/custom-matchers'; +import {FlexLayoutModule} from '../../module'; +import {expectNativeEl, makeCreateTestComponent} from '../../utils/testing/helpers'; + +describe('flex-order', () => { + let fixture: ComponentFixture; + let matchMedia: MockMatchMedia; + let styler: StyleUtils; + let createTestComponent = (template: string) => { + fixture = makeCreateTestComponent(() => TestOrderComponent)(template); + + inject([MatchMedia, StyleUtils], (_matchMedia: MockMatchMedia, _styler: StyleUtils) => { + matchMedia = _matchMedia; + styler = _styler; + })(); + }; + + beforeEach(() => { + jasmine.addMatchers(customMatchers); + TestBed.configureTestingModule({ + declarations: [TestOrderComponent], + imports: [CommonModule, FlexLayoutModule], + providers: [ + MockMatchMediaProvider, + {provide: SERVER_TOKEN, useValue: true}, + ], + }); + }); + + describe('static API', () => { + it('should add correct static values', () => { + createTestComponent(`
`); + expectNativeEl(fixture).toHaveStyle({ + 'order': '1', + }, styler); + }); + }); + + describe('responsive API', () => { + it('should add correct responsive values', () => { + createTestComponent(`
`); + expectNativeEl(fixture).not.toHaveStyle({ + 'order': '1', + }, styler); + matchMedia.activate('lt-sm', true); + expectNativeEl(fixture).toHaveStyle({ + 'order': '1', + }, styler); + matchMedia.activate('sm'); + expectNativeEl(fixture).not.toHaveStyle({ + 'order': '1', + }, styler); + }); + }); +}); + + +// ***************************************************************** +// Template Component +// ***************************************************************** + +@Component({ + selector: 'test-layout', + template: `PlaceHolder Template HTML` +}) +class TestOrderComponent { + constructor() { + } +} From 81974a60340d4dda788ad4d15994ce04618d53c2 Mon Sep 17 00:00:00 2001 From: Adam Plumer Date: Sat, 15 Dec 2018 12:00:08 -0600 Subject: [PATCH 2/4] fixup! fix(fxFlexOrder): remove order after responsive deactivation --- src/lib/core/base/base2.ts | 24 +++++++- .../media-marshaller/media-marshaller.spec.ts | 4 +- .../core/media-marshaller/media-marshaller.ts | 61 ++++++++++++++++--- src/lib/extended/class/class.ts | 2 +- src/lib/extended/img-src/img-src.ts | 5 +- src/lib/extended/show-hide/show-hide.ts | 3 +- src/lib/extended/style/style.ts | 2 +- src/lib/flex/flex-align/flex-align.ts | 3 +- src/lib/flex/flex-offset/flex-offset.ts | 4 +- src/lib/flex/flex-order/flex-order.spec.ts | 2 +- src/lib/flex/flex-order/flex-order.ts | 6 +- src/lib/flex/flex/flex.ts | 5 +- src/lib/flex/layout-align/layout-align.ts | 3 +- src/lib/flex/layout-gap/layout-gap.ts | 5 +- src/lib/flex/layout/layout.ts | 3 +- src/lib/grid/align-columns/align-columns.ts | 3 +- src/lib/grid/align-rows/align-rows.ts | 3 +- src/lib/grid/area/area.ts | 3 +- src/lib/grid/areas/areas.ts | 3 +- src/lib/grid/auto/auto.ts | 3 +- src/lib/grid/column/column.ts | 3 +- src/lib/grid/columns/columns.ts | 3 +- src/lib/grid/gap/gap.ts | 3 +- src/lib/grid/grid-align/grid-align.ts | 3 +- src/lib/grid/row/row.ts | 3 +- src/lib/grid/rows/rows.ts | 3 +- 26 files changed, 106 insertions(+), 59 deletions(-) diff --git a/src/lib/core/base/base2.ts b/src/lib/core/base/base2.ts index c6419bb33..01dd01cd2 100644 --- a/src/lib/core/base/base2.ts +++ b/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'; @@ -18,6 +18,9 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy { protected DIRECTIVE_KEY = ''; protected inputs: string[] = []; protected destroySubject: Subject = new Subject(); + protected observables: Observable[] = []; + /** The least recently used styles for the builder */ + protected lru: StyleDefinition = {}; /** Access to host element's parent DOM node */ protected get parentElement(): HTMLElement | null { @@ -64,6 +67,11 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy { this.marshal.releaseElement(this.nativeElement); } + protected init(): void { + this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY, + this.updateWithValue.bind(this), this.clearStyles.bind(this), this.observables); + } + /** Add styles to the element using predefined style builder */ protected addStyles(input: string, parent?: Object) { const builder = this.styleBuilder; @@ -78,10 +86,20 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy { } } + this.lru = {...genStyles}; this.applyStyleToElement(genStyles); builder.sideEffect(input, genStyles, parent); } + /** Remove generated styles from an element using predefined style builder */ + protected clearStyles() { + Object.keys(this.lru).forEach(k => { + this.lru[k] = ''; + }); + this.applyStyleToElement(this.lru); + this.lru = {}; + } + protected triggerUpdate() { const val = this.marshal.getValue(this.nativeElement, this.DIRECTIVE_KEY); this.marshal.updateElement(this.nativeElement, this.DIRECTIVE_KEY, val); @@ -119,4 +137,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); + } } diff --git a/src/lib/core/media-marshaller/media-marshaller.spec.ts b/src/lib/core/media-marshaller/media-marshaller.spec.ts index 5fac1d60a..eb50e08b4 100644 --- a/src/lib/core/media-marshaller/media-marshaller.spec.ts +++ b/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(); diff --git a/src/lib/core/media-marshaller/media-marshaller.ts b/src/lib/core/media-marshaller/media-marshaller.ts index d03708210..3af9153e0 100644 --- a/src/lib/core/media-marshaller/media-marshaller.ts +++ b/src/lib/core/media-marshaller/media-marshaller.ts @@ -19,6 +19,7 @@ type Builder = Function; type ValueMap = Map; type BreakpointMap = Map; type ElementMap = Map; +type ElementKeyMap = WeakMap>; type SubscriptionMap = Map; type WatcherMap = WeakMap; type BuilderMap = WeakMap>; @@ -37,8 +38,10 @@ export interface ElementMatcher { export class MediaMarshaller { private activatedBreakpoints: BreakPoint[] = []; private elementMap: ElementMap = new Map(); + private elementKeyMap: ElementKeyMap = new WeakMap(); private watcherMap: WatcherMap = new WeakMap(); private builderMap: BuilderMap = new WeakMap(); + private clearBuilderMap: BuilderMap = new WeakMap(); private subject: Subject = new Subject(); get activatedBreakpoint(): string { @@ -72,20 +75,22 @@ export class MediaMarshaller { * @param element * @param key * @param builder optional so that custom bp directives don't have to re-provide this + * @param clearBuilder optional so that custom bp directives don't have to re-provide this * @param observables */ init(element: HTMLElement, key: string, builder?: Builder, + clearBuilder?: Builder, observables: Observable[] = []): void { - if (builder) { - let builders = this.builderMap.get(element); - if (!builders) { - builders = new Map(); - this.builderMap.set(element, builders); - } - builders.set(key, builder); + let keyMap = this.elementKeyMap.get(element); + if (!keyMap) { + keyMap = new Set(); + this.elementKeyMap.set(element, keyMap); } + keyMap.add(key); + initBuilderMap(this.builderMap, element, key, builder); + initBuilderMap(this.clearBuilderMap, element, key, clearBuilder); if (observables) { let watchers = this.watcherMap.get(element); if (!watchers) { @@ -166,12 +171,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 @@ -234,3 +268,14 @@ export class MediaMarshaller { this.matchMedia.registerQuery(queries); } } + +function initBuilderMap(map: BuilderMap, element: HTMLElement, key: string, input?: Builder): void { + if (input !== undefined) { + let oldMap = map.get(element); + if (!oldMap) { + oldMap = new Map(); + map.set(element, oldMap); + } + oldMap.set(key, input); + } +} diff --git a/src/lib/extended/class/class.ts b/src/lib/extended/class/class.ts index bcb740065..1531daa43 100644 --- a/src/lib/extended/class/class.ts +++ b/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) { diff --git a/src/lib/extended/img-src/img-src.ts b/src/lib/extended/img-src/img-src.ts index a6d19de03..7aaf5cfa5 100644 --- a/src/lib/extended/img-src/img-src.ts +++ b/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 `` 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); diff --git a/src/lib/extended/show-hide/show-hide.ts b/src/lib/extended/show-hide/show-hide.ts index 1a704bf6d..d912c02c6 100644 --- a/src/lib/extended/show-hide/show-hide.ts +++ b/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 === '') { diff --git a/src/lib/extended/style/style.ts b/src/lib/extended/style/style.ts index 98e758566..5c9180264 100644 --- a/src/lib/extended/style/style.ts +++ b/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') || '', ''); } diff --git a/src/lib/flex/flex-align/flex-align.ts b/src/lib/flex/flex-align/flex-align.ts index 0050d92e0..64b8d9c5a 100644 --- a/src/lib/flex/flex-align/flex-align.ts +++ b/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; diff --git a/src/lib/flex/flex-offset/flex-offset.ts b/src/lib/flex/flex-offset/flex-offset.ts index 3a79a4f62..9fbeac642 100644 --- a/src/lib/flex/flex-offset/flex-offset.ts +++ b/src/lib/flex/flex-offset/flex-offset.ts @@ -67,6 +67,7 @@ const selector = ` */ export class FlexOffsetDirective extends BaseDirective2 implements OnChanges { protected DIRECTIVE_KEY = 'flex-offset'; + protected observables = [this.directionality.change]; constructor(protected elRef: ElementRef, protected directionality: Directionality, @@ -76,8 +77,7 @@ 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(); if (this.parentElement) { this.marshal.trackValue(this.parentElement, 'layout-gap') .pipe(takeUntil(this.destroySubject)) diff --git a/src/lib/flex/flex-order/flex-order.spec.ts b/src/lib/flex/flex-order/flex-order.spec.ts index a619626b5..ac02692d6 100644 --- a/src/lib/flex/flex-order/flex-order.spec.ts +++ b/src/lib/flex/flex-order/flex-order.spec.ts @@ -60,7 +60,7 @@ describe('flex-order', () => { expectNativeEl(fixture).not.toHaveStyle({ 'order': '1', }, styler); - matchMedia.activate('lt-sm', true); + matchMedia.activate('xs'); expectNativeEl(fixture).toHaveStyle({ 'order': '1', }, styler); diff --git a/src/lib/flex/flex-order/flex-order.ts b/src/lib/flex/flex-order/flex-order.ts index 0a7c8a4cd..df7a84ba7 100644 --- a/src/lib/flex/flex-order/flex-order.ts +++ b/src/lib/flex/flex-order/flex-order.ts @@ -17,8 +17,7 @@ import { @Injectable({providedIn: 'root'}) export class FlexOrderStyleBuilder extends StyleBuilder { buildStyles(value: string) { - const val = parseInt((value || '0'), 10); - return {order: isNaN(val) ? 0 : val}; + return {order: (value && parseInt(value, 10)) || ''}; } } @@ -51,8 +50,7 @@ export class FlexOrderDirective extends BaseDirective2 implements OnChanges { @Optional() protected styleBuilder: FlexOrderStyleBuilder, 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 = flexOrderCache; diff --git a/src/lib/flex/flex/flex.ts b/src/lib/flex/flex/flex.ts index 6577599ad..38d9ef3be 100644 --- a/src/lib/flex/flex/flex.ts +++ b/src/lib/flex/flex/flex.ts @@ -229,8 +229,7 @@ export class FlexDirective extends BaseDirective2 { protected styleBuilder: FlexStyleBuilder, protected marshal: MediaMarshaller) { super(elRef, styleBuilder, styleUtils, marshal); - this.marshal.init(this.elRef.nativeElement, this.DIRECTIVE_KEY, - this.updateStyle.bind(this)); + this.init(); if (this.parentElement) { this.marshal.trackValue(this.parentElement, 'layout') .pipe(takeUntil(this.destroySubject)) @@ -251,7 +250,7 @@ export class FlexDirective extends BaseDirective2 { } /** Input to this is exclusively the basis input value */ - protected updateStyle(value: string) { + protected updateWithValue(value: string) { const addFlexToParent = this.layoutConfig.addFlexToParent !== false; if (!this.direction) { this.direction = this.getFlexFlowDirection(this.parentElement!, addFlexToParent); diff --git a/src/lib/flex/layout-align/layout-align.ts b/src/lib/flex/layout-align/layout-align.ts index 850d9e0ca..62ea5bcb8 100644 --- a/src/lib/flex/layout-align/layout-align.ts +++ b/src/lib/flex/layout-align/layout-align.ts @@ -129,8 +129,7 @@ export class LayoutAlignDirective extends BaseDirective2 { @Optional() protected styleBuilder: LayoutAlignStyleBuilder, protected marshal: MediaMarshaller) { super(elRef, styleBuilder, styleUtils, marshal); - this.marshal.init(this.elRef.nativeElement, this.DIRECTIVE_KEY, - this.updateWithValue.bind(this)); + this.init(); this.marshal.trackValue(this.nativeElement, 'layout') .pipe(takeUntil(this.destroySubject)) .subscribe(this.onLayoutChange.bind(this)); diff --git a/src/lib/flex/layout-gap/layout-gap.ts b/src/lib/flex/layout-gap/layout-gap.ts index c7dc13ac5..c2e6b0961 100644 --- a/src/lib/flex/layout-gap/layout-gap.ts +++ b/src/lib/flex/layout-gap/layout-gap.ts @@ -100,6 +100,7 @@ export class LayoutGapDirective extends BaseDirective2 implements AfterContentIn protected layout = 'row'; // default flex-direction protected DIRECTIVE_KEY = 'layout-gap'; protected observerSubject = new Subject(); + protected observables = [this.directionality.change, this.observerSubject.asObservable()]; /** Special accessor to query for all child 'element' nodes regardless of type, class, etc */ protected get childrenNodes(): HTMLElement[] { @@ -122,9 +123,7 @@ export class LayoutGapDirective extends BaseDirective2 implements AfterContentIn @Optional() protected styleBuilder: LayoutGapStyleBuilder, protected marshal: MediaMarshaller) { super(elRef, styleBuilder, styleUtils, marshal); - this.marshal.init(this.elRef.nativeElement, this.DIRECTIVE_KEY, - this.updateWithValue.bind(this), [this.directionality.change, - this.observerSubject.asObservable()]); + this.init(); this.marshal.trackValue(this.nativeElement, 'layout') .pipe(takeUntil(this.destroySubject)) .subscribe(this.onLayoutChange.bind(this)); diff --git a/src/lib/flex/layout/layout.ts b/src/lib/flex/layout/layout.ts index 4cf6103d0..c52d4ef08 100644 --- a/src/lib/flex/layout/layout.ts +++ b/src/lib/flex/layout/layout.ts @@ -54,8 +54,7 @@ export class LayoutDirective extends BaseDirective2 implements OnChanges { @Optional() protected styleBuilder: LayoutStyleBuilder, 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 = layoutCache; diff --git a/src/lib/grid/align-columns/align-columns.ts b/src/lib/grid/align-columns/align-columns.ts index f7d9d317c..3a42e9b3d 100644 --- a/src/lib/grid/align-columns/align-columns.ts +++ b/src/lib/grid/align-columns/align-columns.ts @@ -45,8 +45,7 @@ export class GridAlignColumnsDirective extends BaseDirective2 { protected styler: StyleUtils, protected marshal: MediaMarshaller) { super(elementRef, styleBuilder, styler, marshal); - this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY, - this.updateWithValue.bind(this)); + this.init(); } // ********************************************* diff --git a/src/lib/grid/align-rows/align-rows.ts b/src/lib/grid/align-rows/align-rows.ts index 8bf437974..1c639aa3f 100644 --- a/src/lib/grid/align-rows/align-rows.ts +++ b/src/lib/grid/align-rows/align-rows.ts @@ -45,8 +45,7 @@ export class GridAlignRowsDirective extends BaseDirective2 { protected styler: StyleUtils, protected marshal: MediaMarshaller) { super(elementRef, styleBuilder, styler, marshal); - this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY, - this.updateWithValue.bind(this)); + this.init(); } // ********************************************* diff --git a/src/lib/grid/area/area.ts b/src/lib/grid/area/area.ts index 3d42edd0f..7bef9d05b 100644 --- a/src/lib/grid/area/area.ts +++ b/src/lib/grid/area/area.ts @@ -34,8 +34,7 @@ export class GridAreaDirective extends BaseDirective2 { @Optional() protected styleBuilder: GridAreaStyleBuilder, 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 = gridAreaCache; diff --git a/src/lib/grid/areas/areas.ts b/src/lib/grid/areas/areas.ts index 515316119..a8dcc6822 100644 --- a/src/lib/grid/areas/areas.ts +++ b/src/lib/grid/areas/areas.ts @@ -50,8 +50,7 @@ export class GridAreasDirective extends BaseDirective2 { @Optional() protected styleBuilder: GridAreasStyleBuiler, protected marshal: MediaMarshaller) { super(elRef, styleBuilder, styleUtils, marshal); - this.marshal.init(this.elRef.nativeElement, this.DIRECTIVE_KEY, - this.updateWithValue.bind(this)); + this.init(); } // ********************************************* diff --git a/src/lib/grid/auto/auto.ts b/src/lib/grid/auto/auto.ts index 76542093d..b975e199c 100644 --- a/src/lib/grid/auto/auto.ts +++ b/src/lib/grid/auto/auto.ts @@ -53,8 +53,7 @@ export class GridAutoDirective extends BaseDirective2 { protected styler: StyleUtils, protected marshal: MediaMarshaller) { super(elementRef, styleBuilder, styler, marshal); - this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY, - this.updateWithValue.bind(this)); + this.init(); } // ********************************************* diff --git a/src/lib/grid/column/column.ts b/src/lib/grid/column/column.ts index 49bb18416..871a788c9 100644 --- a/src/lib/grid/column/column.ts +++ b/src/lib/grid/column/column.ts @@ -33,8 +33,7 @@ export class GridColumnDirective extends BaseDirective2 { protected styler: StyleUtils, protected marshal: MediaMarshaller) { super(elementRef, styleBuilder, styler, marshal); - this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY, - this.addStyles.bind(this)); + this.init(); } protected styleCache = columnCache; diff --git a/src/lib/grid/columns/columns.ts b/src/lib/grid/columns/columns.ts index 6a6935006..99aeb65d9 100644 --- a/src/lib/grid/columns/columns.ts +++ b/src/lib/grid/columns/columns.ts @@ -59,8 +59,7 @@ export class GridColumnsDirective extends BaseDirective2 { protected styler: StyleUtils, protected marshal: MediaMarshaller) { super(elementRef, styleBuilder, styler, marshal); - this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY, - this.updateWithValue.bind(this)); + this.init(); } // ********************************************* diff --git a/src/lib/grid/gap/gap.ts b/src/lib/grid/gap/gap.ts index 2ff7fbd52..672e8c643 100644 --- a/src/lib/grid/gap/gap.ts +++ b/src/lib/grid/gap/gap.ts @@ -46,8 +46,7 @@ export class GridGapDirective extends BaseDirective2 { @Optional() protected styleBuilder: GridGapStyleBuilder, protected marshal: MediaMarshaller) { super(elRef, styleBuilder, styleUtils, marshal); - this.marshal.init(this.elRef.nativeElement, this.DIRECTIVE_KEY, - this.updateWithValue.bind(this)); + this.init(); } // ********************************************* diff --git a/src/lib/grid/grid-align/grid-align.ts b/src/lib/grid/grid-align/grid-align.ts index ec608aaa0..3463329d1 100644 --- a/src/lib/grid/grid-align/grid-align.ts +++ b/src/lib/grid/grid-align/grid-align.ts @@ -35,8 +35,7 @@ export class GridAlignDirective extends BaseDirective2 { protected styler: StyleUtils, protected marshal: MediaMarshaller) { super(elementRef, styleBuilder, styler, marshal); - this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY, - this.addStyles.bind(this)); + this.init(); } protected styleCache = alignCache; diff --git a/src/lib/grid/row/row.ts b/src/lib/grid/row/row.ts index 3a6e651ce..442a0db04 100644 --- a/src/lib/grid/row/row.ts +++ b/src/lib/grid/row/row.ts @@ -33,8 +33,7 @@ export class GridRowDirective extends BaseDirective2 { protected styler: StyleUtils, protected marshal: MediaMarshaller) { super(elementRef, styleBuilder, styler, marshal); - this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY, - this.addStyles.bind(this)); + this.init(); } protected styleCache = rowCache; diff --git a/src/lib/grid/rows/rows.ts b/src/lib/grid/rows/rows.ts index 89ec55a1b..3437a9eaa 100644 --- a/src/lib/grid/rows/rows.ts +++ b/src/lib/grid/rows/rows.ts @@ -59,8 +59,7 @@ export class GridRowsDirective extends BaseDirective2 { protected styler: StyleUtils, protected marshal: MediaMarshaller) { super(elementRef, styleBuilder, styler, marshal); - this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY, - this.updateWithValue.bind(this)); + this.init(); } // ********************************************* From c95f890086471fea395e596cee7726de3a4ca0b8 Mon Sep 17 00:00:00 2001 From: Adam Plumer Date: Sat, 15 Dec 2018 12:06:20 -0600 Subject: [PATCH 3/4] fixup! fix(fxFlexOrder): remove order after responsive deactivation --- src/lib/core/base/base2.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib/core/base/base2.ts b/src/lib/core/base/base2.ts index 01dd01cd2..1c504d836 100644 --- a/src/lib/core/base/base2.ts +++ b/src/lib/core/base/base2.ts @@ -19,8 +19,8 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy { protected inputs: string[] = []; protected destroySubject: Subject = new Subject(); protected observables: Observable[] = []; - /** The least recently used styles for the builder */ - protected lru: StyleDefinition = {}; + /** The most recently used styles for the builder */ + protected mru: StyleDefinition = {}; /** Access to host element's parent DOM node */ protected get parentElement(): HTMLElement | null { @@ -86,18 +86,18 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy { } } - this.lru = {...genStyles}; + 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.lru).forEach(k => { - this.lru[k] = ''; + Object.keys(this.mru).forEach(k => { + this.mru[k] = ''; }); - this.applyStyleToElement(this.lru); - this.lru = {}; + this.applyStyleToElement(this.mru); + this.mru = {}; } protected triggerUpdate() { From 5197c2a8e05068dc9d3002fa4359aefe1b85460c Mon Sep 17 00:00:00 2001 From: Adam Plumer Date: Sun, 16 Dec 2018 10:32:13 -0600 Subject: [PATCH 4/4] fixup! fix(fxFlexOrder): remove order after responsive deactivation --- src/lib/core/base/base2.ts | 16 ++-- .../core/media-marshaller/media-marshaller.ts | 88 ++++++++++++------- src/lib/flex/flex-offset/flex-offset.ts | 7 +- src/lib/flex/layout-gap/layout-gap.ts | 9 +- 4 files changed, 77 insertions(+), 43 deletions(-) diff --git a/src/lib/core/base/base2.ts b/src/lib/core/base/base2.ts index 1c504d836..2d67901d7 100644 --- a/src/lib/core/base/base2.ts +++ b/src/lib/core/base/base2.ts @@ -17,10 +17,9 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy { protected DIRECTIVE_KEY = ''; protected inputs: string[] = []; - protected destroySubject: Subject = new Subject(); - protected observables: Observable[] = []; /** The most recently used styles for the builder */ protected mru: StyleDefinition = {}; + protected destroySubject: Subject = new Subject(); /** Access to host element's parent DOM node */ protected get parentElement(): HTMLElement | null { @@ -67,9 +66,15 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy { this.marshal.releaseElement(this.nativeElement); } - protected init(): void { - this.marshal.init(this.elementRef.nativeElement, this.DIRECTIVE_KEY, - this.updateWithValue.bind(this), this.clearStyles.bind(this), this.observables); + /** Register with central marshaller service */ + protected init(extraTriggers: Observable[] = []): 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 */ @@ -100,6 +105,7 @@ export abstract class BaseDirective2 implements OnChanges, OnDestroy { 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); diff --git a/src/lib/core/media-marshaller/media-marshaller.ts b/src/lib/core/media-marshaller/media-marshaller.ts index 3af9153e0..13950cae0 100644 --- a/src/lib/core/media-marshaller/media-marshaller.ts +++ b/src/lib/core/media-marshaller/media-marshaller.ts @@ -16,6 +16,8 @@ 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; type BreakpointMap = Map; type ElementMap = Map; @@ -39,6 +41,7 @@ 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(); @@ -50,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(); } @@ -74,38 +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 clearBuilder 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, - clearBuilder?: Builder, - observables: Observable[] = []): void { - let keyMap = this.elementKeyMap.get(element); - if (!keyMap) { - keyMap = new Set(); - this.elementKeyMap.set(element, keyMap); - } - keyMap.add(key); - initBuilderMap(this.builderMap, element, key, builder); - initBuilderMap(this.clearBuilderMap, element, key, clearBuilder); - 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[] = []): void { + this.buildElementKeyMap(element, key); + initBuilderMap(this.builderMap, element, key, updateFn); + initBuilderMap(this.clearBuilderMap, element, key, clearFn); + this.watchExtraTriggers(element, key, extraTriggers); } /** @@ -162,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 { return this.subject.asObservable() .pipe(filter(v => v.element === element && v.key === key)); @@ -240,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[]) { + 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); @@ -269,7 +292,10 @@ export class MediaMarshaller { } } -function initBuilderMap(map: BuilderMap, element: HTMLElement, key: string, input?: Builder): void { +function initBuilderMap(map: BuilderMap, + element: HTMLElement, + key: string, + input?: UpdateCallback | ClearCallback): void { if (input !== undefined) { let oldMap = map.get(element); if (!oldMap) { diff --git a/src/lib/flex/flex-offset/flex-offset.ts b/src/lib/flex/flex-offset/flex-offset.ts index 9fbeac642..8a83ee788 100644 --- a/src/lib/flex/flex-offset/flex-offset.ts +++ b/src/lib/flex/flex-offset/flex-offset.ts @@ -67,7 +67,6 @@ const selector = ` */ export class FlexOffsetDirective extends BaseDirective2 implements OnChanges { protected DIRECTIVE_KEY = 'flex-offset'; - protected observables = [this.directionality.change]; constructor(protected elRef: ElementRef, protected directionality: Directionality, @@ -77,9 +76,11 @@ export class FlexOffsetDirective extends BaseDirective2 implements OnChanges { protected marshal: MediaMarshaller, protected styler: StyleUtils) { super(elRef, styleBuilder, styler, marshal); - this.init(); + 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)); } diff --git a/src/lib/flex/layout-gap/layout-gap.ts b/src/lib/flex/layout-gap/layout-gap.ts index c2e6b0961..6b60a30d8 100644 --- a/src/lib/flex/layout-gap/layout-gap.ts +++ b/src/lib/flex/layout-gap/layout-gap.ts @@ -100,7 +100,6 @@ export class LayoutGapDirective extends BaseDirective2 implements AfterContentIn protected layout = 'row'; // default flex-direction protected DIRECTIVE_KEY = 'layout-gap'; protected observerSubject = new Subject(); - protected observables = [this.directionality.change, this.observerSubject.asObservable()]; /** Special accessor to query for all child 'element' nodes regardless of type, class, etc */ protected get childrenNodes(): HTMLElement[] { @@ -108,7 +107,7 @@ export class LayoutGapDirective extends BaseDirective2 implements AfterContentIn const buffer: any[] = []; // iterate backwards ensuring that length is an UInt32 - for (let i = obj.length; i--; ) { + for (let i = obj.length; i--;) { buffer[i] = obj[i]; } return buffer; @@ -123,8 +122,10 @@ export class LayoutGapDirective extends BaseDirective2 implements AfterContentIn @Optional() protected styleBuilder: LayoutGapStyleBuilder, protected marshal: MediaMarshaller) { super(elRef, styleBuilder, styleUtils, marshal); - this.init(); - this.marshal.trackValue(this.nativeElement, 'layout') + const extraTriggers = [this.directionality.change, this.observerSubject.asObservable()]; + this.init(extraTriggers); + this.marshal + .trackValue(this.nativeElement, 'layout') .pipe(takeUntil(this.destroySubject)) .subscribe(this.onLayoutChange.bind(this)); }