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

Commit

Permalink
fix(api): defer getComputedStyle() calls until ngOnInit phase
Browse files Browse the repository at this point in the history
The Show/Hide directives would query the `display` style during construction. For elements constructed during `*ngFor`, calls to getComputedStyle() would incorrectly return “”. Query for styles in CSS stylesheets or inline styles would fail.

Deferring these types of queries to the ngOnInit() lifecycle event, enables valid response to queries for both inline styles and stylesheets.

Fixes #310.
  • Loading branch information
ThomasBurleson committed Aug 10, 2017
1 parent 695bf37 commit a57fa3f
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 5 deletions.
24 changes: 22 additions & 2 deletions src/lib/flexbox/api/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
constructor(protected _mediaMonitor: MediaMonitor,
protected _elementRef: ElementRef,
protected _renderer: Renderer2) {
this._display = this._getDisplayStyle();
}

// *********************************************
Expand All @@ -92,6 +91,15 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
// Lifecycle Methods
// *********************************************

/**
* Use post-component-initialization event to perform extra
* querying such as computed Display style
*/
ngOnInit() {
this._display = this._getDisplayStyle();
this._hasInitialized = true;
}

ngOnChanges(change: SimpleChanges) {
throw new Error(`BaseFxDirective::ngOnChanges should be overridden in subclass: ${change}`);
}
Expand Down Expand Up @@ -126,7 +134,8 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
let element: HTMLElement = source || this._elementRef.nativeElement;
let value = this._lookupStyle(element, 'display');

// Note: 'inline' is the default of all elements, unless UA stylesheet overrides
// Note: 'inline' is the default of all elements, unless UA stylesheet overrides;
// in which case getComputedStyle() should determine a valid value.
return value ? value.trim() : 'inline';
}

Expand Down Expand Up @@ -264,6 +273,10 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
return this._mqActivation.hasKeyValue(key);
}

protected get hasInitialized() {
return this._hasInitialized;
}

/** Original dom Elements CSS display style */
protected _display;

Expand All @@ -277,4 +290,11 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
*/
protected _inputMap = {};

/**
* Has the `ngOnInit()` method fired
*
* Used to allow *ngFor tasks to finish and support queries like
* getComputedStyle() during ngOnInit().
*/
protected _hasInitialized = false;
}
2 changes: 2 additions & 0 deletions src/lib/flexbox/api/flex-align.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ export class FlexAlignDirective extends BaseFxDirective implements OnInit, OnCha
* mql change events to onMediaQueryChange handlers
*/
ngOnInit() {
super.ngOnInit();

this._listenForMediaQueryChanges('align', 'stretch', (changes: MediaChange) => {
this._updateWithValue(changes.value);
});
Expand Down
2 changes: 2 additions & 0 deletions src/lib/flexbox/api/flex-offset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ export class FlexOffsetDirective extends BaseFxDirective implements OnInit, OnCh
* mql change events to onMediaQueryChange handlers
*/
ngOnInit() {
super.ngOnInit();

this._listenForMediaQueryChanges('offset', 0 , (changes: MediaChange) => {
this._updateWithValue(changes.value);
});
Expand Down
2 changes: 2 additions & 0 deletions src/lib/flexbox/api/flex-order.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ export class FlexOrderDirective extends BaseFxDirective implements OnInit, OnCha
* mql change events to onMediaQueryChange handlers
*/
ngOnInit() {
super.ngOnInit();

this._listenForMediaQueryChanges('order', '0', (changes: MediaChange) => {
this._updateWithValue(changes.value);
});
Expand Down
2 changes: 2 additions & 0 deletions src/lib/flexbox/api/flex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
* mql change events to onMediaQueryChange handlers
*/
ngOnInit() {
super.ngOnInit();

this._listenForMediaQueryChanges('flex', '', (changes: MediaChange) => {
this._updateStyle(changes.value);
});
Expand Down
2 changes: 2 additions & 0 deletions src/lib/flexbox/api/layout-align.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ export class LayoutAlignDirective extends BaseFxDirective implements OnInit, OnC
* mql change events to onMediaQueryChange handlers
*/
ngOnInit() {
super.ngOnInit();

this._listenForMediaQueryChanges('align', 'start stretch', (changes: MediaChange) => {
this._updateWithValue(changes.value);
});
Expand Down
2 changes: 2 additions & 0 deletions src/lib/flexbox/api/layout-wrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ export class LayoutWrapDirective extends BaseFxDirective implements OnInit, OnCh
* mql change events to onMediaQueryChange handlers
*/
ngOnInit() {
super.ngOnInit();

this._listenForMediaQueryChanges('wrap', 'wrap', (changes: MediaChange) => {
this._updateWithValue(changes.value);
});
Expand Down
2 changes: 2 additions & 0 deletions src/lib/flexbox/api/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ export class LayoutDirective extends BaseFxDirective implements OnInit, OnChange
* mql change events to onMediaQueryChange handlers
*/
ngOnInit() {
super.ngOnInit();

this._listenForMediaQueryChanges('layout', 'row', (changes: MediaChange) => {
this._updateWithDirection(changes.value);
});
Expand Down
6 changes: 3 additions & 3 deletions src/lib/flexbox/api/show-hide.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ export class ShowHideDirective extends BaseFxDirective implements OnInit, OnChan

super(monitor, elRef, renderer);

this._display = this._getDisplayStyle(); // re-invoke override to use `this._layout`
if (_layout) {
/**
* The Layout can set the display:flex (and incorrectly affect the Hide/Show directives.
Expand Down Expand Up @@ -138,7 +137,7 @@ export class ShowHideDirective extends BaseFxDirective implements OnInit, OnChan
* Then conditionally override with the mq-activated Input's current value
*/
ngOnChanges(changes: SimpleChanges) {
if (changes['show'] != null || this._mqActivation) {
if (this.hasInitialized && (changes['show'] != null || this._mqActivation)) {
this._updateWithValue();
}
}
Expand All @@ -148,8 +147,9 @@ export class ShowHideDirective extends BaseFxDirective implements OnInit, OnChan
* mql change events to onMediaQueryChange handlers
*/
ngOnInit() {
let value = this._getDefaultVal('show', true);
super.ngOnInit();

let value = this._getDefaultVal('show', true);
// Build _mqActivation controller
this._listenForMediaQueryChanges('show', value, (changes: MediaChange) => {
this._updateWithValue(changes.value);
Expand Down

0 comments on commit a57fa3f

Please sign in to comment.