Skip to content

Commit

Permalink
fix(show-hide): work with Angular components and elements without fxL…
Browse files Browse the repository at this point in the history 
…ayout (#881)

* Fixes `fxFlex` child interfering with parent `fxShow`/`fxHide`
* Fixes `fxShow`/`fxHide` not applying the correct computed styles for custom elements

Closes #848 
Closes #724
  • Loading branch information
@CaerusKaru
CaerusKaru authored Nov 13, 2018
1 parent 5e3ec0e commit 3a0ec5d
Show file tree
Hide file tree
Showing 10 changed files with 1,032 additions and 888 deletions.
1,634 changes: 838 additions & 796 deletions package-lock.json
View file

Large diffs are not rendered by default.

50 changes: 25 additions & 25 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@
"version": "7.0.0-beta.19",
"requiredAngularVersion": ">=7.0.0-rc.0",
"dependencies": {
"@angular/cdk": "^7.0.0-rc.0",
"@angular/common": "^7.0.0-rc.0",
"@angular/compiler": "^7.0.0-rc.0",
"@angular/core": "^7.0.0-rc.0",
"@angular/platform-browser": "^7.0.0-rc.0",
"@angular/cdk": "^7.0.3",
"@angular/common": "^7.0.3",
"@angular/compiler": "^7.0.3",
"@angular/core": "^7.0.3",
"@angular/platform-browser": "^7.0.3",
"core-js": "^2.5.7",
"rxjs": "^6.3.0",
"systemjs": "0.19.43",
Expand All @@ -41,36 +41,36 @@
"zone.js": "^0.8.26"
},
"devDependencies": {
"@angular/animations": "^7.0.0-rc.0",
"@angular/compiler-cli": "^7.0.0-rc.0",
"@angular/forms": "^7.0.0-rc.0",
"@angular/http": "^7.0.0-rc.0",
"@angular/material": "^7.0.0-rc.0",
"@angular/platform-browser-dynamic": "^7.0.0-rc.0",
"@angular/platform-server": "^7.0.0-rc.0",
"@angular/router": "^7.0.0-rc.0",
"@angular/animations": "^7.0.3",
"@angular/compiler-cli": "^7.0.3",
"@angular/forms": "^7.0.3",
"@angular/http": "^7.0.3",
"@angular/material": "^7.0.3",
"@angular/platform-browser-dynamic": "^7.0.3",
"@angular/platform-server": "^7.0.3",
"@angular/router": "^7.0.3",
"@google-cloud/storage": "^1.7.0",
"@types/chalk": "^0.4.31",
"@types/fs-extra": "^4.0.5",
"@types/glob": "^5.0.33",
"@types/gulp": "3.8.32",
"@types/hammerjs": "^2.0.34",
"@types/jasmine": "^2.8.8",
"@types/jasmine": "^2.8.11",
"@types/merge2": "^0.3.30",
"@types/minimist": "^1.2.0",
"@types/node": "^7.0.70",
"@types/node": "^7.10.1",
"@types/run-sequence": "^0.0.29",
"@types/rx": "2.5.33",
"axe-webdriverjs": "^1.1.5",
"chalk": "^1.1.3",
"cli-color": "^1.3.0",
"cli-color": "^1.4.0",
"dgeni": "^0.4.10",
"dgeni-packages": "^0.26.7",
"firebase": "^5.5.2",
"dgeni-packages": "^0.26.12",
"firebase": "^5.5.8",
"firebase-admin": "^5.13.1",
"firebase-tools": "^4.2.1",
"fs-extra": "^3.0.1",
"git-semver-tags": "^2.0.0",
"git-semver-tags": "^2.0.2",
"glob": "^7.1.3",
"google-closure-compiler": "20170409.0.0",
"gulp": "^3.9.1",
Expand All @@ -89,20 +89,20 @@
"gulp-sass": "^3.2.1",
"gulp-transform": "^2.0.0",
"hammerjs": "^2.0.8",
"highlight.js": "^9.11.0",
"highlight.js": "^9.13.1",
"http-rewrite-middleware": "^0.1.6",
"jasmine-core": "^2.8.0",
"karma": "^3.0.0",
"karma": "^3.1.1",
"karma-browserstack-launcher": "^1.3.0",
"karma-chrome-launcher": "^2.2.0",
"karma-coverage": "^1.1.2",
"karma-firefox-launcher": "^1.0.1",
"karma-jasmine": "^1.1.2",
"karma-sauce-launcher": "^1.2.0",
"karma-sourcemap-loader": "^0.3.7",
"madge": "^3.2.0",
"madge": "^3.3.0",
"magic-string": "^0.22.5",
"merge2": "^1.2.2",
"merge2": "^1.2.3",
"minimatch": "^3.0.4",
"minimist": "^1.2.0",
"prompt-sync": "^4.1.6",
Expand All @@ -116,12 +116,12 @@
"scss-bundle": "^2.4.0",
"selenium-webdriver": "^3.6.0",
"sorcery": "^0.10.0",
"stylelint": "^9.6.0",
"stylelint": "^9.8.0",
"temp": "0.8.3",
"ts-node": "^3.0.4",
"tsconfig-paths": "^2.3.0",
"tslint": "^5.11.0",
"typescript": "~3.1.1",
"typescript": "^3.1.6",
"uglify-js": "^2.8.14"
}
}
4 changes: 2 additions & 2 deletions src/lib/core/style-utils/style-utils.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ describe('styler', () => {

describe('testing display styles', () => {

it('should default to "display:block" for <div></div>', () => {
it('should not have a default for <div></div>', () => {
componentWithTemplate(`
<div></div>
`);
expectNativeEl(fixture).toHaveCSS({'display': 'block'}, styler);
expectNativeEl(fixture).not.toHaveStyle({'display': 'block'}, styler);
});

it('should find to "display" for inline style <div></div>', () => {
Expand Down
7 changes: 1 addition & 6 deletions src/lib/core/style-utils/style-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ export class StyleUtils {
getFlowDirection(target: HTMLElement): [string, string] {
const query = 'flex-direction';
let value = this.lookupStyle(target, query);
if (value === FALLBACK_STYLE) {
value = '';
}
const hasInlineValue = this.lookupInlineStyle(target, query) ||
(isPlatformServer(this._platformId) && this._serverModuleLoaded) ? value : '';

Expand Down Expand Up @@ -101,7 +98,7 @@ export class StyleUtils {

// 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() : FALLBACK_STYLE;
return value.trim();
}

/**
Expand Down Expand Up @@ -176,5 +173,3 @@ export class StyleUtils {
* map of property name and value (e.g. {display: 'none', flex-order: 5})
*/
export type StyleDefinition = { [property: string]: string | number | null };

const FALLBACK_STYLE = 'block';
45 changes: 18 additions & 27 deletions src/lib/extended/show-hide/hide.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
* 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, OnInit, PLATFORM_ID} from '@angular/core';
import {CommonModule, isPlatformServer} from '@angular/common';
import {Component, OnInit} from '@angular/core';
import {CommonModule} from '@angular/common';
import {ComponentFixture, TestBed, inject} from '@angular/core/testing';
import {
MatchMedia,
Expand All @@ -29,15 +29,12 @@ describe('hide directive', () => {
let fixture: ComponentFixture<any>;
let matchMedia: MockMatchMedia;
let styler: StyleUtils;
let platformId: Object;
let createTestComponent = (template: string) => {
fixture = makeCreateTestComponent(() => TestHideComponent)(template);

inject([MatchMedia, StyleUtils, PLATFORM_ID],
(_matchMedia: MockMatchMedia, _styler: StyleUtils, _platformId: Object) => {
inject([MatchMedia, StyleUtils], (_matchMedia: MockMatchMedia, _styler: StyleUtils) => {
matchMedia = _matchMedia;
styler = _styler;
platformId = _platformId;
})();

return fixture;
Expand Down Expand Up @@ -80,19 +77,19 @@ describe('hide directive', () => {

it('should initial with component visible when set to `false`', () => {
createTestComponent(`<div fxHide="false"></div>`);
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);
});

it('should initial with component visible when set to `0`', () => {
createTestComponent(`<div [fxHide]="isVisible"></div>`);
expectNativeEl(fixture, {isVisible: 0}).toHaveStyle({'display': 'block'}, styler);
expectNativeEl(fixture, {isVisible: 0}).not.toHaveStyle({'display': 'none'}, styler);
});

it('should update styles with binding changes', () => {
createTestComponent(`<div [fxHide]="menuHidden"></div>`);
expectNativeEl(fixture).toHaveStyle({'display': 'none'}, styler);
fixture.componentInstance.toggleMenu();
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);
fixture.componentInstance.toggleMenu();
expectNativeEl(fixture).toHaveStyle({'display': 'none'}, styler);
});
Expand All @@ -103,16 +100,15 @@ describe('hide directive', () => {
'display': 'none'
}, styler);

// TODO(CaerusKaru): Domino doesn't calculate the right styles here
expectNativeEl(fixture, {isHidden: false}).toHaveStyle({
'display': isPlatformServer(platformId) ? 'block' : 'inline-block'
expectNativeEl(fixture, {isHidden: false}).not.toHaveStyle({
'display': 'none'
}, styler);
});

it('should use "flex" display style when the element also has an fxLayout', () => {
createTestComponent(`<div fxLayout [fxHide]="isHidden"></div>`);
expectNativeEl(fixture, {isHidden: true}).toHaveStyle({'display': 'none'}, styler);
expectNativeEl(fixture, {isHidden: false}).toHaveStyle({'display': 'block'}, styler);
expectNativeEl(fixture, {isHidden: false}).not.toHaveStyle({'display': 'none'}, styler);
});


Expand All @@ -125,7 +121,7 @@ describe('hide directive', () => {

expectNativeEl(fixture).toHaveStyle({'display': 'none'}, styler);
matchMedia.activate('xs');
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);
matchMedia.activate('md');
expectNativeEl(fixture).toHaveStyle({'display': 'none'}, styler);
});
Expand Down Expand Up @@ -172,24 +168,24 @@ describe('hide directive', () => {

it('should support use of the `media` observable in templates ', () => {
createTestComponent(`<div [fxHide]="media.isActive('xs')"></div>`);
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);

matchMedia.activate('xs');
expectNativeEl(fixture).toHaveStyle({'display': 'none'}, styler);

matchMedia.activate('lg');
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);
});

it('should support use of the `media` observable in adaptive templates ', () => {
createTestComponent(`<div fxHide="false" [fxHide.md]="media.isActive('xs')"></div>`);
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);

matchMedia.activate('xs');
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);

matchMedia.activate('md');
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);
});

it('should hide when used with fxLayout and the ".md" breakpoint activates', () => {
Expand All @@ -211,7 +207,7 @@ describe('hide directive', () => {
let expectActivation: any =
makeExpectWithActivation(createTestComponent(template), '.hideOnMd');

expectActivation().toHaveStyle({'display': 'block'}, styler);
expectActivation().not.toHaveStyle({'display': 'none'}, styler);
expectActivation('md').toHaveStyle({'display': 'none'}, styler);
});

Expand All @@ -224,14 +220,9 @@ describe('hide directive', () => {
let expectActivation: any =
makeExpectWithActivation(createTestComponent(template), '.hideOnXs');

// TODO(CaerusKaru): the Domino server impl. does not process inline display correctly
expectActivation().toHaveStyle({
'display': isPlatformServer(platformId) ? 'block' : 'inline'
}, styler);
expectActivation().not.toHaveStyle({'display': 'none'}, styler);
expectActivation('xs').toHaveStyle({'display': 'none'}, styler);
expectActivation('md').toHaveStyle({
'display': isPlatformServer(platformId) ? 'block' : 'inline'
}, styler);
expectActivation('md').not.toHaveStyle({'display': 'none'}, styler);
});
});

Expand Down
38 changes: 23 additions & 15 deletions src/lib/extended/show-hide/show-hide.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,22 @@ import {
Optional,
Inject,
PLATFORM_ID,
ViewChild,
AfterViewInit,
} from '@angular/core';
import {isPlatformServer} from '@angular/common';
import {
BaseDirective,
LAYOUT_CONFIG,
LayoutConfigOptions,
MediaChange,
MediaMonitor,
SERVER_TOKEN,
StyleUtils,
} from '@angular/flex-layout/core';
import {FlexDirective, LayoutDirective} from '@angular/flex-layout/flex';
import {Subscription} from 'rxjs';

import {LayoutDirective} from '@angular/flex-layout/flex';

const FALSY = ['false', false, 0];

/**
Expand Down Expand Up @@ -59,7 +62,8 @@ export function negativeOf(hide: any) {
[fxHide.gt-xs], [fxHide.gt-sm], [fxHide.gt-md], [fxHide.gt-lg]
`
})
export class ShowHideDirective extends BaseDirective implements OnInit, OnChanges, OnDestroy {
export class ShowHideDirective extends BaseDirective
implements OnInit, OnChanges, OnDestroy, AfterViewInit {

/**
* Subscription to the parent flex container's layout changes.
Expand Down Expand Up @@ -106,22 +110,17 @@ export class ShowHideDirective extends BaseDirective implements OnInit, OnChange
@Input('fxHide.gt-lg') set hideGtLg(val: string) {this._cacheInput('showGtLg', negativeOf(val)); };
/* tslint:enable */

@ViewChild(FlexDirective) protected _flexChild: FlexDirective | null = null;

constructor(monitor: MediaMonitor,
@Optional() @Self() protected layout: LayoutDirective,
protected elRef: ElementRef,
protected styleUtils: StyleUtils,
@Inject(PLATFORM_ID) protected platformId: Object,
@Optional() @Inject(SERVER_TOKEN) protected serverModuleLoaded: boolean) {
@Optional() @Inject(SERVER_TOKEN) protected serverModuleLoaded: boolean,
@Inject(LAYOUT_CONFIG) protected layoutConfig: LayoutConfigOptions) {

super(monitor, elRef, styleUtils);

if (layout) {
/**
* The Layout can set the display:flex (and incorrectly affect the Hide/Show directives.
* Whenever Layout [on the same element] resets its CSS, then update the Hide/Show CSS
*/
this._layoutWatcher = layout.layout$.subscribe(() => this._updateWithValue());
}
}

// *********************************************
Expand All @@ -134,7 +133,8 @@ export class ShowHideDirective extends BaseDirective implements OnInit, OnChange
* unless it was already explicitly specified inline or in a CSS stylesheet.
*/
protected _getDisplayStyle(): string {
return this.layout ? 'flex' : super._getDisplayStyle();
return (this.layout || (this._flexChild && this.layoutConfig.addFlexToParent)) ?
'flex' : super._getDisplayStyle();
}


Expand All @@ -155,14 +155,22 @@ export class ShowHideDirective extends BaseDirective implements OnInit, OnChange
*/
ngOnInit() {
super.ngOnInit();
this._display = this._getDisplayStyle();
}

ngAfterViewInit() {
this._display = this._getDisplayStyle();
if (this.layout) {
/**
* The Layout can set the display:flex (and incorrectly affect the Hide/Show directives.
* Whenever Layout [on the same element] resets its CSS, then update the Hide/Show CSS
*/
this._layoutWatcher = this.layout.layout$.subscribe(() => this._updateWithValue());
}
let value = this._getDefaultVal('show', true);
// Build _mqActivation controller
this._listenForMediaQueryChanges('show', value, (changes: MediaChange) => {
this._updateWithValue(changes.value);
});

this._updateWithValue();
}

Expand Down
Loading

0 comments on commit 3a0ec5d

Please sign in to comment.