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

Commit 3a0ec5d

Browse files
CaerusKaruCaerusKaru
authored
fix(show-hide): work with Angular components and elements without fxLayout (#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
1 parent 5e3ec0e commit 3a0ec5d

File tree

10 files changed

+1032
-888
lines changed

10 files changed

+1032
-888
lines changed

‎package-lock.json

+838-796
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+25-25
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@
2828
"version": "7.0.0-beta.19",
2929
"requiredAngularVersion": ">=7.0.0-rc.0",
3030
"dependencies": {
31-
"@angular/cdk": "^7.0.0-rc.0",
32-
"@angular/common": "^7.0.0-rc.0",
33-
"@angular/compiler": "^7.0.0-rc.0",
34-
"@angular/core": "^7.0.0-rc.0",
35-
"@angular/platform-browser": "^7.0.0-rc.0",
31+
"@angular/cdk": "^7.0.3",
32+
"@angular/common": "^7.0.3",
33+
"@angular/compiler": "^7.0.3",
34+
"@angular/core": "^7.0.3",
35+
"@angular/platform-browser": "^7.0.3",
3636
"core-js": "^2.5.7",
3737
"rxjs": "^6.3.0",
3838
"systemjs": "0.19.43",
@@ -41,36 +41,36 @@
4141
"zone.js": "^0.8.26"
4242
},
4343
"devDependencies": {
44-
"@angular/animations": "^7.0.0-rc.0",
45-
"@angular/compiler-cli": "^7.0.0-rc.0",
46-
"@angular/forms": "^7.0.0-rc.0",
47-
"@angular/http": "^7.0.0-rc.0",
48-
"@angular/material": "^7.0.0-rc.0",
49-
"@angular/platform-browser-dynamic": "^7.0.0-rc.0",
50-
"@angular/platform-server": "^7.0.0-rc.0",
51-
"@angular/router": "^7.0.0-rc.0",
44+
"@angular/animations": "^7.0.3",
45+
"@angular/compiler-cli": "^7.0.3",
46+
"@angular/forms": "^7.0.3",
47+
"@angular/http": "^7.0.3",
48+
"@angular/material": "^7.0.3",
49+
"@angular/platform-browser-dynamic": "^7.0.3",
50+
"@angular/platform-server": "^7.0.3",
51+
"@angular/router": "^7.0.3",
5252
"@google-cloud/storage": "^1.7.0",
5353
"@types/chalk": "^0.4.31",
5454
"@types/fs-extra": "^4.0.5",
5555
"@types/glob": "^5.0.33",
5656
"@types/gulp": "3.8.32",
5757
"@types/hammerjs": "^2.0.34",
58-
"@types/jasmine": "^2.8.8",
58+
"@types/jasmine": "^2.8.11",
5959
"@types/merge2": "^0.3.30",
6060
"@types/minimist": "^1.2.0",
61-
"@types/node": "^7.0.70",
61+
"@types/node": "^7.10.1",
6262
"@types/run-sequence": "^0.0.29",
6363
"@types/rx": "2.5.33",
6464
"axe-webdriverjs": "^1.1.5",
6565
"chalk": "^1.1.3",
66-
"cli-color": "^1.3.0",
66+
"cli-color": "^1.4.0",
6767
"dgeni": "^0.4.10",
68-
"dgeni-packages": "^0.26.7",
69-
"firebase": "^5.5.2",
68+
"dgeni-packages": "^0.26.12",
69+
"firebase": "^5.5.8",
7070
"firebase-admin": "^5.13.1",
7171
"firebase-tools": "^4.2.1",
7272
"fs-extra": "^3.0.1",
73-
"git-semver-tags": "^2.0.0",
73+
"git-semver-tags": "^2.0.2",
7474
"glob": "^7.1.3",
7575
"google-closure-compiler": "20170409.0.0",
7676
"gulp": "^3.9.1",
@@ -89,20 +89,20 @@
8989
"gulp-sass": "^3.2.1",
9090
"gulp-transform": "^2.0.0",
9191
"hammerjs": "^2.0.8",
92-
"highlight.js": "^9.11.0",
92+
"highlight.js": "^9.13.1",
9393
"http-rewrite-middleware": "^0.1.6",
9494
"jasmine-core": "^2.8.0",
95-
"karma": "^3.0.0",
95+
"karma": "^3.1.1",
9696
"karma-browserstack-launcher": "^1.3.0",
9797
"karma-chrome-launcher": "^2.2.0",
9898
"karma-coverage": "^1.1.2",
9999
"karma-firefox-launcher": "^1.0.1",
100100
"karma-jasmine": "^1.1.2",
101101
"karma-sauce-launcher": "^1.2.0",
102102
"karma-sourcemap-loader": "^0.3.7",
103-
"madge": "^3.2.0",
103+
"madge": "^3.3.0",
104104
"magic-string": "^0.22.5",
105-
"merge2": "^1.2.2",
105+
"merge2": "^1.2.3",
106106
"minimatch": "^3.0.4",
107107
"minimist": "^1.2.0",
108108
"prompt-sync": "^4.1.6",
@@ -116,12 +116,12 @@
116116
"scss-bundle": "^2.4.0",
117117
"selenium-webdriver": "^3.6.0",
118118
"sorcery": "^0.10.0",
119-
"stylelint": "^9.6.0",
119+
"stylelint": "^9.8.0",
120120
"temp": "0.8.3",
121121
"ts-node": "^3.0.4",
122122
"tsconfig-paths": "^2.3.0",
123123
"tslint": "^5.11.0",
124-
"typescript": "~3.1.1",
124+
"typescript": "^3.1.6",
125125
"uglify-js": "^2.8.14"
126126
}
127127
}

