Skip to content

Commit

Permalink
fix(flexbox): scan flex-direction in css stylesheet (#365)
Browse files Browse the repository at this point in the history
`fxFlex` works best if the parent container has flexbox styles assigned (display, flex-direction, etc.). If not assigned (inline or via stylesheet), then these styles are auto-injected inline on the DOM parent element.

When scanning parent DOM element for flexbox flow direction, first scan the in element's inline style and then scan the computed stylesheet for the `flex-direction` value.

> It is assumed that if the parent element has a `flex-direction` style property set, then the other required properties (display, border-box, etc.)  have also been assigned.

Fixes #272, #364.
  • Loading branch information
ThomasBurleson authored and mmalerba committed Aug 7, 2017
1 parent 0f13b14 commit 635c4f5
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 6 deletions.
12 changes: 9 additions & 3 deletions src/lib/flexbox/api/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {ResponsiveActivation, KeyOptions} from '../responsive/responsive-activat
import {MediaMonitor} from '../../media-query/media-monitor';
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}).
Expand Down Expand Up @@ -120,9 +119,16 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
let element: HTMLElement = source || this._elementRef.nativeElement;
let value = this._lookupStyle(element, 'display');

return value ? value.trim() : ((element.nodeType === 1) ? 'block' : 'inline-block');
return value ? value.trim() :
((element.nodeType === 1) ? 'block' : 'inline-block');
}

/**
* Determine the DOM element's Flexbox flow (flex-direction).
*
* Check inline style first then check computed (stylesheet) style.
* And optionally add the flow value to element's inline style.
*/
protected _getFlowDirection(target: any, addIfMissing = false): string {
let value = 'row';

Expand All @@ -146,7 +152,7 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
try {
if (element) {
let immediateValue = getDom().getStyle(element, styleName);
value = immediateValue || getDom().getComputedStyle(element).display;
value = immediateValue || getDom().getComputedStyle(element).getPropertyValue(styleName);
}
} catch (e) {
// TODO: platform-server throws an exception for getComputedStyle
Expand Down
20 changes: 20 additions & 0 deletions src/lib/flexbox/api/flex.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,26 @@ describe('flex directive', () => {
expect(element).not.toHaveCssStyle({'min-width': '30px'});
});

it('should CSS stylesheet and not inject flex-direction on parent', () => {
componentWithTemplate(`
<style>
.test { flex-direction:column; display: flex; }
</style>
<div class='test'>
<div fxFlex='30px' fxFlex.gt-sm='50' > </div>
</div>
`);

fixture.detectChanges();
let parent = queryFor(fixture, '.test')[0].nativeElement;
let element = queryFor(fixture, '[fxFlex]')[0].nativeElement;

// parent flex-direction found with 'column' with child height styles
expect(parent).toHaveCssStyle({'flex-direction': 'column', 'display': 'flex'});
expect(element).toHaveCssStyle({'min-height': '30px'});
expect(element).not.toHaveCssStyle({'min-width': '30px'});
});

it('should not work with non-direct-parent fxLayouts', async(() => {
componentWithTemplate(`
<div fxLayout='column'>
Expand Down
6 changes: 3 additions & 3 deletions src/lib/flexbox/api/flex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
@Input('fxFlex.lt-md') set flexLtMd(val) { this._cacheInput('flexLtMd', val); };
@Input('fxFlex.lt-lg') set flexLtLg(val) { this._cacheInput('flexLtLg', val); };
@Input('fxFlex.lt-xl') set flexLtXl(val) { this._cacheInput('flexLtXl', val); };

/* tslint:enable */
// Explicitly @SkipSelf on LayoutDirective and LayoutWrapDirective because we want the
// parent flex container for this flex item.

// Note: Explicitly @SkipSelf on LayoutDirective and LayoutWrapDirective because we are looking
// for the parent flex container for this flex item.
constructor(monitor: MediaMonitor,
elRef: ElementRef,
renderer: Renderer,
Expand Down

0 comments on commit 635c4f5

Please sign in to comment.