From 0a51ccbd68afa6c5ca9252fa2b9568db49833a9c Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Thu, 18 Jun 2015 15:44:44 -0700 Subject: [PATCH] =?UTF-8?q?feat(render):=20don=E2=80=99t=20use=20the=20ref?= =?UTF-8?q?lector=20for=20setting=20properties?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING CHANGES: - host actions don't take an expression as value any more but only a method name, and assumes to get an array via the EventEmitter with the method arguments. - Renderer.setElementProperty does not take `style.`/... prefixes any more. Use the new methods `Renderer.setElementAttribute`, ... instead Part of #2476 Closes #2637 --- .../src/change_detection/binding_record.ts | 74 +++++-- .../src/core/compiler/element_binder.ts | 4 +- .../src/core/compiler/element_injector.ts | 4 +- .../src/core/compiler/proto_view_factory.ts | 34 +++- modules/angular2/src/core/compiler/view.ts | 19 +- modules/angular2/src/dom/browser_adapter.dart | 24 ++- modules/angular2/src/dom/browser_adapter.ts | 7 +- modules/angular2/src/dom/dom_adapter.ts | 6 +- modules/angular2/src/dom/html_adapter.dart | 18 +- modules/angular2/src/dom/parse5_adapter.ts | 15 +- modules/angular2/src/facade/collection.ts | 4 +- modules/angular2/src/render/api.ts | 49 ++++- .../src/render/dom/compiler/compiler.ts | 5 +- .../render/dom/compiler/directive_parser.ts | 18 +- .../angular2/src/render/dom/dom_renderer.ts | 24 ++- .../src/render/dom/view/element_binder.ts | 10 +- .../dom/view/property_setter_factory.ts | 156 -------------- .../src/render/dom/view/proto_view_builder.ts | 139 ++++++++----- modules/angular2/src/render/dom/view/view.ts | 46 +++-- .../template_compiler/generator.dart | 3 +- .../change_detector_config.ts | 6 +- .../core/compiler/element_injector_spec.ts | 2 +- .../test/core/compiler/integration_spec.ts | 15 +- .../angular2/test/facade/collection_spec.ts | 17 +- .../dom/compiler/directive_parser_spec.ts | 14 +- .../dom/dom_renderer_integration_spec.ts | 67 +++--- .../dom/view/property_setter_factory_spec.ts | 190 ------------------ .../dom/view/proto_view_builder_spec.ts | 115 +++++++++++ .../test/render/dom/view/view_spec.ts | 85 +++++++- .../expected/hello.ng_deps.dart | 7 +- .../src/components/grid_list/grid_list.ts | 2 +- tools/broccoli/broccoli-merge-trees.ts | 32 +-- 32 files changed, 643 insertions(+), 568 deletions(-) delete mode 100644 modules/angular2/src/render/dom/view/property_setter_factory.ts delete mode 100644 modules/angular2/test/render/dom/view/property_setter_factory_spec.ts create mode 100644 modules/angular2/test/render/dom/view/proto_view_builder_spec.ts diff --git a/modules/angular2/src/change_detection/binding_record.ts b/modules/angular2/src/change_detection/binding_record.ts index ca67656c09..6665d45a23 100644 --- a/modules/angular2/src/change_detection/binding_record.ts +++ b/modules/angular2/src/change_detection/binding_record.ts @@ -5,13 +5,17 @@ import {DirectiveIndex, DirectiveRecord} from './directive_record'; const DIRECTIVE = "directive"; const DIRECTIVE_LIFECYCLE = "directiveLifecycle"; -const ELEMENT = "element"; +const ELEMENT_PROPERTY = "elementProperty"; +const ELEMENT_ATTRIBUTE = "elementAttribute"; +const ELEMENT_CLASS = "elementClass"; +const ELEMENT_STYLE = "elementStyle"; const TEXT_NODE = "textNode"; export class BindingRecord { constructor(public mode: string, public implicitReceiver: any, public ast: AST, - public elementIndex: number, public propertyName: string, public setter: SetterFn, - public lifecycleEvent: string, public directiveRecord: DirectiveRecord) {} + public elementIndex: number, public propertyName: string, public propertyUnit: string, + public setter: SetterFn, public lifecycleEvent: string, + public directiveRecord: DirectiveRecord) {} callOnChange(): boolean { return isPresent(this.directiveRecord) && this.directiveRecord.callOnChange; @@ -25,41 +29,85 @@ export class BindingRecord { isDirectiveLifecycle(): boolean { return this.mode === DIRECTIVE_LIFECYCLE; } - isElement(): boolean { return this.mode === ELEMENT; } + isElementProperty(): boolean { return this.mode === ELEMENT_PROPERTY; } + + isElementAttribute(): boolean { return this.mode === ELEMENT_ATTRIBUTE; } + + isElementClass(): boolean { return this.mode === ELEMENT_CLASS; } + + isElementStyle(): boolean { return this.mode === ELEMENT_STYLE; } isTextNode(): boolean { return this.mode === TEXT_NODE; } static createForDirective(ast: AST, propertyName: string, setter: SetterFn, directiveRecord: DirectiveRecord): BindingRecord { - return new BindingRecord(DIRECTIVE, 0, ast, 0, propertyName, setter, null, directiveRecord); + return new BindingRecord(DIRECTIVE, 0, ast, 0, propertyName, null, setter, null, + directiveRecord); } static createDirectiveOnCheck(directiveRecord: DirectiveRecord): BindingRecord { - return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, "onCheck", + return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, null, "onCheck", directiveRecord); } static createDirectiveOnInit(directiveRecord: DirectiveRecord): BindingRecord { - return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, "onInit", + return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, null, "onInit", directiveRecord); } static createDirectiveOnChange(directiveRecord: DirectiveRecord): BindingRecord { - return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, "onChange", + return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, null, "onChange", directiveRecord); } - static createForElement(ast: AST, elementIndex: number, propertyName: string): BindingRecord { - return new BindingRecord(ELEMENT, 0, ast, elementIndex, propertyName, null, null, null); + static createForElementProperty(ast: AST, elementIndex: number, + propertyName: string): BindingRecord { + return new BindingRecord(ELEMENT_PROPERTY, 0, ast, elementIndex, propertyName, null, null, null, + null); + } + + static createForElementAttribute(ast: AST, elementIndex: number, + attributeName: string): BindingRecord { + return new BindingRecord(ELEMENT_ATTRIBUTE, 0, ast, elementIndex, attributeName, null, null, + null, null); + } + + static createForElementClass(ast: AST, elementIndex: number, className: string): BindingRecord { + return new BindingRecord(ELEMENT_CLASS, 0, ast, elementIndex, className, null, null, null, + null); + } + + static createForElementStyle(ast: AST, elementIndex: number, styleName: string, + unit: string): BindingRecord { + return new BindingRecord(ELEMENT_STYLE, 0, ast, elementIndex, styleName, unit, null, null, + null); } static createForHostProperty(directiveIndex: DirectiveIndex, ast: AST, propertyName: string): BindingRecord { - return new BindingRecord(ELEMENT, directiveIndex, ast, directiveIndex.elementIndex, - propertyName, null, null, null); + return new BindingRecord(ELEMENT_PROPERTY, directiveIndex, ast, directiveIndex.elementIndex, + propertyName, null, null, null, null); + } + + static createForHostAttribute(directiveIndex: DirectiveIndex, ast: AST, + attributeName: string): BindingRecord { + return new BindingRecord(ELEMENT_ATTRIBUTE, directiveIndex, ast, directiveIndex.elementIndex, + attributeName, null, null, null, null); + } + + static createForHostClass(directiveIndex: DirectiveIndex, ast: AST, + className: string): BindingRecord { + return new BindingRecord(ELEMENT_CLASS, directiveIndex, ast, directiveIndex.elementIndex, + className, null, null, null, null); + } + + static createForHostStyle(directiveIndex: DirectiveIndex, ast: AST, styleName: string, + unit: string): BindingRecord { + return new BindingRecord(ELEMENT_STYLE, directiveIndex, ast, directiveIndex.elementIndex, + styleName, unit, null, null, null); } static createForTextNode(ast: AST, elementIndex: number): BindingRecord { - return new BindingRecord(TEXT_NODE, 0, ast, elementIndex, null, null, null, null); + return new BindingRecord(TEXT_NODE, 0, ast, elementIndex, null, null, null, null, null); } } diff --git a/modules/angular2/src/core/compiler/element_binder.ts b/modules/angular2/src/core/compiler/element_binder.ts index 9bd1d1edbb..ac9e3ee4a2 100644 --- a/modules/angular2/src/core/compiler/element_binder.ts +++ b/modules/angular2/src/core/compiler/element_binder.ts @@ -6,9 +6,9 @@ import {List, StringMap} from 'angular2/src/facade/collection'; import * as viewModule from './view'; export class ElementBinder { - // updated later when events are bound - nestedProtoView: viewModule.AppProtoView = null; // updated later, so we are able to resolve cycles + nestedProtoView: viewModule.AppProtoView = null; + // updated later when events are bound hostListeners: StringMap> = null; constructor(public index: int, public parent: ElementBinder, public distanceToParent: int, diff --git a/modules/angular2/src/core/compiler/element_injector.ts b/modules/angular2/src/core/compiler/element_injector.ts index 968320cb6e..3c6a90220c 100644 --- a/modules/angular2/src/core/compiler/element_injector.ts +++ b/modules/angular2/src/core/compiler/element_injector.ts @@ -315,13 +315,13 @@ export class EventEmitterAccessor { } export class HostActionAccessor { - constructor(public actionExpression: string, public getter: Function) {} + constructor(public methodName: string, public getter: Function) {} subscribe(view: viewModule.AppView, boundElementIndex: number, directive: Object) { var eventEmitter = this.getter(directive); return ObservableWrapper.subscribe( eventEmitter, - actionObj => view.callAction(boundElementIndex, this.actionExpression, actionObj)); + actionArgs => view.invokeElementMethod(boundElementIndex, this.methodName, actionArgs)); } } diff --git a/modules/angular2/src/core/compiler/proto_view_factory.ts b/modules/angular2/src/core/compiler/proto_view_factory.ts index fb0ed81d64..d67f73c2b8 100644 --- a/modules/angular2/src/core/compiler/proto_view_factory.ts +++ b/modules/angular2/src/core/compiler/proto_view_factory.ts @@ -66,9 +66,20 @@ class BindingRecordsCreator { _createElementPropertyRecords(bindings: List, boundElementIndex: number, renderElementBinder: renderApi.ElementBinder) { - MapWrapper.forEach(renderElementBinder.propertyBindings, (astWithSource, propertyName) => { - - bindings.push(BindingRecord.createForElement(astWithSource, boundElementIndex, propertyName)); + ListWrapper.forEach(renderElementBinder.propertyBindings, (binding) => { + if (binding.type === renderApi.PropertyBindingType.PROPERTY) { + bindings.push(BindingRecord.createForElementProperty(binding.astWithSource, + boundElementIndex, binding.property)); + } else if (binding.type === renderApi.PropertyBindingType.ATTRIBUTE) { + bindings.push(BindingRecord.createForElementAttribute(binding.astWithSource, + boundElementIndex, binding.property)); + } else if (binding.type === renderApi.PropertyBindingType.CLASS) { + bindings.push(BindingRecord.createForElementClass(binding.astWithSource, boundElementIndex, + binding.property)); + } else if (binding.type === renderApi.PropertyBindingType.STYLE) { + bindings.push(BindingRecord.createForElementStyle(binding.astWithSource, boundElementIndex, + binding.property, binding.unit)); + } }); } @@ -103,10 +114,21 @@ class BindingRecordsCreator { for (var i = 0; i < directiveBinders.length; i++) { var directiveBinder = directiveBinders[i]; // host properties - MapWrapper.forEach(directiveBinder.hostPropertyBindings, (astWithSource, propertyName) => { + ListWrapper.forEach(directiveBinder.hostPropertyBindings, (binding) => { var dirIndex = new DirectiveIndex(boundElementIndex, i); - - bindings.push(BindingRecord.createForHostProperty(dirIndex, astWithSource, propertyName)); + if (binding.type === renderApi.PropertyBindingType.PROPERTY) { + bindings.push(BindingRecord.createForHostProperty(dirIndex, binding.astWithSource, + binding.property)); + } else if (binding.type === renderApi.PropertyBindingType.ATTRIBUTE) { + bindings.push(BindingRecord.createForHostAttribute(dirIndex, binding.astWithSource, + binding.property)); + } else if (binding.type === renderApi.PropertyBindingType.CLASS) { + bindings.push( + BindingRecord.createForHostClass(dirIndex, binding.astWithSource, binding.property)); + } else if (binding.type === renderApi.PropertyBindingType.STYLE) { + bindings.push(BindingRecord.createForHostStyle(dirIndex, binding.astWithSource, + binding.property, binding.unit)); + } }); } } diff --git a/modules/angular2/src/core/compiler/view.ts b/modules/angular2/src/core/compiler/view.ts index 82413b7fa6..b6579a293e 100644 --- a/modules/angular2/src/core/compiler/view.ts +++ b/modules/angular2/src/core/compiler/view.ts @@ -99,11 +99,20 @@ export class AppView implements ChangeDispatcher, EventDispatcher { // dispatch to element injector or text nodes based on context notifyOnBinding(b: BindingRecord, currentValue: any): void { - if (b.isElement()) { + if (b.isElementProperty()) { this.renderer.setElementProperty(this.render, b.elementIndex, b.propertyName, currentValue); - } else { - // we know it refers to _textNodes. + } else if (b.isElementAttribute()) { + this.renderer.setElementAttribute(this.render, b.elementIndex, b.propertyName, currentValue); + } else if (b.isElementClass()) { + this.renderer.setElementClass(this.render, b.elementIndex, b.propertyName, currentValue); + } else if (b.isElementStyle()) { + var unit = isPresent(b.propertyUnit) ? b.propertyUnit : ''; + this.renderer.setElementStyle(this.render, b.elementIndex, b.propertyName, + `${currentValue}${unit}`); + } else if (b.isTextNode()) { this.renderer.setText(this.render, b.elementIndex, currentValue); + } else { + throw new BaseException('Unsupported directive record'); } } @@ -124,8 +133,8 @@ export class AppView implements ChangeDispatcher, EventDispatcher { return isPresent(childView) ? childView.changeDetector : null; } - callAction(elementIndex: number, actionExpression: string, action: Object) { - this.renderer.callAction(this.render, elementIndex, actionExpression, action); + invokeElementMethod(elementIndex: number, methodName: string, args: List) { + this.renderer.invokeElementMethod(this.render, elementIndex, methodName, args); } // implementation of EventDispatcher#dispatchEvent diff --git a/modules/angular2/src/dom/browser_adapter.dart b/modules/angular2/src/dom/browser_adapter.dart index fc9ea93785..49293a2be3 100644 --- a/modules/angular2/src/dom/browser_adapter.dart +++ b/modules/angular2/src/dom/browser_adapter.dart @@ -1,7 +1,6 @@ library angular.core.facade.dom; import 'dart:html'; -import 'dart:js' show JsObject; import 'dom_adapter.dart' show setRootDomAdapter; import 'generic_browser_adapter.dart' show GenericBrowserDomAdapter; import '../facade/browser.dart'; @@ -97,9 +96,28 @@ final _keyCodeToKeyMap = const { }; class BrowserDomAdapter extends GenericBrowserDomAdapter { + js.JsFunction _setProperty; + js.JsFunction _getProperty; + js.JsFunction _hasProperty; + BrowserDomAdapter() { + _setProperty = js.context.callMethod('eval', ['(function(el, prop, value) { el[prop] = value; })']); + _getProperty = js.context.callMethod('eval', ['(function(el, prop) { return el[prop]; })']); + _hasProperty = js.context.callMethod('eval', ['(function(el, prop) { return prop in el; })']); + } static void makeCurrent() { setRootDomAdapter(new BrowserDomAdapter()); } + bool hasProperty(Element element, String name) => + _hasProperty.apply([element, name]); + + void setProperty(Element element, String name, Object value) => + _setProperty.apply([element, name, value]); + + getProperty(Element element, String name) => + _getProperty.apply([element, name]); + + invoke(Element element, String methodName, List args) => + this.getProperty(element, methodName).apply(args, thisArg: element); // TODO(tbosch): move this into a separate environment class once we have it logError(error) { @@ -108,7 +126,7 @@ class BrowserDomAdapter extends GenericBrowserDomAdapter { @override Map get attrToPropMap => const { - 'innerHtml': 'innerHtml', + 'innerHtml': 'innerHTML', 'readonly': 'readOnly', 'tabindex': 'tabIndex', }; @@ -221,8 +239,6 @@ class BrowserDomAdapter extends GenericBrowserDomAdapter { ShadowRoot getShadowRoot(Element el) => el.shadowRoot; Element getHost(Element el) => (el as ShadowRoot).host; clone(Node node) => node.clone(true); - bool hasProperty(Element element, String name) => - new JsObject.fromBrowserObject(element).hasProperty(name); List getElementsByClassName(Element element, String name) => element.getElementsByClassName(name); List getElementsByTagName(Element element, String name) => diff --git a/modules/angular2/src/dom/browser_adapter.ts b/modules/angular2/src/dom/browser_adapter.ts index e055873aa1..ee0b7ccc29 100644 --- a/modules/angular2/src/dom/browser_adapter.ts +++ b/modules/angular2/src/dom/browser_adapter.ts @@ -50,6 +50,12 @@ var _chromeNumKeyPadMap = { export class BrowserDomAdapter extends GenericBrowserDomAdapter { static makeCurrent() { setRootDomAdapter(new BrowserDomAdapter()); } + hasProperty(element, name: string) { return name in element; } + setProperty(el: /*element*/ any, name: string, value: any) { el[name] = value; } + getProperty(el: /*element*/ any, name: string): any { return el[name]; } + invoke(el: /*element*/ any, methodName: string, args: List): any { + el[methodName].apply(el, args); + } // TODO(tbosch): move this into a separate environment class once we have it logError(error) { window.console.error(error); } @@ -152,7 +158,6 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter { getShadowRoot(el: HTMLElement): DocumentFragment { return (el).shadowRoot; } getHost(el: HTMLElement): HTMLElement { return (el).host; } clone(node: Node) { return node.cloneNode(true); } - hasProperty(element, name: string) { return name in element; } getElementsByClassName(element, name: string) { return element.getElementsByClassName(name); } getElementsByTagName(element, name: string) { return element.getElementsByTagName(name); } classList(element): List { diff --git a/modules/angular2/src/dom/dom_adapter.ts b/modules/angular2/src/dom/dom_adapter.ts index ab2e2e63d6..74804d818c 100644 --- a/modules/angular2/src/dom/dom_adapter.ts +++ b/modules/angular2/src/dom/dom_adapter.ts @@ -16,6 +16,11 @@ function _abstract() { * Provides DOM operations in an environment-agnostic way. */ export class DomAdapter { + hasProperty(element, name: string): boolean { throw _abstract(); } + setProperty(el: /*element*/ any, name: string, value: any) { throw _abstract(); } + getProperty(el: /*element*/ any, name: string): any { throw _abstract(); } + invoke(el: /*element*/ any, methodName: string, args: List): any { throw _abstract(); } + logError(error) { throw _abstract(); } /** @@ -70,7 +75,6 @@ export class DomAdapter { getHost(el): any { throw _abstract(); } getDistributedNodes(el): List { throw _abstract(); } clone(node): any { throw _abstract(); } - hasProperty(element, name: string): boolean { throw _abstract(); } getElementsByClassName(element, name: string): List { throw _abstract(); } getElementsByTagName(element, name: string): List { throw _abstract(); } classList(element): List { throw _abstract(); } diff --git a/modules/angular2/src/dom/html_adapter.dart b/modules/angular2/src/dom/html_adapter.dart index b41ddc4156..049dd46f70 100644 --- a/modules/angular2/src/dom/html_adapter.dart +++ b/modules/angular2/src/dom/html_adapter.dart @@ -10,13 +10,24 @@ class Html5LibDomAdapter implements DomAdapter { setRootDomAdapter(new Html5LibDomAdapter()); } + hasProperty(element, String name) { + // This is needed for serverside compile to generate the right getters/setters... + return true; + } + + void setProperty(Element element, String name, Object value) => throw 'not implemented'; + + getProperty(Element element, String name) => throw 'not implemented'; + + invoke(Element element, String methodName, List args) => throw 'not implemented'; + logError(error) { stderr.writeln('${error}'); } @override final attrToPropMap = const { - 'innerHtml': 'innerHtml', + 'innerHtml': 'innerHTML', 'readonly': 'readOnly', 'tabindex': 'tabIndex', }; @@ -184,11 +195,6 @@ class Html5LibDomAdapter implements DomAdapter { throw 'not implemented'; } clone(node) => node.clone(true); - - hasProperty(element, String name) { - // This is needed for serverside compile to generate the right getters/setters... - return true; - } getElementsByClassName(element, String name) { throw 'not implemented'; } diff --git a/modules/angular2/src/dom/parse5_adapter.ts b/modules/angular2/src/dom/parse5_adapter.ts index 936f60f59a..125381836a 100644 --- a/modules/angular2/src/dom/parse5_adapter.ts +++ b/modules/angular2/src/dom/parse5_adapter.ts @@ -26,6 +26,20 @@ function _notImplemented(methodName) { export class Parse5DomAdapter extends DomAdapter { static makeCurrent() { setRootDomAdapter(new Parse5DomAdapter()); } + hasProperty(element, name: string) { return _HTMLElementPropertyList.indexOf(name) > -1; } + // TODO(tbosch): don't even call this method when we run the tests on server side + // by not using the DomRenderer in tests. Keeping this for now to make tests happy... + setProperty(el: /*element*/ any, name: string, value: any) { + if (name === 'innerHTML') { + this.setInnerHTML(el, value); + } else { + el[name] = value; + } + } + // TODO(tbosch): don't even call this method when we run the tests on server side + // by not using the DomRenderer in tests. Keeping this for now to make tests happy... + getProperty(el: /*element*/ any, name: string): any { return el[name]; } + logError(error) { console.error(error); } get attrToPropMap() { return _attrToPropMap; } @@ -268,7 +282,6 @@ export class Parse5DomAdapter extends DomAdapter { return newParser.parseFragment(serialized).childNodes[0]; } } - hasProperty(element, name: string) { return _HTMLElementPropertyList.indexOf(name) > -1; } getElementsByClassName(element, name: string) { return this.querySelectorAll(element, "." + name); } diff --git a/modules/angular2/src/facade/collection.ts b/modules/angular2/src/facade/collection.ts index 3f6ba5de0a..175083d788 100644 --- a/modules/angular2/src/facade/collection.ts +++ b/modules/angular2/src/facade/collection.ts @@ -69,8 +69,8 @@ export class MapWrapper { static delete(m: Map, k: K) { m.delete(k); } static clearValues(m: Map) { _clearValues(m); } static iterable(m) { return m; } - static keys(m: Map): List { return m.keys(); } - static values(m: Map): List { return m.values(); } + static keys(m: Map): List { return (Array).from(m.keys()); } + static values(m: Map): List { return (Array).from(m.values()); } } /** diff --git a/modules/angular2/src/render/api.ts b/modules/angular2/src/render/api.ts index 3230d5dd8d..7048777d89 100644 --- a/modules/angular2/src/render/api.ts +++ b/modules/angular2/src/render/api.ts @@ -19,17 +19,30 @@ import {ASTWithSource} from 'angular2/change_detection'; * - render compiler is not on the critical path as * its output will be stored in precompiled templates. */ + export class EventBinding { constructor(public fullName: string, public source: ASTWithSource) {} } +export enum PropertyBindingType { + PROPERTY, + ATTRIBUTE, + CLASS, + STYLE +} + +export class ElementPropertyBinding { + constructor(public type: PropertyBindingType, public astWithSource: ASTWithSource, + public property: string, public unit: string = null) {} +} + export class ElementBinder { index: number; parentIndex: number; distanceToParent: number; directives: List; nestedProtoView: ProtoViewDto; - propertyBindings: Map; + propertyBindings: List; variableBindings: Map; // Note: this contains a preprocessed AST // that replaced the values that should be extracted from the element @@ -45,7 +58,7 @@ export class ElementBinder { distanceToParent?: number, directives?: List, nestedProtoView?: ProtoViewDto, - propertyBindings?: Map, + propertyBindings?: List, variableBindings?: Map, eventBindings?: List, textBindings?: List, @@ -72,12 +85,12 @@ export class DirectiveBinder { // that replaced the values that should be extracted from the element // with a local name eventBindings: List; - hostPropertyBindings: Map; + hostPropertyBindings: List; constructor({directiveIndex, propertyBindings, eventBindings, hostPropertyBindings}: { directiveIndex?: number, propertyBindings?: Map, eventBindings?: List, - hostPropertyBindings?: Map + hostPropertyBindings?: List }) { this.directiveIndex = directiveIndex; this.propertyBindings = propertyBindings; @@ -358,19 +371,33 @@ export class Renderer { /** * Sets a property on an element. - * Note: This will fail if the property was not mentioned previously as a host property - * in the ProtoView */ setElementProperty(viewRef: RenderViewRef, elementIndex: number, propertyName: string, propertyValue: any) {} /** - * Calls an action. - * Note: This will fail if the action was not mentioned previously as a host action - * in the ProtoView + * Sets an attribute on an element. */ - callAction(viewRef: RenderViewRef, elementIndex: number, actionExpression: string, - actionArgs: any) {} + setElementAttribute(viewRef: RenderViewRef, elementIndex: number, attributeName: string, + attributeValue: string) {} + + /** + * Sets a class on an element. + */ + setElementClass(viewRef: RenderViewRef, elementIndex: number, className: string, isAdd: boolean) { + } + + /** + * Sets a style on an element. + */ + setElementStyle(viewRef: RenderViewRef, elementIndex: number, styleName: string, + styleValue: string) {} + + /** + * Calls a method on an element. + */ + invokeElementMethod(viewRef: RenderViewRef, elementIndex: number, methodName: string, + args: List) {} /** * Sets the value of a text node. diff --git a/modules/angular2/src/render/dom/compiler/compiler.ts b/modules/angular2/src/render/dom/compiler/compiler.ts index ef346d6c9c..2a42fdec10 100644 --- a/modules/angular2/src/render/dom/compiler/compiler.ts +++ b/modules/angular2/src/render/dom/compiler/compiler.ts @@ -17,7 +17,6 @@ import {TemplateLoader} from 'angular2/src/render/dom/compiler/template_loader'; import {CompileStepFactory, DefaultStepFactory} from './compile_step_factory'; import {Parser} from 'angular2/change_detection'; import {ShadowDomStrategy} from '../shadow_dom/shadow_dom_strategy'; -import {PropertySetterFactory} from '../view/property_setter_factory'; /** * The compiler loads and translates the html templates of components into @@ -25,8 +24,6 @@ import {PropertySetterFactory} from '../view/property_setter_factory'; * the CompilePipeline and the CompileSteps. */ export class DomCompiler extends RenderCompiler { - _propertySetterFactory: PropertySetterFactory = new PropertySetterFactory(); - constructor(public _stepFactory: CompileStepFactory, public _templateLoader: TemplateLoader) { super(); } @@ -58,7 +55,7 @@ export class DomCompiler extends RenderCompiler { var pipeline = new CompilePipeline(this._stepFactory.createSteps(viewDef, subTaskPromises)); var compileElements = pipeline.process(tplElement, protoViewType, viewDef.componentId); - var protoView = compileElements[0].inheritedProtoView.build(this._propertySetterFactory); + var protoView = compileElements[0].inheritedProtoView.build(); if (subTaskPromises.length > 0) { return PromiseWrapper.all(subTaskPromises).then((_) => protoView); diff --git a/modules/angular2/src/render/dom/compiler/directive_parser.ts b/modules/angular2/src/render/dom/compiler/directive_parser.ts index 89faf4d4ca..0371e43f26 100644 --- a/modules/angular2/src/render/dom/compiler/directive_parser.ts +++ b/modules/angular2/src/render/dom/compiler/directive_parser.ts @@ -91,11 +91,6 @@ export class DirectiveParser implements CompileStep { this._bindDirectiveEvent(eventName, action, current, directiveBinderBuilder); }); } - if (isPresent(dirMetadata.hostActions)) { - MapWrapper.forEach(dirMetadata.hostActions, (action, actionName) => { - this._bindHostAction(actionName, action, current, directiveBinderBuilder); - }); - } if (isPresent(dirMetadata.hostProperties)) { MapWrapper.forEach(dirMetadata.hostProperties, (expression, hostPropertyName) => { this._bindHostProperty(hostPropertyName, expression, current, directiveBinderBuilder); @@ -134,9 +129,8 @@ export class DirectiveParser implements CompileStep { elProp = bindConfig; pipes = []; } - - var bindingAst = compileElement.bindElement().propertyBindings.get(dashCaseToCamelCase(elProp)); - + elProp = dashCaseToCamelCase(elProp); + var bindingAst = compileElement.bindElement().propertyBindings.get(elProp); if (isBlank(bindingAst)) { var attributeValue = compileElement.attrs().get(camelCaseToDashCase(elProp)); if (isPresent(attributeValue)) { @@ -147,9 +141,8 @@ export class DirectiveParser implements CompileStep { // Bindings are optional, so this binding only needs to be set up if an expression is given. if (isPresent(bindingAst)) { - directiveBinderBuilder.bindProperty(dirProperty, bindingAst); + directiveBinderBuilder.bindProperty(dirProperty, bindingAst, elProp); } - compileElement.bindElement().bindPropertyToDirective(dashCaseToCamelCase(elProp)); } _bindDirectiveEvent(eventName, action, compileElement, directiveBinderBuilder) { @@ -162,11 +155,6 @@ export class DirectiveParser implements CompileStep { } } - _bindHostAction(actionName, actionExpression, compileElement, directiveBinderBuilder) { - var ast = this._parser.parseAction(actionExpression, compileElement.elementDescription); - directiveBinderBuilder.bindHostAction(actionName, actionExpression, ast); - } - _bindHostProperty(hostPropertyName, expression, compileElement, directiveBinderBuilder) { var ast = this._parser.parseSimpleBinding( expression, `hostProperties of ${compileElement.elementDescription}`); diff --git a/modules/angular2/src/render/dom/dom_renderer.ts b/modules/angular2/src/render/dom/dom_renderer.ts index 0d57bec527..67a8582ca7 100644 --- a/modules/angular2/src/render/dom/dom_renderer.ts +++ b/modules/angular2/src/render/dom/dom_renderer.ts @@ -187,10 +187,28 @@ export class DomRenderer extends Renderer { view.setElementProperty(elementIndex, propertyName, propertyValue); } - callAction(viewRef: RenderViewRef, elementIndex: number, actionExpression: string, - actionArgs: any): void { + setElementAttribute(viewRef: RenderViewRef, elementIndex: number, attributeName: string, + attributeValue: string): void { var view = resolveInternalDomView(viewRef); - view.callAction(elementIndex, actionExpression, actionArgs); + view.setElementAttribute(elementIndex, attributeName, attributeValue); + } + + setElementClass(viewRef: RenderViewRef, elementIndex: number, className: string, + isAdd: boolean): void { + var view = resolveInternalDomView(viewRef); + view.setElementClass(elementIndex, className, isAdd); + } + + setElementStyle(viewRef: RenderViewRef, elementIndex: number, styleName: string, + styleValue: string): void { + var view = resolveInternalDomView(viewRef); + view.setElementStyle(elementIndex, styleName, styleValue); + } + + invokeElementMethod(viewRef: RenderViewRef, elementIndex: number, methodName: string, + args: List): void { + var view = resolveInternalDomView(viewRef); + view.invokeElementMethod(elementIndex, methodName, args); } setText(viewRef: RenderViewRef, textNodeIndex: number, text: string): void { diff --git a/modules/angular2/src/render/dom/view/element_binder.ts b/modules/angular2/src/render/dom/view/element_binder.ts index c14faec03a..ba0df970fe 100644 --- a/modules/angular2/src/render/dom/view/element_binder.ts +++ b/modules/angular2/src/render/dom/view/element_binder.ts @@ -1,5 +1,4 @@ import {AST} from 'angular2/change_detection'; -import {SetterFn} from 'angular2/src/reflection/types'; import {List, ListWrapper} from 'angular2/src/facade/collection'; import * as protoViewModule from './proto_view'; @@ -13,13 +12,10 @@ export class ElementBinder { componentId: string; parentIndex: number; distanceToParent: number; - propertySetters: Map; - hostActions: Map; elementIsEmpty: boolean; constructor({textNodeIndices, contentTagSelector, nestedProtoView, componentId, eventLocals, - localEvents, globalEvents, hostActions, parentIndex, distanceToParent, - propertySetters, elementIsEmpty}: { + localEvents, globalEvents, parentIndex, distanceToParent, elementIsEmpty}: { contentTagSelector?: string, textNodeIndices?: List, nestedProtoView?: protoViewModule.DomProtoView, @@ -29,8 +25,6 @@ export class ElementBinder { componentId?: string, parentIndex?: number, distanceToParent?: number, - propertySetters?: Map, - hostActions?: Map, elementIsEmpty?: boolean } = {}) { this.textNodeIndices = textNodeIndices; @@ -40,10 +34,8 @@ export class ElementBinder { this.eventLocals = eventLocals; this.localEvents = localEvents; this.globalEvents = globalEvents; - this.hostActions = hostActions; this.parentIndex = parentIndex; this.distanceToParent = distanceToParent; - this.propertySetters = propertySetters; this.elementIsEmpty = elementIsEmpty; } } diff --git a/modules/angular2/src/render/dom/view/property_setter_factory.ts b/modules/angular2/src/render/dom/view/property_setter_factory.ts deleted file mode 100644 index 403d897904..0000000000 --- a/modules/angular2/src/render/dom/view/property_setter_factory.ts +++ /dev/null @@ -1,156 +0,0 @@ -import { - StringWrapper, - RegExpWrapper, - BaseException, - isPresent, - isBlank, - isString, - stringify -} from 'angular2/src/facade/lang'; -import {ListWrapper, StringMapWrapper} from 'angular2/src/facade/collection'; -import {DOM} from 'angular2/src/dom/dom_adapter'; -import {camelCaseToDashCase, dashCaseToCamelCase} from '../util'; -import {reflector} from 'angular2/src/reflection/reflection'; - -const STYLE_SEPARATOR = '.'; -const ATTRIBUTE_PREFIX = 'attr.'; -const CLASS_PREFIX = 'class.'; -const STYLE_PREFIX = 'style.'; - -export class PropertySetterFactory { - static noopSetter(el, value) {} - - private _lazyPropertySettersCache: StringMap = StringMapWrapper.create(); - private _eagerPropertySettersCache: StringMap = StringMapWrapper.create(); - private _innerHTMLSetterCache: Function = (el, value) => DOM.setInnerHTML(el, value); - private _attributeSettersCache: StringMap = StringMapWrapper.create(); - private _classSettersCache: StringMap = StringMapWrapper.create(); - private _styleSettersCache: StringMap = StringMapWrapper.create(); - - createSetter(protoElement: /*element*/ any, isNgComponent: boolean, property: string): Function { - var setterFn, styleParts, styleSuffix; - if (StringWrapper.startsWith(property, ATTRIBUTE_PREFIX)) { - setterFn = - this._attributeSetterFactory(StringWrapper.substring(property, ATTRIBUTE_PREFIX.length)); - } else if (StringWrapper.startsWith(property, CLASS_PREFIX)) { - setterFn = this._classSetterFactory(StringWrapper.substring(property, CLASS_PREFIX.length)); - } else if (StringWrapper.startsWith(property, STYLE_PREFIX)) { - styleParts = property.split(STYLE_SEPARATOR); - styleSuffix = styleParts.length > 2 ? ListWrapper.get(styleParts, 2) : ''; - setterFn = this._styleSetterFactory(ListWrapper.get(styleParts, 1), styleSuffix); - } else if (StringWrapper.equals(property, 'innerHtml')) { - setterFn = this._innerHTMLSetterCache; - } else { - property = this._resolvePropertyName(property); - setterFn = this._propertySetterFactory(protoElement, isNgComponent, property); - } - return setterFn; - } - - private _propertySetterFactory(protoElement, isNgComponent: boolean, property: string): Function { - var setterFn; - var tagName = DOM.tagName(protoElement); - var possibleCustomElement = tagName.indexOf('-') !== -1; - if (possibleCustomElement && !isNgComponent) { - // need to use late check to be able to set properties on custom elements - setterFn = StringMapWrapper.get(this._lazyPropertySettersCache, property); - if (isBlank(setterFn)) { - var propertySetterFn = reflector.setter(property); - setterFn = (receiver, value) => { - if (DOM.hasProperty(receiver, property)) { - return propertySetterFn(receiver, value); - } - }; - StringMapWrapper.set(this._lazyPropertySettersCache, property, setterFn); - } - } else { - setterFn = StringMapWrapper.get(this._eagerPropertySettersCache, property); - if (isBlank(setterFn)) { - if (DOM.hasProperty(protoElement, property)) { - setterFn = reflector.setter(property); - } else { - setterFn = PropertySetterFactory.noopSetter; - } - StringMapWrapper.set(this._eagerPropertySettersCache, property, setterFn); - } - } - return setterFn; - } - - private _isValidAttributeValue(attrName: string, value: any): boolean { - if (attrName == "role") { - return isString(value); - } else { - return isPresent(value); - } - } - - private _attributeSetterFactory(attrName: string): Function { - var setterFn = StringMapWrapper.get(this._attributeSettersCache, attrName); - var dashCasedAttributeName; - - if (isBlank(setterFn)) { - dashCasedAttributeName = camelCaseToDashCase(attrName); - setterFn = (element, value) => { - if (this._isValidAttributeValue(dashCasedAttributeName, value)) { - DOM.setAttribute(element, dashCasedAttributeName, stringify(value)); - } else { - if (isPresent(value)) { - throw new BaseException("Invalid " + dashCasedAttributeName + - " attribute, only string values are allowed, got '" + - stringify(value) + "'"); - } - DOM.removeAttribute(element, dashCasedAttributeName); - } - }; - StringMapWrapper.set(this._attributeSettersCache, attrName, setterFn); - } - - return setterFn; - } - - private _classSetterFactory(className: string): Function { - var setterFn = StringMapWrapper.get(this._classSettersCache, className); - var dashCasedClassName; - if (isBlank(setterFn)) { - dashCasedClassName = camelCaseToDashCase(className); - setterFn = (element, isAdd) => { - if (isAdd) { - DOM.addClass(element, dashCasedClassName); - } else { - DOM.removeClass(element, dashCasedClassName); - } - }; - StringMapWrapper.set(this._classSettersCache, className, setterFn); - } - - return setterFn; - } - - private _styleSetterFactory(styleName: string, styleSuffix: string): Function { - var cacheKey = styleName + styleSuffix; - var setterFn = StringMapWrapper.get(this._styleSettersCache, cacheKey); - var dashCasedStyleName; - - if (isBlank(setterFn)) { - dashCasedStyleName = camelCaseToDashCase(styleName); - setterFn = (element, value) => { - var valAsStr; - if (isPresent(value)) { - valAsStr = stringify(value); - DOM.setStyle(element, dashCasedStyleName, valAsStr + styleSuffix); - } else { - DOM.removeStyle(element, dashCasedStyleName); - } - }; - StringMapWrapper.set(this._styleSettersCache, cacheKey, setterFn); - } - - return setterFn; - } - - private _resolvePropertyName(attrName: string): string { - var mappedPropName = StringMapWrapper.get(DOM.attrToPropMap, attrName); - return isPresent(mappedPropName) ? mappedPropName : attrName; - } -} diff --git a/modules/angular2/src/render/dom/view/proto_view_builder.ts b/modules/angular2/src/render/dom/view/proto_view_builder.ts index 06a8ff97ae..b5b6b4409f 100644 --- a/modules/angular2/src/render/dom/view/proto_view_builder.ts +++ b/modules/angular2/src/render/dom/view/proto_view_builder.ts @@ -1,5 +1,12 @@ -import {isPresent, isBlank, BaseException} from 'angular2/src/facade/lang'; -import {ListWrapper, MapWrapper, Set, SetWrapper, List} from 'angular2/src/facade/collection'; +import {isPresent, isBlank, BaseException, StringWrapper} from 'angular2/src/facade/lang'; +import { + ListWrapper, + MapWrapper, + Set, + SetWrapper, + List, + StringMapWrapper +} from 'angular2/src/facade/collection'; import {DOM} from 'angular2/src/dom/dom_adapter'; import { @@ -13,7 +20,6 @@ import { import {DomProtoView, DomProtoViewRef, resolveInternalDomProtoView} from './proto_view'; import {ElementBinder, Event, HostAction} from './element_binder'; -import {PropertySetterFactory} from './property_setter_factory'; import * as api from '../../api'; @@ -43,53 +49,28 @@ export class ProtoViewBuilder { this.variableBindings.set(value, name); } - build(setterFactory: PropertySetterFactory): api.ProtoViewDto { + build(): api.ProtoViewDto { var renderElementBinders = []; var apiElementBinders = []; var transitiveContentTagCount = 0; var boundTextNodeCount = 0; ListWrapper.forEach(this.elements, (ebb: ElementBinderBuilder) => { - var propertySetters = new Map(); - var hostActions = new Map(); - + var directiveTemplatePropertyNames = new Set(); var apiDirectiveBinders = ListWrapper.map(ebb.directives, (dbb: DirectiveBuilder) => { ebb.eventBuilder.merge(dbb.eventBuilder); - - MapWrapper.forEach(dbb.hostPropertyBindings, (_, hostPropertyName) => { - propertySetters.set(hostPropertyName, - setterFactory.createSetter(ebb.element, isPresent(ebb.componentId), - hostPropertyName)); - }); - - ListWrapper.forEach(dbb.hostActions, (hostAction) => { - hostActions.set(hostAction.actionExpression, hostAction.expression); - }); - + ListWrapper.forEach(dbb.templatePropertyNames, + (name) => directiveTemplatePropertyNames.add(name)); return new api.DirectiveBinder({ directiveIndex: dbb.directiveIndex, propertyBindings: dbb.propertyBindings, eventBindings: dbb.eventBindings, - hostPropertyBindings: dbb.hostPropertyBindings + hostPropertyBindings: + buildElementPropertyBindings(ebb.element, isPresent(ebb.componentId), + dbb.hostPropertyBindings, directiveTemplatePropertyNames) }); }); - - MapWrapper.forEach(ebb.propertyBindings, (_, propertyName) => { - var propSetter = - setterFactory.createSetter(ebb.element, isPresent(ebb.componentId), propertyName); - - if (propSetter === PropertySetterFactory.noopSetter) { - if (!SetWrapper.has(ebb.propertyBindingsToDirectives, propertyName)) { - throw new BaseException( - `Can't bind to '${propertyName}' since it isn't a know property of the '${DOM.tagName(ebb.element).toLowerCase()}' element and there are no matching directives with a corresponding property`); - } - } - - propertySetters.set(propertyName, propSetter); - }); - - var nestedProtoView = - isPresent(ebb.nestedProtoView) ? ebb.nestedProtoView.build(setterFactory) : null; + var nestedProtoView = isPresent(ebb.nestedProtoView) ? ebb.nestedProtoView.build() : null; var nestedRenderProtoView = isPresent(nestedProtoView) ? resolveInternalDomProtoView(nestedProtoView.render) : null; if (isPresent(nestedRenderProtoView)) { @@ -105,7 +86,9 @@ export class ProtoViewBuilder { distanceToParent: ebb.distanceToParent, directives: apiDirectiveBinders, nestedProtoView: nestedProtoView, - propertyBindings: ebb.propertyBindings, + propertyBindings: + buildElementPropertyBindings(ebb.element, isPresent(ebb.componentId), + ebb.propertyBindings, directiveTemplatePropertyNames), variableBindings: ebb.variableBindings, eventBindings: ebb.eventBindings, textBindings: ebb.textBindings, @@ -124,8 +107,6 @@ export class ProtoViewBuilder { eventLocals: new LiteralArray(ebb.eventBuilder.buildEventLocals()), localEvents: ebb.eventBuilder.buildLocalEvents(), globalEvents: ebb.eventBuilder.buildGlobalEvents(), - hostActions: hostActions, - propertySetters: propertySetters, elementIsEmpty: childNodeInfo.elementIsEmpty })); }); @@ -216,7 +197,9 @@ export class ElementBinderBuilder { return this.nestedProtoView; } - bindProperty(name, expression) { this.propertyBindings.set(name, expression); } + bindProperty(name: string, expression: ASTWithSource) { + this.propertyBindings.set(name, expression); + } bindPropertyToDirective(name: string) { // we are filling in a set of property names that are bound to a property @@ -257,20 +240,27 @@ export class ElementBinderBuilder { } export class DirectiveBuilder { + // mapping from directive property name to AST for that directive propertyBindings: Map = new Map(); + // property names used in the template + templatePropertyNames: List = []; hostPropertyBindings: Map = new Map(); - hostActions: List = []; eventBindings: List = []; eventBuilder: EventBuilder = new EventBuilder(); constructor(public directiveIndex: number) {} - bindProperty(name, expression) { this.propertyBindings.set(name, expression); } + bindProperty(name: string, expression: ASTWithSource, elProp: string) { + this.propertyBindings.set(name, expression); + if (isPresent(elProp)) { + // we are filling in a set of property names that are bound to a property + // of at least one directive. This allows us to report "dangling" bindings. + this.templatePropertyNames.push(elProp); + } + } - bindHostProperty(name, expression) { this.hostPropertyBindings.set(name, expression); } - - bindHostAction(actionName: string, actionExpression: string, expression: ASTWithSource) { - this.hostActions.push(new HostAction(actionName, actionExpression, expression)); + bindHostProperty(name: string, expression: ASTWithSource) { + this.hostPropertyBindings.set(name, expression); } bindEvent(name, expression, target = null) { @@ -347,3 +337,60 @@ export class EventBuilder extends AstTransformer { } } } + +var PROPERTY_PARTS_SEPARATOR = new RegExp('\\.'); +const ATTRIBUTE_PREFIX = 'attr'; +const CLASS_PREFIX = 'class'; +const STYLE_PREFIX = 'style'; + +function buildElementPropertyBindings(protoElement: /*element*/ any, isNgComponent: boolean, + bindingsInTemplate: Map, + directiveTempaltePropertyNames: Set) { + var propertyBindings = []; + MapWrapper.forEach(bindingsInTemplate, (ast, propertyNameInTemplate) => { + var propertyBinding = createElementPropertyBinding(ast, propertyNameInTemplate); + if (isValidElementPropertyBinding(protoElement, isNgComponent, propertyBinding)) { + propertyBindings.push(propertyBinding); + } else if (!SetWrapper.has(directiveTempaltePropertyNames, propertyNameInTemplate)) { + throw new BaseException( + `Can't bind to '${propertyNameInTemplate}' since it isn't a know property of the '${DOM.tagName(protoElement).toLowerCase()}' element and there are no matching directives with a corresponding property`); + } + }); + return propertyBindings; +} + +function isValidElementPropertyBinding(protoElement: /*element*/ any, isNgComponent: boolean, + binding: api.ElementPropertyBinding): boolean { + if (binding.type === api.PropertyBindingType.PROPERTY) { + var tagName = DOM.tagName(protoElement); + var possibleCustomElement = tagName.indexOf('-') !== -1; + if (possibleCustomElement && !isNgComponent) { + // can't tell now as we don't know which properties a custom element will get + // once it is instantiated + return true; + } else { + return DOM.hasProperty(protoElement, binding.property); + } + } + return true; +} + +function createElementPropertyBinding(ast: ASTWithSource, + propertyNameInTemplate: string): api.ElementPropertyBinding { + var parts = StringWrapper.split(propertyNameInTemplate, PROPERTY_PARTS_SEPARATOR); + if (parts.length === 1) { + var propName = parts[0]; + var mappedPropName = StringMapWrapper.get(DOM.attrToPropMap, propName); + propName = isPresent(mappedPropName) ? mappedPropName : propName; + return new api.ElementPropertyBinding(api.PropertyBindingType.PROPERTY, ast, propName); + } else if (parts[0] == ATTRIBUTE_PREFIX) { + return new api.ElementPropertyBinding(api.PropertyBindingType.ATTRIBUTE, ast, parts[1]); + } else if (parts[0] == CLASS_PREFIX) { + return new api.ElementPropertyBinding(api.PropertyBindingType.CLASS, ast, parts[1]); + } else if (parts[0] == STYLE_PREFIX) { + var unit = parts.length > 2 ? parts[2] : null; + return new api.ElementPropertyBinding(api.PropertyBindingType.STYLE, ast, parts[1], unit); + } else { + throw new BaseException(`Invalid property name ${propertyNameInTemplate}`); + } +} diff --git a/modules/angular2/src/render/dom/view/view.ts b/modules/angular2/src/render/dom/view/view.ts index e4b804351d..7703dea628 100644 --- a/modules/angular2/src/render/dom/view/view.ts +++ b/modules/angular2/src/render/dom/view/view.ts @@ -1,13 +1,13 @@ import {DOM} from 'angular2/src/dom/dom_adapter'; import {ListWrapper, MapWrapper, Map, StringMapWrapper, List} from 'angular2/src/facade/collection'; -import {Locals} from 'angular2/change_detection'; -import {isPresent, isBlank, BaseException} from 'angular2/src/facade/lang'; +import {isPresent, isBlank, BaseException, stringify} from 'angular2/src/facade/lang'; import {DomProtoView} from './proto_view'; import {LightDom} from '../shadow_dom/light_dom'; import {DomElement} from './element'; import {RenderViewRef, EventDispatcher} from '../../api'; +import {camelCaseToDashCase} from '../util'; export function resolveInternalDomView(viewRef: RenderViewRef) { return (viewRef)._view; @@ -40,20 +40,42 @@ export class DomView { } setElementProperty(elementIndex: number, propertyName: string, value: any) { - var setter = this.proto.elementBinders[elementIndex].propertySetters.get(propertyName); - setter(this.boundElements[elementIndex].element, value); + DOM.setProperty(this.boundElements[elementIndex].element, propertyName, value); } - callAction(elementIndex: number, actionExpression: string, actionArgs: any) { - var binder = this.proto.elementBinders[elementIndex]; - var hostAction = binder.hostActions.get(actionExpression); - hostAction.eval(this.boundElements[elementIndex].element, this._localsWithAction(actionArgs)); + setElementAttribute(elementIndex: number, attributeName: string, value: string) { + var element = this.boundElements[elementIndex].element; + var dashCasedAttributeName = camelCaseToDashCase(attributeName); + if (isPresent(value)) { + DOM.setAttribute(element, dashCasedAttributeName, stringify(value)); + } else { + DOM.removeAttribute(element, dashCasedAttributeName); + } } - _localsWithAction(action: Object): Locals { - var map = new Map(); - map.set('$action', action); - return new Locals(null, map); + setElementClass(elementIndex: number, className: string, isAdd: boolean) { + var element = this.boundElements[elementIndex].element; + var dashCasedClassName = camelCaseToDashCase(className); + if (isAdd) { + DOM.addClass(element, dashCasedClassName); + } else { + DOM.removeClass(element, dashCasedClassName); + } + } + + setElementStyle(elementIndex: number, styleName: string, value: string) { + var element = this.boundElements[elementIndex].element; + var dashCasedStyleName = camelCaseToDashCase(styleName); + if (isPresent(value)) { + DOM.setStyle(element, dashCasedStyleName, stringify(value)); + } else { + DOM.removeStyle(element, dashCasedStyleName); + } + } + + invokeElementMethod(elementIndex: number, methodName: string, args: List) { + var element = this.boundElements[elementIndex].element; + DOM.invoke(element, methodName, args); } setText(textIndex: number, value: string) { DOM.setText(this.boundTextNodes[textIndex], value); } diff --git a/modules/angular2/src/transform/template_compiler/generator.dart b/modules/angular2/src/transform/template_compiler/generator.dart index 0bf7b9080e..c53d69a8e8 100644 --- a/modules/angular2/src/transform/template_compiler/generator.dart +++ b/modules/angular2/src/transform/template_compiler/generator.dart @@ -8,7 +8,6 @@ import 'package:angular2/src/core/compiler/proto_view_factory.dart'; import 'package:angular2/src/render/api.dart'; import 'package:angular2/src/render/dom/compiler/compile_pipeline.dart'; import 'package:angular2/src/render/dom/compiler/template_loader.dart'; -import 'package:angular2/src/render/dom/view/property_setter_factory.dart'; import 'package:angular2/src/render/xhr.dart' show XHR; import 'package:angular2/src/reflection/reflection.dart'; import 'package:angular2/src/services/url_resolver.dart'; @@ -106,7 +105,7 @@ class _TemplateExtractor { var compileElements = pipeline.process(templateEl, ViewType.COMPONENT, viewDef.componentId); var protoViewDto = compileElements[0].inheritedProtoView - .build(new PropertySetterFactory()); + .build(); reflector.reflectionCapabilities = savedReflectionCapabilities; diff --git a/modules/angular2/test/change_detection/change_detector_config.ts b/modules/angular2/test/change_detection/change_detector_config.ts index feafbf166e..098b917aad 100644 --- a/modules/angular2/test/change_detection/change_detector_config.ts +++ b/modules/angular2/test/change_detection/change_detector_config.ts @@ -28,7 +28,7 @@ function _getParser() { function _createBindingRecords(expression: string): List { var ast = _getParser().parseBinding(expression, 'location'); - return [BindingRecord.createForElement(ast, 0, PROP_NAME)]; + return [BindingRecord.createForElementProperty(ast, 0, PROP_NAME)]; } function _convertLocalsToVariableBindings(locals: Locals): List { @@ -247,8 +247,8 @@ class _DirectiveUpdating { 'interpolation': new _DirectiveUpdating( [ - BindingRecord.createForElement(_getParser().parseInterpolation('B{{a}}A', 'location'), - 0, PROP_NAME) + BindingRecord.createForElementProperty( + _getParser().parseInterpolation('B{{a}}A', 'location'), 0, PROP_NAME) ], []) }; diff --git a/modules/angular2/test/core/compiler/element_injector_spec.ts b/modules/angular2/test/core/compiler/element_injector_spec.ts index ed2495e128..760b24ca7b 100644 --- a/modules/angular2/test/core/compiler/element_injector_spec.ts +++ b/modules/angular2/test/core/compiler/element_injector_spec.ts @@ -446,7 +446,7 @@ export function main() { expect(inj.hostActionAccessors.length).toEqual(1); var accessor = inj.hostActionAccessors[0][0]; - expect(accessor.actionExpression).toEqual('onAction'); + expect(accessor.methodName).toEqual('onAction'); expect(accessor.getter(new HasHostAction())).toEqual('hostAction'); }); }); diff --git a/modules/angular2/test/core/compiler/integration_spec.ts b/modules/angular2/test/core/compiler/integration_spec.ts index 2183ae5e31..0c3643a459 100644 --- a/modules/angular2/test/core/compiler/integration_spec.ts +++ b/modules/angular2/test/core/compiler/integration_spec.ts @@ -1141,10 +1141,10 @@ export function main() { it('should specify a location of an error that happened during change detection (directive property)', inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => { - tb.overrideView(MyComp, new viewAnn.View({ - template: '', - directives: [ChildComp] - })); + tb.overrideView( + MyComp, + new viewAnn.View( + {template: '', directives: [ChildComp]})); tb.createView(MyComp, {context: ctx}) .then((view) => { @@ -1474,17 +1474,14 @@ class DirectiveUpdatingHostProperties { constructor() { this.id = "one"; } } -@Directive({ - selector: '[update-host-actions]', - host: {'@setAttr': 'setAttribute("key", $action["attrValue"])'} -}) +@Directive({selector: '[update-host-actions]', host: {'@setAttr': 'setAttribute'}}) @Injectable() class DirectiveUpdatingHostActions { setAttr: EventEmitter; constructor() { this.setAttr = new EventEmitter(); } - triggerSetAttr(attrValue) { ObservableWrapper.callNext(this.setAttr, {'attrValue': attrValue}); } + triggerSetAttr(attrValue) { ObservableWrapper.callNext(this.setAttr, ["key", attrValue]); } } @Directive({selector: '[listener]', host: {'(event)': 'onEvent($event)'}}) diff --git a/modules/angular2/test/facade/collection_spec.ts b/modules/angular2/test/facade/collection_spec.ts index 832de208c5..7795d45b76 100644 --- a/modules/angular2/test/facade/collection_spec.ts +++ b/modules/angular2/test/facade/collection_spec.ts @@ -1,6 +1,12 @@ import {describe, it, expect, beforeEach, ddescribe, iit, xit} from 'angular2/test_lib'; -import {List, ListWrapper, StringMap, StringMapWrapper} from 'angular2/src/facade/collection'; +import { + List, + ListWrapper, + StringMap, + StringMapWrapper, + MapWrapper +} from 'angular2/src/facade/collection'; export function main() { describe('ListWrapper', () => { @@ -109,5 +115,14 @@ export function main() { expect(StringMapWrapper.equals(m2, m1)).toBe(false); }); }); + + describe('MapWrapper', () => { + it('should return a list of keys values', () => { + var m = new Map(); + m.set('a', 'b'); + expect(MapWrapper.keys(m)).toEqual(['a']); + expect(MapWrapper.values(m)).toEqual(['b']); + }); + }); }); } diff --git a/modules/angular2/test/render/dom/compiler/directive_parser_spec.ts b/modules/angular2/test/render/dom/compiler/directive_parser_spec.ts index 8b877d1a0b..b390481d8d 100644 --- a/modules/angular2/test/render/dom/compiler/directive_parser_spec.ts +++ b/modules/angular2/test/render/dom/compiler/directive_parser_spec.ts @@ -27,8 +27,7 @@ export function main() { someDirectiveWithInvalidHostProperties, someDirectiveWithHostAttributes, someDirectiveWithEvents, - someDirectiveWithGlobalEvents, - someDirectiveWithHostActions + someDirectiveWithGlobalEvents ]; parser = new Parser(new Lexer()); }); @@ -161,12 +160,6 @@ export function main() { expect(eventBinding.source.source).toEqual('doItGlobal()'); }); - it('should bind directive host actions', () => { - var results = process(el('
')); - var directiveBinding = results[0].directives[0]; - expect(directiveBinding.hostActions[0].actionName).toEqual('focus'); - }); - // TODO: assertions should be enabled when running tests: // https://github.com/angular/angular/issues/1340 describe('component directives', () => { @@ -255,11 +248,6 @@ var someDirectiveWithHostAttributes = DirectiveMetadata.create({ var someDirectiveWithEvents = DirectiveMetadata.create( {selector: '[some-decor-events]', host: MapWrapper.createFromStringMap({'(click)': 'doIt()'})}); -var someDirectiveWithHostActions = DirectiveMetadata.create({ - selector: '[some-decor-host-actions]', - host: MapWrapper.createFromStringMap({'@focus': 'focus()'}) -}); - var someDirectiveWithGlobalEvents = DirectiveMetadata.create({ selector: '[some-decor-globalevents]', host: MapWrapper.createFromStringMap({'(window:resize)': 'doItGlobal()'}) diff --git a/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts b/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts index a3ee55aaf7..47cee1a733 100644 --- a/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts +++ b/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts @@ -91,12 +91,13 @@ export function main() { }); })); - it('should update element properties', inject([AsyncTestCompleter, DomTestbed], (async, tb) => { + it('should update any element property/attributes/class/style independent of the compilation', + inject([AsyncTestCompleter, DomTestbed], (async, tb) => { tb.compileAll([ someComponent, new ViewDefinition({ componentId: 'someComponent', - template: 'asdf', + template: '', directives: [] }) ]) @@ -104,33 +105,50 @@ export function main() { var rootView = tb.createRootView(protoViewDtos[0]); var cmpView = tb.createComponentView(rootView.viewRef, 0, protoViewDtos[1]); + var el = DOM.childNodes(tb.rootEl)[0]; tb.renderer.setElementProperty(cmpView.viewRef, 0, 'value', 'hello'); + expect(el.value).toEqual('hello'); + + tb.renderer.setElementClass(cmpView.viewRef, 0, 'a', true); expect(DOM.childNodes(tb.rootEl)[0].value).toEqual('hello'); + tb.renderer.setElementClass(cmpView.viewRef, 0, 'a', false); + expect(DOM.hasClass(el, 'a')).toBe(false); + + tb.renderer.setElementStyle(cmpView.viewRef, 0, 'width', '10px'); + expect(DOM.getStyle(el, 'width')).toEqual('10px'); + tb.renderer.setElementStyle(cmpView.viewRef, 0, 'width', null); + expect(DOM.getStyle(el, 'width')).toEqual(''); + + tb.renderer.setElementAttribute(cmpView.viewRef, 0, 'someAttr', 'someValue'); + expect(DOM.getAttribute(el, 'some-attr')).toEqual('someValue'); + async.done(); }); })); - it('should call actions on the element', - inject([AsyncTestCompleter, DomTestbed], (async, tb) => { - tb.compileAll([ - someComponent, - new ViewDefinition({ - componentId: 'someComponent', - template: '', - directives: [directiveWithHostActions] - }) - ]) - .then((protoViewDtos) => { - var views = tb.createRootViews(protoViewDtos); - var componentView = views[1]; + if (DOM.supportsDOMEvents()) { + it('should call actions on the element independent of the compilation', + inject([AsyncTestCompleter, DomTestbed], (async, tb) => { + tb.compileAll([ + someComponent, + new ViewDefinition({ + componentId: 'someComponent', + template: '', + directives: [] + }) + ]) + .then((protoViewDtos) => { + var views = tb.createRootViews(protoViewDtos); + var componentView = views[1]; - tb.renderer.callAction(componentView.viewRef, 0, 'value = "val"', null); - - expect(DOM.getValue(DOM.childNodes(tb.rootEl)[0])).toEqual('val'); - async.done(); - }); - })); + tb.renderer.invokeElementMethod(componentView.viewRef, 0, 'setAttribute', + ['a', 'b']); + expect(DOM.getAttribute(DOM.childNodes(tb.rootEl)[0], 'a')).toEqual('b'); + async.done(); + }); + })); + } it('should add and remove views to and from containers', inject([AsyncTestCompleter, DomTestbed], (async, tb) => { @@ -188,10 +206,3 @@ export function main() { var someComponent = DirectiveMetadata.create( {id: 'someComponent', type: DirectiveMetadata.COMPONENT_TYPE, selector: 'some-comp'}); - -var directiveWithHostActions = DirectiveMetadata.create({ - id: 'withHostActions', - type: DirectiveMetadata.DIRECTIVE_TYPE, - selector: '[with-host-actions]', - host: MapWrapper.createFromStringMap({'@setValue': 'value = "val"'}) -}); diff --git a/modules/angular2/test/render/dom/view/property_setter_factory_spec.ts b/modules/angular2/test/render/dom/view/property_setter_factory_spec.ts deleted file mode 100644 index 7be8c4ec56..0000000000 --- a/modules/angular2/test/render/dom/view/property_setter_factory_spec.ts +++ /dev/null @@ -1,190 +0,0 @@ -import { - describe, - ddescribe, - it, - iit, - xit, - xdescribe, - expect, - beforeEach, - el, - IS_DARTIUM -} from 'angular2/test_lib'; -import {PropertySetterFactory} from 'angular2/src/render/dom/view/property_setter_factory'; -import {DOM} from 'angular2/src/dom/dom_adapter'; - -export function main() { - var div, input, setterFactory; - beforeEach(() => { - div = el('
'); - input = el(''); - setterFactory = new PropertySetterFactory(); - }); - describe('property setter factory', () => { - - describe('property setters', () => { - - it('should set an existing property', () => { - var setterFn = setterFactory.createSetter(div, false, 'title'); - setterFn(div, 'Hello'); - expect(div.title).toEqual('Hello'); - - var otherSetterFn = setterFactory.createSetter(div, false, 'title'); - expect(setterFn).toBe(otherSetterFn); - }); - - if (!IS_DARTIUM) { - it('should use a noop setter if the property did not exist when the setter was created', - () => { - var setterFn = setterFactory.createSetter(div, false, 'someProp'); - div.someProp = ''; - setterFn(div, 'Hello'); - expect(div.someProp).toEqual(''); - }); - - it('should use a noop setter if the property did not exist when the setter was created for ng components', - () => { - var ce = el(''); - var setterFn = setterFactory.createSetter(ce, true, 'someProp'); - ce.someProp = ''; - setterFn(ce, 'Hello'); - expect(ce.someProp).toEqual(''); - }); - - it('should set the property for custom elements even if it was not present when the setter was created', - () => { - var ce = el(''); - var setterFn = setterFactory.createSetter(ce, false, 'someProp'); - ce.someProp = ''; - // Our CJS DOM adapter does not support custom properties, - // need to exclude here. - if (DOM.hasProperty(ce, 'someProp')) { - setterFn(ce, 'Hello'); - expect(ce.someProp).toEqual('Hello'); - } - }); - } - - }); - - describe('non-standard property setters', () => { - - it('should map readonly name to readOnly property', () => { - var setterFn = setterFactory.createSetter(input, false, 'readonly'); - expect(input.readOnly).toBeFalsy(); - setterFn(input, true); - expect(input.readOnly).toBeTruthy(); - - var otherSetterFn = setterFactory.createSetter(input, false, 'readonly'); - expect(setterFn).toBe(otherSetterFn); - }); - - it('should return a setter for innerHtml', () => { - var setterFn = setterFactory.createSetter(div, false, 'innerHtml'); - setterFn(div, ''); - expect(DOM.getInnerHTML(div)).toEqual(''); - - var otherSetterFn = setterFactory.createSetter(div, false, 'innerHtml'); - expect(setterFn).toBe(otherSetterFn); - }); - - it('should return a setter for tabIndex', () => { - var setterFn = setterFactory.createSetter(div, false, 'tabindex'); - setterFn(div, 1); - expect(div.tabIndex).toEqual(1); - - var otherSetterFn = setterFactory.createSetter(div, false, 'tabindex'); - expect(setterFn).toBe(otherSetterFn); - }); - - }); - - describe('attribute setters', () => { - - it('should return a setter for an attribute', () => { - var setterFn = setterFactory.createSetter(div, false, 'attr.role'); - setterFn(div, 'button'); - expect(DOM.getAttribute(div, 'role')).toEqual('button'); - setterFn(div, null); - expect(DOM.getAttribute(div, 'role')).toEqual(null); - expect(() => { setterFn(div, 4); }) - .toThrowError("Invalid role attribute, only string values are allowed, got '4'"); - - var otherSetterFn = setterFactory.createSetter(div, false, 'attr.role'); - expect(setterFn).toBe(otherSetterFn); - }); - - it('should de-normalize attribute names', () => { - var setterFn = setterFactory.createSetter(div, false, 'attr.ariaLabel'); - setterFn(div, 'fancy button'); - expect(DOM.getAttribute(div, 'aria-label')).toEqual('fancy button'); - - var otherSetterFn = setterFactory.createSetter(div, false, 'attr.ariaLabel'); - expect(setterFn).toBe(otherSetterFn); - }); - }); - - describe('classList setters', () => { - - it('should return a setter for a class', () => { - var setterFn = setterFactory.createSetter(div, false, 'class.active'); - setterFn(div, true); - expect(DOM.hasClass(div, 'active')).toEqual(true); - setterFn(div, false); - expect(DOM.hasClass(div, 'active')).toEqual(false); - - var otherSetterFn = setterFactory.createSetter(div, false, 'class.active'); - expect(setterFn).toBe(otherSetterFn); - }); - - it('should de-normalize class names', () => { - var setterFn = setterFactory.createSetter(div, false, 'class.veryActive'); - setterFn(div, true); - expect(DOM.hasClass(div, 'very-active')).toEqual(true); - setterFn(div, false); - expect(DOM.hasClass(div, 'very-active')).toEqual(false); - - var otherSetterFn = setterFactory.createSetter(div, false, 'class.veryActive'); - expect(setterFn).toBe(otherSetterFn); - }); - }); - - describe('style setters', () => { - - it('should return a setter for a style', () => { - var setterFn = setterFactory.createSetter(div, false, 'style.width'); - setterFn(div, '40px'); - expect(DOM.getStyle(div, 'width')).toEqual('40px'); - setterFn(div, null); - expect(DOM.getStyle(div, 'width')).toEqual(''); - - var otherSetterFn = setterFactory.createSetter(div, false, 'style.width'); - expect(setterFn).toBe(otherSetterFn); - }); - - it('should de-normalize style names', () => { - var setterFn = setterFactory.createSetter(div, false, 'style.textAlign'); - setterFn(div, 'right'); - expect(DOM.getStyle(div, 'text-align')).toEqual('right'); - setterFn(div, null); - expect(DOM.getStyle(div, 'text-align')).toEqual(''); - - var otherSetterFn = setterFactory.createSetter(div, false, 'style.textAlign'); - expect(setterFn).toBe(otherSetterFn); - }); - - it('should return a setter for a style with a unit', () => { - var setterFn = setterFactory.createSetter(div, false, 'style.height.px'); - setterFn(div, 40); - expect(DOM.getStyle(div, 'height')).toEqual('40px'); - setterFn(div, null); - expect(DOM.getStyle(div, 'height')).toEqual(''); - - var otherSetterFn = setterFactory.createSetter(div, false, 'style.height.px'); - expect(setterFn).toBe(otherSetterFn); - }); - - }); - - }); -} diff --git a/modules/angular2/test/render/dom/view/proto_view_builder_spec.ts b/modules/angular2/test/render/dom/view/proto_view_builder_spec.ts new file mode 100644 index 0000000000..81b3596567 --- /dev/null +++ b/modules/angular2/test/render/dom/view/proto_view_builder_spec.ts @@ -0,0 +1,115 @@ +import { + describe, + ddescribe, + it, + iit, + xit, + xdescribe, + expect, + beforeEach, + el, + IS_DARTIUM +} from 'angular2/test_lib'; + +import {ProtoViewBuilder} from 'angular2/src/render/dom/view/proto_view_builder'; +import {ASTWithSource, AST} from 'angular2/change_detection'; +import {PropertyBindingType, ViewType} from 'angular2/src/render/api'; + +export function main() { + function emptyExpr() { return new ASTWithSource(new AST(), 'empty', 'empty'); } + + describe('ProtoViewBuilder', () => { + var builder; + beforeEach(() => { builder = new ProtoViewBuilder(el('
'), ViewType.EMBEDDED); }); + + describe('verification of properties', () => { + + it('should throw for unknown properties', () => { + builder.bindElement(el('
')).bindProperty('unknownProperty', emptyExpr()); + expect(() => builder.build()) + .toThrowError( + `Can't bind to 'unknownProperty' since it isn't a know property of the 'div' element and there are no matching directives with a corresponding property`); + }); + + it('should should allow unknown properties if a directive uses it', () => { + builder.bindElement(el('
')).bindProperty('unknownProperty', emptyExpr()); + expect(() => builder.build()) + .toThrowError( + `Can't bind to 'unknownProperty' since it isn't a know property of the 'div' element and there are no matching directives with a corresponding property`); + }); + + it('should allow unknown properties on custom elements', () => { + var binder = builder.bindElement(el('')); + binder.bindProperty('unknownProperty', emptyExpr()); + binder.bindDirective(0).bindProperty('someDirProperty', emptyExpr(), 'unknownProperty'); + expect(() => builder.build()).not.toThrow(); + }); + + it('should throw for unkown properties on custom elements if there is an ng component', () => { + var binder = builder.bindElement(el('')); + binder.bindProperty('unknownProperty', emptyExpr()); + binder.setComponentId('someComponent'); + expect(() => builder.build()) + .toThrowError( + `Can't bind to 'unknownProperty' since it isn't a know property of the 'some-custom' element and there are no matching directives with a corresponding property`); + }); + + }); + + describe('property normalization', () => { + it('should normalize "innerHtml" to "innerHTML"', () => { + builder.bindElement(el('
')).bindProperty('innerHtml', emptyExpr()); + var pv = builder.build(); + expect(pv.elementBinders[0].propertyBindings[0].property).toEqual('innerHTML'); + }); + + it('should normalize "tabindex" to "tabIndex"', () => { + builder.bindElement(el('
')).bindProperty('tabindex', emptyExpr()); + var pv = builder.build(); + expect(pv.elementBinders[0].propertyBindings[0].property).toEqual('tabIndex'); + }); + + it('should normalize "readonly" to "readOnly"', () => { + builder.bindElement(el('')).bindProperty('readonly', emptyExpr()); + var pv = builder.build(); + expect(pv.elementBinders[0].propertyBindings[0].property).toEqual('readOnly'); + }); + + }); + + describe('property binding types', () => { + it('should detect property names', () => { + builder.bindElement(el('
')).bindProperty('tabindex', emptyExpr()); + var pv = builder.build(); + expect(pv.elementBinders[0].propertyBindings[0].type).toEqual(PropertyBindingType.PROPERTY); + }); + + it('should detect attribute names', () => { + builder.bindElement(el('
')).bindProperty('attr.someName', emptyExpr()); + var pv = builder.build(); + expect(pv.elementBinders[0].propertyBindings[0].type) + .toEqual(PropertyBindingType.ATTRIBUTE); + }); + + it('should detect class names', () => { + builder.bindElement(el('
')).bindProperty('class.someName', emptyExpr()); + var pv = builder.build(); + expect(pv.elementBinders[0].propertyBindings[0].type).toEqual(PropertyBindingType.CLASS); + }); + + it('should detect style names', () => { + builder.bindElement(el('
')).bindProperty('style.someName', emptyExpr()); + var pv = builder.build(); + expect(pv.elementBinders[0].propertyBindings[0].type).toEqual(PropertyBindingType.STYLE); + }); + + it('should detect style units', () => { + builder.bindElement(el('
')).bindProperty('style.someName.someUnit', emptyExpr()); + var pv = builder.build(); + expect(pv.elementBinders[0].propertyBindings[0].unit).toEqual('someUnit'); + }); + }); + + + }); +} diff --git a/modules/angular2/test/render/dom/view/view_spec.ts b/modules/angular2/test/render/dom/view/view_spec.ts index b037a04db3..b13692364b 100644 --- a/modules/angular2/test/render/dom/view/view_spec.ts +++ b/modules/angular2/test/render/dom/view/view_spec.ts @@ -16,6 +16,7 @@ import { proxy } from 'angular2/test_lib'; import {isBlank} from 'angular2/src/facade/lang'; +import {ListWrapper} from 'angular2/src/facade/collection'; import {DomProtoView} from 'angular2/src/render/dom/view/proto_view'; import {ElementBinder} from 'angular2/src/render/dom/view/element_binder'; @@ -40,7 +41,7 @@ export function main() { function createView(pv = null, boundElementCount = 0) { if (isBlank(pv)) { - pv = createProtoView(); + pv = createProtoView(ListWrapper.createFixedSize(boundElementCount)); } var root = el('
'); var boundElements = []; @@ -72,5 +73,87 @@ export function main() { }); + describe('setElementProperty', () => { + var el, view; + beforeEach(() => { + view = createView(null, 1); + el = view.boundElements[0].element; + }); + + it('should update the property value', () => { + view.setElementProperty(0, 'title', 'Hello'); + expect(el.title).toEqual('Hello'); + }); + + }); + + describe('setElementAttribute', () => { + var el, view; + beforeEach(() => { + view = createView(null, 1); + el = view.boundElements[0].element; + }); + + it('should update and remove an attribute', () => { + view.setElementAttribute(0, 'role', 'button'); + expect(DOM.getAttribute(el, 'role')).toEqual('button'); + view.setElementAttribute(0, 'role', null); + expect(DOM.getAttribute(el, 'role')).toEqual(null); + }); + + it('should de-normalize attribute names', () => { + view.setElementAttribute(0, 'ariaLabel', 'fancy button'); + expect(DOM.getAttribute(el, 'aria-label')).toEqual('fancy button'); + }); + }); + + describe('setElementClass', () => { + var el, view; + beforeEach(() => { + view = createView(null, 1); + el = view.boundElements[0].element; + }); + + it('should set and remove a class', () => { + view.setElementClass(0, 'active', true); + expect(DOM.hasClass(el, 'active')).toEqual(true); + + view.setElementClass(0, 'active', false); + expect(DOM.hasClass(el, 'active')).toEqual(false); + }); + + it('should de-normalize class names', () => { + view.setElementClass(0, 'veryActive', true); + expect(DOM.hasClass(el, 'very-active')).toEqual(true); + + view.setElementClass(0, 'veryActive', false); + expect(DOM.hasClass(el, 'very-active')).toEqual(false); + }); + }); + + describe('setElementStyle', () => { + var el, view; + beforeEach(() => { + view = createView(null, 1); + el = view.boundElements[0].element; + }); + + it('should set and remove styles', () => { + view.setElementStyle(0, 'width', '40px'); + expect(DOM.getStyle(el, 'width')).toEqual('40px'); + + view.setElementStyle(0, 'width', null); + expect(DOM.getStyle(el, 'width')).toEqual(''); + }); + + it('should de-normalize style names', () => { + view.setElementStyle(0, 'textAlign', 'right'); + expect(DOM.getStyle(el, 'text-align')).toEqual('right'); + view.setElementStyle(0, 'textAlign', null); + expect(DOM.getStyle(el, 'text-align')).toEqual(''); + }); + + }); + }); } diff --git a/modules/angular2/test/transform/template_compiler/inline_expression_files/expected/hello.ng_deps.dart b/modules/angular2/test/transform/template_compiler/inline_expression_files/expected/hello.ng_deps.dart index e9faabb671..1fc79e64aa 100644 --- a/modules/angular2/test/transform/template_compiler/inline_expression_files/expected/hello.ng_deps.dart +++ b/modules/angular2/test/transform/template_compiler/inline_expression_files/expected/hello.ng_deps.dart @@ -18,9 +18,6 @@ void initReflector(reflector) { ] }) ..registerGetters({'b': (o) => o.b, 'greeting': (o) => o.greeting}) - ..registerSetters({ - 'b': (o, v) => o.b = v, - 'greeting': (o, v) => o.greeting = v, - 'a': (o, v) => o.a = v - }); + ..registerSetters( + {'b': (o, v) => o.b = v, 'greeting': (o, v) => o.greeting = v}); } diff --git a/modules/angular2_material/src/components/grid_list/grid_list.ts b/modules/angular2_material/src/components/grid_list/grid_list.ts index 756c729875..e1cfc51b94 100644 --- a/modules/angular2_material/src/components/grid_list/grid_list.ts +++ b/modules/angular2_material/src/components/grid_list/grid_list.ts @@ -225,7 +225,7 @@ export class MdGridList { '[style.left]': 'styleLeft', '[style.marginTop]': 'styleMarginTop', '[style.paddingTop]': 'stylePaddingTop', - '[role]': '"listitem"' + '[attr.role]': '"listitem"' }, lifecycle: [onDestroy, onChange] }) diff --git a/tools/broccoli/broccoli-merge-trees.ts b/tools/broccoli/broccoli-merge-trees.ts index b749e4c92f..60672c1a2b 100644 --- a/tools/broccoli/broccoli-merge-trees.ts +++ b/tools/broccoli/broccoli-merge-trees.ts @@ -73,23 +73,25 @@ export class MergeTrees implements DiffingBroccoliPlugin { // Update cache treeDiffs.reverse().forEach((treeDiff: DiffResult, index) => { index = treeDiffs.length - 1 - index; - treeDiff.removedPaths.forEach((removedPath) => { - let cache = this.pathCache[removedPath]; - // ASSERT(cache !== undefined); - // ASSERT(contains(cache, index)); - if (cache[cache.length - 1] === index) { - pathsToRemove.push(path.join(this.cachePath, removedPath)); - cache.pop(); - if (cache.length === 0) { - this.pathCache[removedPath] = undefined; - } else if (!emitted[removedPath]) { - if (cache.length === 1 && !overwrite) { - throw pathOverwrittenError(removedPath); + if (treeDiff.removedPaths) { + treeDiff.removedPaths.forEach((removedPath) => { + let cache = this.pathCache[removedPath]; + // ASSERT(cache !== undefined); + // ASSERT(contains(cache, index)); + if (cache[cache.length - 1] === index) { + pathsToRemove.push(path.join(this.cachePath, removedPath)); + cache.pop(); + if (cache.length === 0) { + this.pathCache[removedPath] = undefined; + } else if (!emitted[removedPath]) { + if (cache.length === 1 && !overwrite) { + throw pathOverwrittenError(removedPath); + } + emit(removedPath); } - emit(removedPath); } - } - }); + }); + } let pathsToUpdate = treeDiff.addedPaths;