Skip to content

Commit

Permalink
fix(ngStyle, ngClass): StyleDirective security fixes & merge activate…
Browse files Browse the repository at this point in the history 
…d styles (#198)

BREAKING CHANGE:

* `[style.<alias>]` selectors are deprecated in favor of `[ngStyle.<alias>]` selectors
* `[class.<alias>]` selectors are deprecated in favor of `[ngClass.<alias>]` selectors
* default styles are merged with activated styles

```html
<div  fxLayout
  [class.xs]="['xs-1', 'xs-2']"
  [style]="{'font-size': '10px', 'margin-left' : '13px'}"
  [style.xs]="{'font-size': '16px'}"
  [style.md]="{'font-size': '12px'}">
</div>
```

```html
<div  fxLayout
  [ngClass.xs]="['xs-1', 'xs-2']"
  [ngStyle]="{'font-size': '10px', 'margin-left' : '13px'}"
  [ngStyle.xs]="{'font-size': '16px'}"
  [ngStyle.md]="{'font-size': '12px'}">
</div>
```

Fixes #197.
  • Loading branch information
@ThomasBurleson @kara
ThomasBurleson authored and kara committed Feb 27, 2017
1 parent 10e1230 commit eb22fe5
Show file tree
Hide file tree
Showing 11 changed files with 542 additions and 210 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@
"ts-node": "^0.7.3",
"tslint": "^4.2.0",
"tslint-loader": "^3.3.0",
"typescript": "2.0.10",
"typescript": "^2.0.10",
"url-loader": "^0.5.7",
"webpack": "2.2.0-rc.3",
"webpack-bundle-analyzer": "^2.2.0",
Expand Down
9 changes: 6 additions & 3 deletions src/demo-app/app/github-issues/DemosGithubIssues.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import {Component} from '@angular/core';
template: `
<demo-issue-5345></demo-issue-5345>
<demo-issue-9897></demo-issue-9897>
<demo-issue-181></demo-issue-181>
<demo-issue-135> </demo-issue-135>
<demo-issue-181></demo-issue-181>
<demo-issue-197></demo-issue-197>
`
})
export class DemosGithubIssues {
Expand All @@ -19,16 +20,18 @@ import {FlexLayoutModule} from "../../../lib"; // `gulp build:components` to

import {DemoIssue5345} from "./issue.5345.demo";
import {DemoIssue9897} from "./issue.9897.demo";
import {DemoIssue181} from './issue.181.demo';
import {DemoIssue135} from "./issue.135.demo";
import {DemoIssue181} from './issue.181.demo';
import {DemoIssue197} from './issue.197.demo';

@NgModule({
declarations: [
DemosGithubIssues, // used by the Router with the root app component
DemoIssue5345,
DemoIssue9897,
DemoIssue135,
DemoIssue181,
DemoIssue135
DemoIssue197
],
imports: [
CommonModule,
Expand Down
58 changes: 58 additions & 0 deletions src/demo-app/app/github-issues/issue.197.demo.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import {Component, OnDestroy} from '@angular/core';
import {Subscription} from "rxjs/Subscription";
import 'rxjs/add/operator/filter';

import {MediaChange} from "../../../lib/media-query/media-change";
import {ObservableMedia} from "../../../lib/media-query/observable-media-service";

// [ngStyle="{'font-size.px': 10, color: 'rgb(0,0,0)', 'text-align':'left'}"
// style="font-size:10px; color:black; text-align:left;"
@Component({
selector: 'demo-issue-197',
styleUrls: [
'../demo-app/material2.css'
],
template: `
<md-card class="card-demo" >
<md-card-title><a href="https://github.com/angular/flex-layout/issues/197" target="_blank">Issue #197</a></md-card-title>
<md-card-subtitle>Responsive Style directive should merge with default inline style:</md-card-subtitle>
<md-card-content>
<div class="containerX">
<div class="coloredContainerX box fixed">
<div class="box1"
fxFlexFill
style="font-size:12px; color:black; text-align:left;"
[style.sm]="{'font-size': '16px', color: '#a63db8', 'text-align': 'center'}"
ngStyle.md="font-size: 24px; color : #0000ff; text-align: right;">
&lt;div fxFlexFill <br/>
&nbsp;&nbsp;&nbsp;&nbsp;style="font-size:10px; color:black; text-align:'left';"<br/>
&nbsp;&nbsp;&nbsp;&nbsp;[style.sm]="&#123;'font-size':'16px', color:#a63db8, text-align:'center' &#125;"<br/>
&nbsp;&nbsp;&nbsp;&nbsp;ngStyle.md="font-size:24px; color:#00f;" text-align:'right'&gt;<br/>
&lt;/div&gt;
</div>
</div>
</div>
</md-card-content>
<md-card-footer style="width:95%;padding-left:20px;margin-top:-5px;">
<div class="hint" >Active mediaQuery: <span style="padding-left: 20px; color: rgba(0, 0, 0, 0.54)">{{ activeMediaQuery }}</span></div>
</md-card-footer>
</md-card>
`
})
export class DemoIssue197 implements OnDestroy {
public activeMediaQuery = "";

constructor(media$: ObservableMedia) {
this._watcher = media$.subscribe((change: MediaChange) => {
let value = change ? `'${change.mqAlias}' = (${change.mediaQuery})` : "";
this.activeMediaQuery = value;
});
}

ngOnDestroy() {
this._watcher.unsubscribe();
}

private _watcher: Subscription;
}
41 changes: 23 additions & 18 deletions src/lib/flexbox/api/class.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,44 +57,49 @@ describe('class directive', () => {
const selector = `class-${mq}`;
it(`should apply '${selector}' with '${mq}' media query`, () => {
fixture = createTestComponent(`
<div class.${mq}="${selector}">
<div ngClass.${mq}="${selector}">
</div>
`);
activateMediaQuery(mq, true);
activateMediaQuery(mq);
expectNativeEl(fixture).toHaveCssClass(selector);
});
});

it('should keep existing class selector', () => {
fixture = createTestComponent(`
<div class="existing-class" class.xs="xs-class">
</div>
`);
<div class="existing-class" ngClass.xs="xs-class">
</div>
`);

expectNativeEl(fixture).toHaveCssClass('existing-class');
activateMediaQuery('xs', true);
activateMediaQuery('xs');
expectNativeEl(fixture).toHaveCssClass('existing-class');

activateMediaQuery('lg');
expectNativeEl(fixture).toHaveCssClass('existing-class');
expectNativeEl(fixture).not.toHaveCssClass('xs-class');
});

it('should allow more than one responsive breakpoint on one element', () => {
fixture = createTestComponent(`
<div class.xs="xs-class"
class.md="md-class">
<div ngClass.xs="xs-class"
ngClass.md="md-class">
</div>
`);
activateMediaQuery('xs', true);
activateMediaQuery('xs');
expectNativeEl(fixture).toHaveCssClass('xs-class');
expectNativeEl(fixture).not.toHaveCssClass('md-class');
activateMediaQuery('md', true);
activateMediaQuery('md');
expectNativeEl(fixture).not.toHaveCssClass('xs-class');
expectNativeEl(fixture).toHaveCssClass('md-class');
});

it('should work with ngClass object notation', () => {
fixture = createTestComponent(`
<div [class.xs]="{'xs-1': hasXs1, 'xs-2': hasXs2}">
</div>
`);
activateMediaQuery('xs', true);
<div [ngClass.xs]="{'xs-1': hasXs1, 'xs-2': hasXs2}">
</div>
`);
activateMediaQuery('xs');
expectNativeEl(fixture, {hasXs1: true, hasXs2: false}).toHaveCssClass('xs-1');
expectNativeEl(fixture, {hasXs1: true, hasXs2: false}).not.toHaveCssClass('xs-2');

Expand All @@ -104,10 +109,10 @@ describe('class directive', () => {

it('should work with ngClass array notation', () => {
fixture = createTestComponent(`
<div [class.xs]="['xs-1', 'xs-2']">
</div>
`);
activateMediaQuery('xs', true);
<div [ngClass.xs]="['xs-1', 'xs-2']">
</div>
`);
activateMediaQuery('xs');
expectNativeEl(fixture).toHaveCssClass('xs-1');
expectNativeEl(fixture).toHaveCssClass('xs-2');
});
Expand Down
97 changes: 38 additions & 59 deletions src/lib/flexbox/api/class.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,66 +32,43 @@ export type NgClassType = string | string[] | Set<string> | {[klass: string]: an
*/
@Directive({
selector: `
[class.xs],
[class.gt-xs],
[class.sm],
[class.gt-sm],
[class.md],
[class.gt-md],
[class.lg],
[class.gt-lg],
[class.xl]
[ngClass.xs], [class.xs],
[ngClass.gt-xs], [class.gt-xs],
[ngClass.sm], [class.sm],
[ngClass.gt-sm], [class.gt-sm],
[ngClass.md], [class.md],
[ngClass.gt-md], [class.gt-md],
[ngClass.lg], [class.lg],
[ngClass.gt-lg], [class.gt-lg]
`
})
export class ClassDirective extends NgClass implements OnInit, OnChanges, OnDestroy {

@Input('class.xs')
set classXs(val: NgClassType) {
this._base.cacheInput('classXs', val);
}

@Input('class.gt-xs')
set classGtXs(val: NgClassType) {
this._base.cacheInput('classGtXs', val);
};

@Input('class.sm')
set classSm(val: NgClassType) {
this._base.cacheInput('classSm', val);
};

@Input('class.gt-sm')
set classGtSm(val: NgClassType) {
this._base.cacheInput('classGtSm', val);
};

@Input('class.md')
set classMd(val: NgClassType) {
this._base.cacheInput('classMd', val);
};

@Input('class.gt-md')
set classGtMd(val: NgClassType) {
this._base.cacheInput('classGtMd', val);
};

@Input('class.lg')
set classLg(val: NgClassType) {
this._base.cacheInput('classLg', val);
};

@Input('class.gt-lg')
set classGtLg(val: NgClassType) {
this._base.cacheInput('classGtLg', val);
};

@Input('class.xl')
set classXl(val: NgClassType) {
this._base.cacheInput('classXl', val);
};

constructor(private monitor: MediaMonitor,
private _bpRegistry: BreakPointRegistry,
/* tslint:disable */
@Input('ngClass.xs') set ngClassXs(val: NgClassType) { this._base.cacheInput('classXs', val, true); }
@Input('ngClass.gt-xs') set ngClassGtXs(val: NgClassType) { this._base.cacheInput('classGtXs', val, true); };
@Input('ngClass.sm') set ngClassSm(val: NgClassType) { this._base.cacheInput('classSm', val, true); };
@Input('ngClass.gt-sm') set ngClassGtSm(val: NgClassType) { this._base.cacheInput('classGtSm', val, true);} ;
@Input('ngClass.md') set ngClassMd(val: NgClassType) { this._base.cacheInput('classMd', val, true); };
@Input('ngClass.gt-md') set ngClassGtMd(val: NgClassType) { this._base.cacheInput('classGtMd', val, true);};
@Input('ngClass.lg') set ngClassLg(val: NgClassType) { this._base.cacheInput('classLg', val, true);};
@Input('ngClass.gt-lg') set ngClassGtLg(val: NgClassType) { this._base.cacheInput('classGtLg', val, true); };
@Input('ngClass.xl') set ngClassXl(val: NgClassType) { this._base.cacheInput('classXl', val, true); };

/** Deprecated selectors */
@Input('class.xs') set classXs(val: NgClassType) { this._base.cacheInput('classXs', val, true); }
@Input('class.gt-xs') set classGtXs(val: NgClassType) { this._base.cacheInput('classGtXs', val, true); };
@Input('class.sm') set classSm(val: NgClassType) { this._base.cacheInput('classSm', val, true); };
@Input('class.gt-sm') set classGtSm(val: NgClassType) { this._base.cacheInput('classGtSm', val, true); };
@Input('class.md') set classMd(val: NgClassType) { this._base.cacheInput('classMd', val, true);};
@Input('class.gt-md') set classGtMd(val: NgClassType) { this._base.cacheInput('classGtMd', val, true);};
@Input('class.lg') set classLg(val: NgClassType) { this._base.cacheInput('classLg', val, true); };
@Input('class.gt-lg') set classGtLg(val: NgClassType) { this._base.cacheInput('classGtLg', val, true); };
@Input('class.xl') set classXl(val: NgClassType) { this._base.cacheInput('classXl', val, true); };

/* tslint:enable */
constructor(protected monitor: MediaMonitor,
protected _bpRegistry: BreakPointRegistry,
_iterableDiffers: IterableDiffers, _keyValueDiffers: KeyValueDiffers,
_ngEl: ElementRef, _renderer: Renderer) {
super(_iterableDiffers, _keyValueDiffers, _ngEl, _renderer);
Expand All @@ -102,7 +79,9 @@ export class ClassDirective extends NgClass implements OnInit, OnChanges, OnDest
* For @Input changes on the current mq activation property, see onMediaQueryChanges()
*/
ngOnChanges(changes: SimpleChanges) {
const changed = this._bpRegistry.items.some(it => `class${it.suffix}` in changes);
const changed = this._bpRegistry.items.some(it => {
return (`ngClass${it.suffix}` in changes) || (`class${it.suffix}` in changes);
});
if (changed || this._base.mqActivation) {
this._updateStyle();
}
Expand All @@ -123,7 +102,7 @@ export class ClassDirective extends NgClass implements OnInit, OnChanges, OnDest
this._base.ngOnDestroy();
}

private _updateStyle(value?: NgClassType) {
protected _updateStyle(value?: NgClassType) {
let clazz = value || this._base.queryInput("class") || '';
if (this._base.mqActivation) {
clazz = this._base.mqActivation.activatedInput;
Expand All @@ -136,6 +115,6 @@ export class ClassDirective extends NgClass implements OnInit, OnChanges, OnDest
* Special adapter to cross-cut responsive behaviors
* into the ClassDirective
*/
private _base: BaseFxDirectiveAdapter;
protected _base: BaseFxDirectiveAdapter;
}

Loading

0 comments on commit eb22fe5

Please sign in to comment.