From 145d658d501395e3bd696c8e287f951ab13102fd Mon Sep 17 00:00:00 2001 From: Thomas Burleson Date: Tue, 31 Jan 2017 18:49:23 +1300 Subject: [PATCH] fix(fxLayoutGap): fxLayoutWrap to apply gap logic for reverse directions * `margin-right` be used with `fxLayout="row" fxLayoutWrap fxLayoutGap` combinations * `margin-bottom` be used with `fxLayout="column" fxLayoutWrap fxLayoutGap` combinations fixes #108 --- src/lib/flexbox/api/layout-gap.spec.ts | 115 ++++++++++++++++++------- src/lib/flexbox/api/layout-gap.ts | 46 +++++----- 2 files changed, 107 insertions(+), 54 deletions(-) diff --git a/src/lib/flexbox/api/layout-gap.spec.ts b/src/lib/flexbox/api/layout-gap.spec.ts index ddbff551d..f6161a6a8 100644 --- a/src/lib/flexbox/api/layout-gap.spec.ts +++ b/src/lib/flexbox/api/layout-gap.spec.ts @@ -7,7 +7,7 @@ */ import {Component, OnInit} from '@angular/core'; import {CommonModule} from '@angular/common'; -import {TestBed, ComponentFixture} from '@angular/core/testing'; +import {TestBed, ComponentFixture, async} from '@angular/core/testing'; import {BreakPointsProvider} from '../../media-query/breakpoints/break-points'; import {BreakPointRegistry} from '../../media-query/breakpoints/break-point-registry'; @@ -54,7 +54,10 @@ describe('layout-gap directive', () => { fixture1.detectChanges(); const nodes = queryFor(fixture1, 'span'); - expect(nodes[1].nativeElement).toHaveCssStyle({[marginKey]: margin}); + const styles = {[marginKey]: margin}; + + expect(nodes[0].nativeElement).toHaveCssStyle(styles); + expect(nodes[1].nativeElement).not.toHaveCssStyle(styles); } describe('with static features', () => { @@ -65,9 +68,7 @@ describe('layout-gap directive', () => {
`; - expectDomForQuery(template, "[fxFlex]").not.toHaveCssStyle({ - 'margin-left': '13px;', - }); + expectDomForQuery(template, "[fxFlex]").not.toHaveCssStyle({'margin-right': '13px;'}); }); it('should add gap styles to all children except the 1st child', () => { @@ -83,9 +84,10 @@ describe('layout-gap directive', () => { let nodes = queryFor(fixture, "[fxFlex]"); expect(nodes.length).toEqual(3); - expect(nodes[0].nativeElement).not.toHaveCssStyle({'margin-left': '13px'}); - expect(nodes[1].nativeElement).toHaveCssStyle({'margin-left': '13px'}); - expect(nodes[2].nativeElement).toHaveCssStyle({'margin-left': '13px'}); + expect(nodes[0].nativeElement).toHaveCssStyle({'margin-right': '13px'}); + expect(nodes[1].nativeElement).toHaveCssStyle({'margin-right': '13px'}); + expect(nodes[2].nativeElement).not.toHaveCssStyle({'margin-right': '13px'}); + expect(nodes[2].nativeElement).not.toHaveCssStyle({'margin-right': '0px'}); }); it('should add gap styles to dynamics rows EXCEPT first', () => { @@ -100,15 +102,15 @@ describe('layout-gap directive', () => { let nodes = queryFor(fixture, "[fxFlex]"); expect(nodes.length).toEqual(4); - expect(nodes[0].nativeElement).not.toHaveCssStyle({'margin-left': '13px'}); - expect(nodes[1].nativeElement).toHaveCssStyle({'margin-left': '13px'}); - expect(nodes[2].nativeElement).toHaveCssStyle({'margin-left': '13px'}); - expect(nodes[3].nativeElement).toHaveCssStyle({'margin-left': '13px'}); + expect(nodes[0].nativeElement).toHaveCssStyle({'margin-right': '13px'}); + expect(nodes[1].nativeElement).toHaveCssStyle({'margin-right': '13px'}); + expect(nodes[2].nativeElement).toHaveCssStyle({'margin-right': '13px'}); + expect(nodes[3].nativeElement).not.toHaveCssStyle({'margin-right': '13px'}); + expect(nodes[3].nativeElement).not.toHaveCssStyle({'margin-right': '0px'}); }); - it('should add update gap styles when row items are removed', () => { - let nodes, - template = ` + it('should add update gap styles when row items are removed', async(() => { + let template = `
@@ -117,22 +119,29 @@ describe('layout-gap directive', () => { fixture.componentInstance.direction = "row"; fixture.detectChanges(); - nodes = queryFor(fixture, "[fxFlex]"); + let nodes = queryFor(fixture, "[fxFlex]"); expect(nodes.length).toEqual(4); - fixture.componentInstance.rows.shift(); + fixture.componentInstance.rows = new Array(3); fixture.detectChanges(); - nodes = queryFor(fixture, "[fxFlex]"); - expect(nodes.length).toEqual(3); + setTimeout(() => { + // Since the layoutGap directive detects the *ngFor changes by using a MutationObserver, the + // browser will take up some time, to actually announce the changes to the directive. + // (Kudos to @DevVersion) - expect(nodes[0].nativeElement).toHaveCssStyle({'margin-left': '0px'}); - expect(nodes[1].nativeElement).toHaveCssStyle({'margin-left': '13px'}); - expect(nodes[2].nativeElement).toHaveCssStyle({'margin-left': '13px'}); - }); + nodes = queryFor(fixture, "[fxFlex]"); + expect(nodes.length).toEqual(3); + + expect(nodes[0].nativeElement).toHaveCssStyle({'margin-right': '13px'}); + expect(nodes[1].nativeElement).toHaveCssStyle({'margin-right': '13px'}); + expect(nodes[2].nativeElement).not.toHaveCssStyle({'margin-right': '13px'}); + }); + + })); it('should apply margin-top for column layouts', () => { - verifyCorrectMargin('column', 'margin-top'); + verifyCorrectMargin('column', 'margin-bottom'); }); it('should apply margin-right for row-reverse layouts', () => { @@ -158,8 +167,8 @@ describe('layout-gap directive', () => { fixture.detectChanges(); let nodes = queryFor(fixture, 'span'); - expect(nodes[1].nativeElement).not.toHaveCssStyle({'margin-left': '8px'}); - expect(nodes[1].nativeElement).toHaveCssStyle({'margin-top': '8px'}); + expect(nodes[0].nativeElement).not.toHaveCssStyle({'margin-right': '8px'}); + expect(nodes[0].nativeElement).toHaveCssStyle({'margin-bottom': '8px'}); // layout = column-reverse, use margin-bottom @@ -167,8 +176,8 @@ describe('layout-gap directive', () => { fixture.detectChanges(); nodes = queryFor(fixture, 'span'); - expect(nodes[1].nativeElement).not.toHaveCssStyle({'margin-top': '8px'}); - expect(nodes[1].nativeElement).toHaveCssStyle({'margin-bottom': '8px'}); + expect(nodes[0].nativeElement).not.toHaveCssStyle({'margin-right': '8px'}); + expect(nodes[0].nativeElement).toHaveCssStyle({'margin-bottom': '8px'}); // layout = row-reverse, use margin-right @@ -176,8 +185,8 @@ describe('layout-gap directive', () => { fixture.detectChanges(); nodes = queryFor(fixture, 'span'); - expect(nodes[1].nativeElement).not.toHaveCssStyle({'margin-bottom': '8px'}); - expect(nodes[1].nativeElement).toHaveCssStyle({'margin-right': '8px'}); + expect(nodes[0].nativeElement).not.toHaveCssStyle({'margin-bottom': '8px'}); + expect(nodes[0].nativeElement).toHaveCssStyle({'margin-right': '8px'}); }); it('should recognize hidden elements when applying gaps', () => { @@ -196,10 +205,50 @@ describe('layout-gap directive', () => { let nodes = queryFor(fixture, '[fxFlex]'); expect(nodes.length).toEqual(3); - expect(nodes[0].nativeElement).not.toHaveCssStyle({'margin-left': '0px'}); - expect(nodes[1].nativeElement).toHaveCssStyle({'margin-left': '0px'}); - expect(nodes[2].nativeElement).toHaveCssStyle({'margin-left': '16px'}); + expect(nodes[0].nativeElement).not.toHaveCssStyle({'margin-right': '0px'}); + expect(nodes[0].nativeElement).not.toHaveCssStyle({'margin-right': '16px'}); + expect(nodes[1].nativeElement).toHaveCssStyle({'margin-right': '16px'}); + expect(nodes[2].nativeElement).not.toHaveCssStyle({'margin-right': '16px'}); + + }); + it('should adjust gaps based on layout-wrap presence', () => { + let styles = ['.col1 { display:none !important;']; + let template = ` +
+
Div 1
+
Div 2
+
Div 2
+
Div 3
+
+ `; + fixture = createTestComponent(template, styles); + fixture.componentInstance.gap = '16px'; + fixture.componentInstance.direction = 'row'; + fixture.detectChanges(); + + let nodes = queryFor(fixture, '[fxFlex]'); + + expect(nodes.length).toEqual(4); + expect(nodes[0].nativeElement).not.toHaveCssStyle({'margin-right': '16px'}); + expect(nodes[1].nativeElement).toHaveCssStyle({'margin-right': '16px'}); + expect(nodes[2].nativeElement).toHaveCssStyle({'margin-right': '16px'}); + expect(nodes[3].nativeElement).not.toHaveCssStyle({'margin-right': '16px'}); + + fixture.componentInstance.gap = '8px'; + fixture.componentInstance.direction = 'column'; + fixture.detectChanges(); + + nodes = queryFor(fixture, '[fxFlex]'); + + expect(nodes.length).toEqual(4); + expect(nodes[0].nativeElement).not.toHaveCssStyle({'margin-bottom': '8px'}); + expect(nodes[1].nativeElement).toHaveCssStyle({'margin-bottom': '8px'}); + expect(nodes[2].nativeElement).toHaveCssStyle({'margin-bottom': '8px'}); + expect(nodes[3].nativeElement).not.toHaveCssStyle({'margin-bottom': '8px'}); }); }); diff --git a/src/lib/flexbox/api/layout-gap.ts b/src/lib/flexbox/api/layout-gap.ts index db8cf0440..f77a733d5 100644 --- a/src/lib/flexbox/api/layout-gap.ts +++ b/src/lib/flexbox/api/layout-gap.ts @@ -141,10 +141,15 @@ export class LayoutGapDirective extends BaseFxDirective implements AfterContentI */ private _watchContentChanges() { let onMutationCallback = (mutations) => { - // update gap styles only for 'addedNodes' events - mutations - .filter((it: MutationRecord) => it.addedNodes && it.addedNodes.length) - .map(() => this._updateWithValue()); + let validatedChanges = (it: MutationRecord) => { + return (it.addedNodes && it.addedNodes.length) || + (it.removedNodes && it.removedNodes.length); + }; + + // update gap styles only for child 'added' or 'removed' events + if (mutations.filter(validatedChanges).length) { + this._updateWithValue(); + } }; this._observer = new MutationObserver(onMutationCallback); @@ -173,23 +178,28 @@ export class LayoutGapDirective extends BaseFxDirective implements AfterContentI // Gather all non-hidden Element nodes let items = this.childrenNodes - .filter(el => (el.nodeType === 1)) // only Element types - .filter(el => this._getDisplayStyle(el) != "none"); + .filter(el => (el.nodeType === 1)) // only Element types + .filter(el => this._getDisplayStyle(el) != "none"); + let numItems = items.length; - // Reset 1st child element to 0px gap - let skipped = items.filter((el, j) => j == 0); - this._applyStyleToElements(this._buildCSS(0), skipped); + if (numItems > 1) { + let lastItem = items[numItems - 1]; - // For each `element` child, set the padding styles... - items = items.filter((el, j) => j > 0); // skip first element since gaps are needed - this._applyStyleToElements(this._buildCSS(value), items); + // For each `element` children EXCEPT the last, + // set the margin right/bottom styles... + items = items.filter((el, j) => j < numItems - 1); + this._applyStyleToElements(this._buildCSS(value), items); + + // Clear all gaps for all visible elements + this._applyStyleToElements(this._buildCSS(), [lastItem]); + } } /** * Prepare margin CSS, remove any previous explicitly * assigned margin assignments */ - private _buildCSS(value) { + private _buildCSS(value: any = null) { let key, margins = { 'margin-left': null, 'margin-right': null, @@ -199,19 +209,13 @@ export class LayoutGapDirective extends BaseFxDirective implements AfterContentI switch (this._layout) { case 'column': - key = 'margin-top'; - break; case 'column-reverse': key = 'margin-bottom'; break; - case 'row-reverse': - key = 'margin-right'; - break; case "row" : - key = 'margin-left'; - break; + case 'row-reverse': default : - key = 'margin-left'; + key = 'margin-right'; break; } margins[key] = value;