-
Notifications
You must be signed in to change notification settings - Fork 27.2k
feat(NgStyle): add new NgStyle directive #2665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import {Directive, onCheck} from 'angular2/annotations'; | ||
| import {ElementRef} from 'angular2/core'; | ||
| import {PipeRegistry} from 'angular2/src/change_detection/pipes/pipe_registry'; | ||
| import {isPresent, print} from 'angular2/src/facade/lang'; | ||
| import {Renderer} from 'angular2/src/render/api'; | ||
|
|
||
| @Directive({selector: '[ng-style]', lifecycle: [onCheck], properties: ['rawStyle: ng-style']}) | ||
| export class NgStyle { | ||
| _pipe; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. types |
||
| _rawStyle; | ||
|
|
||
| constructor(private _pipeRegistry: PipeRegistry, private _ngEl: ElementRef, | ||
| private _renderer: Renderer) {} | ||
|
|
||
| set rawStyle(v) { | ||
| this._rawStyle = v; | ||
| this._pipe = this._pipeRegistry.get('keyValDiff', this._rawStyle); | ||
| } | ||
|
|
||
| onCheck() { | ||
| var diff = this._pipe.transform(this._rawStyle); | ||
| if (isPresent(diff) && isPresent(diff.wrapped)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have any benchmarks on how this could effect large Angular applications?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we've got any specific benchmarks for the directives discussed here, but my understanding is that the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't have microbenchmarks testing transform or onCheck. But since this is how ngFor is implemented, pretty much every macrobenchmark tests it indirectly. I don't expect the performance to be a problem here. The ".onCheck" call is monomorphic. The transform call will most likely be monomorphic too.
onCheck is called every single time an object is checked. onCheck is used for a deep check when all the references stay the same. |
||
| this._applyChanges(diff.wrapped); | ||
| } | ||
| } | ||
|
|
||
| private _applyChanges(diff): void { | ||
| diff.forEachAddedItem((record) => { this._setStyle(record.key, record.currentValue); }); | ||
| diff.forEachChangedItem((record) => { this._setStyle(record.key, record.currentValue); }); | ||
| diff.forEachRemovedItem((record) => { this._setStyle(record.key, null); }); | ||
| } | ||
|
|
||
| private _setStyle(name: string, val: string): void { | ||
| this._renderer.setElementStyle(this._ngEl, name, val); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| import { | ||
| AsyncTestCompleter, | ||
| beforeEach, | ||
| beforeEachBindings, | ||
| ddescribe, | ||
| xdescribe, | ||
| describe, | ||
| el, | ||
| expect, | ||
| iit, | ||
| inject, | ||
| it, | ||
| xit, | ||
| } from 'angular2/test_lib'; | ||
|
|
||
| import {StringMapWrapper} from 'angular2/src/facade/collection'; | ||
|
|
||
| import {Component, View} from 'angular2/angular2'; | ||
|
|
||
| import {TestBed} from 'angular2/src/test_lib/test_bed'; | ||
| import {DOM} from 'angular2/src/dom/dom_adapter'; | ||
| import {NgStyle} from 'angular2/src/directives/ng_style'; | ||
|
|
||
| export function main() { | ||
| describe('binding to CSS styles', () => { | ||
|
|
||
| it('should add styles specified in an object literal', | ||
| inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => { | ||
| var template = `<div [ng-style]="{'text-align': 'right'}"></div>`; | ||
|
|
||
| tb.createView(TestComponent, {html: template}) | ||
| .then((view) => { | ||
| view.detectChanges(); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'text-align')).toEqual('right'); | ||
|
|
||
| async.done(); | ||
| }); | ||
| })); | ||
|
|
||
| it('should add and change styles specified in an object expression', | ||
| inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => { | ||
| var template = `<div [ng-style]="expr"></div>`; | ||
|
|
||
| tb.createView(TestComponent, {html: template}) | ||
| .then((view) => { | ||
| var expr: Map<string, any>; | ||
|
|
||
| view.context.expr = {'text-align': 'right'}; | ||
| view.detectChanges(); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'text-align')).toEqual('right'); | ||
|
|
||
| expr = view.context.expr; | ||
| expr['text-align'] = 'left'; | ||
| view.detectChanges(); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'text-align')).toEqual('left'); | ||
|
|
||
| async.done(); | ||
| }); | ||
| })); | ||
|
|
||
| it('should remove styles when deleting a key in an object expression', | ||
| inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => { | ||
| var template = `<div [ng-style]="expr"></div>`; | ||
|
|
||
| tb.createView(TestComponent, {html: template}) | ||
| .then((view) => { | ||
| view.context.expr = {'text-align': 'right'}; | ||
| view.detectChanges(); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'text-align')).toEqual('right'); | ||
|
|
||
| StringMapWrapper.delete(view.context.expr, 'text-align'); | ||
| view.detectChanges(); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'text-align')).toEqual(''); | ||
|
|
||
| async.done(); | ||
| }); | ||
| })); | ||
|
|
||
| it('should co-operate with the style attribute', | ||
| inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => { | ||
| var template = `<div style="font-size: 12px" [ng-style]="expr"></div>`; | ||
|
|
||
| tb.createView(TestComponent, {html: template}) | ||
| .then((view) => { | ||
| view.context.expr = {'text-align': 'right'}; | ||
| view.detectChanges(); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'text-align')).toEqual('right'); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'font-size')).toEqual('12px'); | ||
|
|
||
| StringMapWrapper.delete(view.context.expr, 'text-align'); | ||
| view.detectChanges(); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'text-align')).toEqual(''); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'font-size')).toEqual('12px'); | ||
|
|
||
| async.done(); | ||
| }); | ||
| })); | ||
|
|
||
| it('should co-operate with the style.[styleName]="expr" special-case in the compiler', | ||
| inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => { | ||
| var template = `<div [style.font-size.px]="12" [ng-style]="expr"></div>`; | ||
|
|
||
| tb.createView(TestComponent, {html: template}) | ||
| .then((view) => { | ||
| view.context.expr = {'text-align': 'right'}; | ||
| view.detectChanges(); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'text-align')).toEqual('right'); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'font-size')).toEqual('12px'); | ||
|
|
||
| StringMapWrapper.delete(view.context.expr, 'text-align'); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'font-size')).toEqual('12px'); | ||
|
|
||
| view.detectChanges(); | ||
| expect(DOM.getStyle(view.rootNodes[0], 'text-align')).toEqual(''); | ||
|
|
||
| async.done(); | ||
| }); | ||
| })); | ||
| }) | ||
| } | ||
|
|
||
| @Component({selector: 'test-cmp'}) | ||
| @View({directives: [NgStyle]}) | ||
| class TestComponent { | ||
| expr; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, good catch!