‎src/lib/core/style-utils/style-utils.spec.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ describe('styler', () => {
3939

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

42-
it('should default to "display:block" for <div></div>', () => {
42+
it('should not have a default for <div></div>', () => {
4343
componentWithTemplate(`
4444
<div></div>
4545
`);
46-
expectNativeEl(fixture).toHaveCSS({'display': 'block'}, styler);
46+
expectNativeEl(fixture).not.toHaveStyle({'display': 'block'}, styler);
4747
});
4848

4949
it('should find to "display" for inline style <div></div>', () => {

‎src/lib/core/style-utils/style-utils.ts

+1-6
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,6 @@ export class StyleUtils {
5454
getFlowDirection(target: HTMLElement): [string, string] {
5555
const query = 'flex-direction';
5656
let value = this.lookupStyle(target, query);
57-
if (value === FALLBACK_STYLE) {
58-
value = '';
59-
}
6057
const hasInlineValue = this.lookupInlineStyle(target, query) ||
6158
(isPlatformServer(this._platformId) && this._serverModuleLoaded) ? value : '';
6259

@@ -101,7 +98,7 @@ export class StyleUtils {
10198

10299
// Note: 'inline' is the default of all elements, unless UA stylesheet overrides;
103100
// in which case getComputedStyle() should determine a valid value.
104-
return value ? value.trim() : FALLBACK_STYLE;
101+
return value.trim();
105102
}
106103

107104
/**
@@ -176,5 +173,3 @@ export class StyleUtils {
176173
* map of property name and value (e.g. {display: 'none', flex-order: 5})
177174
*/
178175
export type StyleDefinition = { [property: string]: string | number | null };
179-
180-
const FALLBACK_STYLE = 'block';

‎src/lib/extended/show-hide/hide.spec.ts

+18-27
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.io/license
77
*/
8-
import {Component, OnInit, PLATFORM_ID} from '@angular/core';
9-
import {CommonModule, isPlatformServer} from '@angular/common';
8+
import {Component, OnInit} from '@angular/core';
9+
import {CommonModule} from '@angular/common';
1010
import {ComponentFixture, TestBed, inject} from '@angular/core/testing';
1111
import {
1212
MatchMedia,
@@ -29,15 +29,12 @@ describe('hide directive', () => {
2929
let fixture: ComponentFixture<any>;
3030
let matchMedia: MockMatchMedia;
3131
let styler: StyleUtils;
32-
let platformId: Object;
3332
let createTestComponent = (template: string) => {
3433
fixture = makeCreateTestComponent(() => TestHideComponent)(template);
3534

36-
inject([MatchMedia, StyleUtils, PLATFORM_ID],
37-
(_matchMedia: MockMatchMedia, _styler: StyleUtils, _platformId: Object) => {
35+
inject([MatchMedia, StyleUtils], (_matchMedia: MockMatchMedia, _styler: StyleUtils) => {
3836
matchMedia = _matchMedia;
3937
styler = _styler;
40-
platformId = _platformId;
4138
})();
4239

4340
return fixture;
@@ -80,19 +77,19 @@ describe('hide directive', () => {
8077

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

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

9188
it('should update styles with binding changes', () => {
9289
createTestComponent(`<div [fxHide]="menuHidden"></div>`);
9390
expectNativeEl(fixture).toHaveStyle({'display': 'none'}, styler);
9491
fixture.componentInstance.toggleMenu();
95-
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
92+
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);
9693
fixture.componentInstance.toggleMenu();
9794
expectNativeEl(fixture).toHaveStyle({'display': 'none'}, styler);
9895
});
@@ -103,16 +100,15 @@ describe('hide directive', () => {
103100
'display': 'none'
104101
}, styler);
105102

106-
// TODO(CaerusKaru): Domino doesn't calculate the right styles here
107-
expectNativeEl(fixture, {isHidden: false}).toHaveStyle({
108-
'display': isPlatformServer(platformId) ? 'block' : 'inline-block'
103+
expectNativeEl(fixture, {isHidden: false}).not.toHaveStyle({
104+
'display': 'none'
109105
}, styler);
110106
});
111107

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

118114

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

126122
expectNativeEl(fixture).toHaveStyle({'display': 'none'}, styler);
127123
matchMedia.activate('xs');
128-
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
124+
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);
129125
matchMedia.activate('md');
130126
expectNativeEl(fixture).toHaveStyle({'display': 'none'}, styler);
131127
});
@@ -172,24 +168,24 @@ describe('hide directive', () => {
172168

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

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

180176
matchMedia.activate('lg');
181-
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
177+
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);
182178
});
183179

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

188184
matchMedia.activate('xs');
189-
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
185+
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);
190186

191187
matchMedia.activate('md');
192-
expectNativeEl(fixture).toHaveStyle({'display': 'block'}, styler);
188+
expectNativeEl(fixture).not.toHaveStyle({'display': 'none'}, styler);
193189
});
194190

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

214-
expectActivation().toHaveStyle({'display': 'block'}, styler);
210+
expectActivation().not.toHaveStyle({'display': 'none'}, styler);
215211
expectActivation('md').toHaveStyle({'display': 'none'}, styler);
216212
});
217213

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

227-
// TODO(CaerusKaru): the Domino server impl. does not process inline display correctly
228-
expectActivation().toHaveStyle({
229-
'display': isPlatformServer(platformId) ? 'block' : 'inline'
230-
}, styler);
223+
expectActivation().not.toHaveStyle({'display': 'none'}, styler);
231224
expectActivation('xs').toHaveStyle({'display': 'none'}, styler);
232-
expectActivation('md').toHaveStyle({
233-
'display': isPlatformServer(platformId) ? 'block' : 'inline'
234-
}, styler);
225+
expectActivation('md').not.toHaveStyle({'display': 'none'}, styler);
235226
});
236227
});
237228

‎src/lib/extended/show-hide/show-hide.ts

+23-15
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,22 @@ import {
1717
Optional,
1818
Inject,
1919
PLATFORM_ID,
20+
ViewChild,
21+
AfterViewInit,
2022
} from '@angular/core';
2123
import {isPlatformServer} from '@angular/common';
2224
import {
2325
BaseDirective,
26+
LAYOUT_CONFIG,
27+
LayoutConfigOptions,
2428
MediaChange,
2529
MediaMonitor,
2630
SERVER_TOKEN,
2731
StyleUtils,
2832
} from '@angular/flex-layout/core';
33+
import {FlexDirective, LayoutDirective} from '@angular/flex-layout/flex';
2934
import {Subscription} from 'rxjs';
3035

31-
import {LayoutDirective} from '@angular/flex-layout/flex';
32-
3336
const FALSY = ['false', false, 0];
3437

3538
/**
@@ -59,7 +62,8 @@ export function negativeOf(hide: any) {
5962
[fxHide.gt-xs], [fxHide.gt-sm], [fxHide.gt-md], [fxHide.gt-lg]
6063
`
6164
})
62-
export class ShowHideDirective extends BaseDirective implements OnInit, OnChanges, OnDestroy {
65+
export class ShowHideDirective extends BaseDirective
66+
implements OnInit, OnChanges, OnDestroy, AfterViewInit {
6367

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

113+
@ViewChild(FlexDirective) protected _flexChild: FlexDirective | null = null;
114+
109115
constructor(monitor: MediaMonitor,
110116
@Optional() @Self() protected layout: LayoutDirective,
111117
protected elRef: ElementRef,
112118
protected styleUtils: StyleUtils,
113119
@Inject(PLATFORM_ID) protected platformId: Object,
114-
@Optional() @Inject(SERVER_TOKEN) protected serverModuleLoaded: boolean) {
120+
@Optional() @Inject(SERVER_TOKEN) protected serverModuleLoaded: boolean,
121+
@Inject(LAYOUT_CONFIG) protected layoutConfig: LayoutConfigOptions) {
115122

116123
super(monitor, elRef, styleUtils);
117-
118-
if (layout) {
119-
/**
120-
* The Layout can set the display:flex (and incorrectly affect the Hide/Show directives.
121-
* Whenever Layout [on the same element] resets its CSS, then update the Hide/Show CSS
122-
*/
123-
this._layoutWatcher = layout.layout$.subscribe(() => this._updateWithValue());
124-
}
125124
}
126125

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

140140

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

160+
ngAfterViewInit() {
161+
this._display = this._getDisplayStyle();
162+
if (this.layout) {
163+
/**
164+
* The Layout can set the display:flex (and incorrectly affect the Hide/Show directives.
165+
* Whenever Layout [on the same element] resets its CSS, then update the Hide/Show CSS
166+
*/
167+
this._layoutWatcher = this.layout.layout$.subscribe(() => this._updateWithValue());
168+
}
160169
let value = this._getDefaultVal('show', true);
161170
// Build _mqActivation controller
162171
this._listenForMediaQueryChanges('show', value, (changes: MediaChange) => {
163172
this._updateWithValue(changes.value);
164173
});
165-
166174
this._updateWithValue();
167175
}
168176

0 commit comments

Comments
 (0)