Skip to content
This repository has been archived by the owner on Jan 6, 2025. It is now read-only.

Commit

Permalink
fix(fxLayoutGap): skip hidden element nodes
Browse files Browse the repository at this point in the history
fxLayoutGap should consider (and skip) hidden elements when applying layout-gap logic.

fixes #136.
  • Loading branch information
ThomasBurleson committed Jan 30, 2017
1 parent 6e46561 commit 3c098fc
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 30 deletions.
24 changes: 18 additions & 6 deletions src/lib/flexbox/api/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
* found in the LICENSE file at https://angular.io/license
*/
import {ElementRef, Renderer, OnDestroy} from '@angular/core';

import {__platform_browser_private__} from '@angular/platform-browser';
const getDOM = __platform_browser_private__.getDOM;

import {applyCssPrefixes} from '../../utils/auto-prefixer';

import {ResponsiveActivation, KeyOptions} from '../responsive/responsive-activation';
Expand All @@ -16,7 +20,7 @@ import {MediaQuerySubscriber} from '../../media-query/media-change';
* Definition of a css style. Either a property name (e.g. "flex-basis") or an object
* map of property name and value (e.g. {display: 'none', flex-order: 5}).
*/
export type StyleDefinition = string|{[property: string]: string|number};
export type StyleDefinition = string|{ [property: string]: string|number };

/** Abstract base class for the Layout API styling directives. */
export abstract class BaseFxDirective implements OnDestroy {
Expand Down Expand Up @@ -86,9 +90,10 @@ export abstract class BaseFxDirective implements OnDestroy {
* Note: this allows use to preserve the original style
* and optional restore it when the mediaQueries deactivate
*/
protected _getDisplayStyle(): string {
let element: HTMLElement = this._elementRef.nativeElement;
return (element.style as any)['display'] || "flex";
protected _getDisplayStyle(source?: HTMLElement): string {
let element: HTMLElement = source || this._elementRef.nativeElement;
let value = (element.style as any)['display'] || getDOM().getComputedStyle(element)['display'];
return value.trim();
}

/**
Expand All @@ -109,7 +114,7 @@ export abstract class BaseFxDirective implements OnDestroy {

// Iterate all properties in hashMap and set styles
for (let key in styles) {
this._renderer.setElementStyle(element, key, styles[key]);
this._renderer.setElementStyle(element, key, styles[key] as string);
}
}

Expand All @@ -122,7 +127,7 @@ export abstract class BaseFxDirective implements OnDestroy {
elements.forEach(el => {
// Iterate all properties in hashMap and set styles
for (let key in styles) {
this._renderer.setElementStyle(el, key, styles[key]);
this._renderer.setElementStyle(el, key, styles[key] as string);
}
});

Expand Down Expand Up @@ -172,4 +177,11 @@ export abstract class BaseFxDirective implements OnDestroy {
return buffer;
}

/**
* Fast validator for presence of attribute on the host element
*/
protected hasKeyValue(key) {
return this._mqActivation.hasKeyValue(key);
}

}
50 changes: 36 additions & 14 deletions src/lib/flexbox/api/layout-gap.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,22 @@ describe('layout-gap directive', () => {
});
});

function verifyCorrectMargin(layout: string, marginKey: string) {
const margin = '8px';
const template = `
<div fxLayout='${layout}' fxLayoutGap='${margin}'>
<span></span>
<span></span>
</div>
`;

const fixture1 = createTestComponent(template);
fixture1.detectChanges();

const nodes = queryFor(fixture1, 'span');
expect(nodes[1].nativeElement).toHaveCssStyle({[marginKey]: margin});
}

describe('with static features', () => {

it('should not add gap styles for a single child', () => {
Expand Down Expand Up @@ -163,23 +179,29 @@ describe('layout-gap directive', () => {
expect(nodes[1].nativeElement).not.toHaveCssStyle({'margin-bottom': '8px'});
expect(nodes[1].nativeElement).toHaveCssStyle({'margin-right': '8px'});
});
});

function verifyCorrectMargin(layout: string, marginKey: string) {
const margin = '8px';
const template = `
<div fxLayout='${layout}' fxLayoutGap='${margin}'>
<span></span>
<span></span>
</div>
`;
it('should recognize hidden elements when applying gaps', () => {
let styles = ['.col1 { display:none !important;'];
let template = `
<div class="container" fxLayout="row" fxLayoutGap="16px">
<div fxFlex class="col1">Div 1</div>
<div fxFlex class="col2">Div 2</div>
<div fxFlex class="col3">Div 3</div>
</div>
`;
fixture = createTestComponent(template, styles);
fixture.componentInstance.direction = 'row';
fixture.detectChanges();

const fixture1 = createTestComponent(template);
fixture1.detectChanges();
let nodes = queryFor(fixture, '[fxFlex]');

const nodes = queryFor(fixture1, 'span');
expect(nodes[1].nativeElement).toHaveCssStyle({[marginKey]: margin});
}
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'});

});
});

describe('with responsive features', () => {

Expand Down
16 changes: 8 additions & 8 deletions src/lib/flexbox/api/layout-gap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,18 +171,18 @@ export class LayoutGapDirective extends BaseFxDirective implements AfterContentI
value = this._mqActivation.activatedInput;
}

// Reset 1st child element to 0px gap
// Gather all non-hidden Element nodes
let items = this.childrenNodes
.filter(el => (el.nodeType === 1)) // only Element types
.filter((el, j) => j == 0);
this._applyStyleToElements(this._buildCSS(0), items);
.filter(el => (el.nodeType === 1)) // only Element types
.filter(el => this._getDisplayStyle(el) != "none");

// Reset 1st child element to 0px gap
let skipped = items.filter((el, j) => j == 0);
this._applyStyleToElements(this._buildCSS(0), skipped);

// For each `element` child, set the padding styles...
items = this.childrenNodes
.filter(el => (el.nodeType === 1)) // only Element types
.filter((el, j) => j > 0); // skip first element since gaps are needed
items = items.filter((el, j) => j > 0); // skip first element since gaps are needed
this._applyStyleToElements(this._buildCSS(value), items);

}

/**
Expand Down
8 changes: 8 additions & 0 deletions src/lib/flexbox/responsive/responsive-activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ export class ResponsiveActivation {
return this._hasKeyValue(key) ? this._lookupKeyValue(key) : this._options.defaultValue;
}

/**
* Fast validator for presence of attribute on the host element
*/
public hasKeyValue(key) {
let value = this._options.inputKeys[key];
return typeof value !== 'undefined';
}

/**
* Remove interceptors, restore original functions, and forward the onDestroy() call
*/
Expand Down
9 changes: 7 additions & 2 deletions src/lib/utils/testing/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,18 @@ export function makeCreateTestComponent(getClass: ComponentClazzFn) {
let componentAny: Type<any>;

// Return actual `createTestComponent()` function
return function createTestComponent(template: string): ComponentFixture<Type<any>> {
return function createTestComponent(template: string, styles?: any): ComponentFixture<Type<any>> {
if (!componentAny) {
// Defer access to Component class to enable metadata to be configured properly...
componentAny = getClass();
}
return TestBed
.overrideComponent(componentAny, {set: {template: template}})
.overrideComponent(componentAny, {
set: {
template: template,
styles: styles || [],
}
})
.createComponent(componentAny);
};
}
Expand Down

0 comments on commit 3c098fc

Please sign in to comment.