From e1d401916b9d262104a73511a3f78ab55f08a2a8 Mon Sep 17 00:00:00 2001 From: Victor Date: Wed, 19 Mar 2014 15:54:10 +0100 Subject: [PATCH 1/7] doc(NodeAttrs): rework class comments, make namings consistent --- lib/core_dom/directive.dart | 27 ++++++++++++++++++++------- lib/core_dom/view_factory.dart | 2 +- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/core_dom/directive.dart b/lib/core_dom/directive.dart index bf183fac9..9a6c868b2 100644 --- a/lib/core_dom/directive.dart +++ b/lib/core_dom/directive.dart @@ -7,9 +7,12 @@ typedef void _AttributeChanged(String newValue); typedef void Mustache(bool hasObservers); /** - * NodeAttrs is a facade for element attributes. The facade is responsible - * for normalizing attribute names as well as allowing access to the - * value of the directive. + * NodeAttrs is a facade for element attributes. + * + * This facade allows reading and writing the attribute values as well as + * adding observers triggered when: + * - The value of an attribute changes, + * - An element becomes observed. */ class NodeAttrs { final dom.Element element; @@ -30,15 +33,20 @@ class NodeAttrs { } else { element.attributes[attrName] = value; } + if (_observers != null && _observers.containsKey(attrName)) { _observers[attrName].forEach((notifyFn) => notifyFn(value)); } } /** - * Observe changes to the attribute by invoking the [notifyFn] function. On - * registration the [notifyFn] function gets invoked synchronize with the - * current value. + * Observes changes to the attribute by invoking the [notifyFn] + * function. On registration the [notifyFn] function gets invoked in order to + * synchronize with the current value. + * + * When an observed is registered on an attributes any existing + * [_observerListeners] will be called with the first parameter set to + * [:true:] */ observe(String attrName, notifyFn(String value)) { if (_observers == null) _observers = >{}; @@ -61,6 +69,11 @@ class NodeAttrs { Iterable get keys => element.attributes.keys; + /** + * Registers a listener to be called when the attribute [attrName] becomes + * observed. On registration [notifyFn] function gets invoked with [:false:] + * as the first argument. + */ void listenObserverChanges(String attrName, Mustache notifyFn) { _mustacheAttrs[attrName] = new _MustacheAttr(notifyFn); notifyFn(false); @@ -68,7 +81,7 @@ class NodeAttrs { } /** - * TemplateLoader is an asynchronous access to ShadowRoot which is + * [TemplateLoader] is an asynchronous access to ShadowRoot which is * loaded asynchronously. It allows a Component to be notified when its * ShadowRoot is ready. */ diff --git a/lib/core_dom/view_factory.dart b/lib/core_dom/view_factory.dart index e538175fe..beb810c30 100644 --- a/lib/core_dom/view_factory.dart +++ b/lib/core_dom/view_factory.dart @@ -248,7 +248,7 @@ class _AnchorAttrs extends NodeAttrs { _AnchorAttrs(DirectiveRef this._directiveRef): super(null); - operator [](name) => name == '.' ? _directiveRef.value : null; + DirectiveRef operator [](name) => name == '.' ? _directiveRef.value : null; void observe(String attributeName, _AttributeChanged notifyFn) { notifyFn(attributeName == '.' ? _directiveRef.value : null); From a03e16b3c35030943f5fa05e367473668be8bf85 Mon Sep 17 00:00:00 2001 From: Victor Date: Wed, 19 Mar 2014 18:00:42 +0100 Subject: [PATCH 2/7] style: cleanup --- lib/core_dom/view_factory.dart | 2 +- lib/directive/input_select.dart | 8 +++----- lib/directive/ng_bind_html.dart | 4 ++-- lib/directive/ng_repeat.dart | 1 + 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/core_dom/view_factory.dart b/lib/core_dom/view_factory.dart index beb810c30..00f6b9cdf 100644 --- a/lib/core_dom/view_factory.dart +++ b/lib/core_dom/view_factory.dart @@ -248,7 +248,7 @@ class _AnchorAttrs extends NodeAttrs { _AnchorAttrs(DirectiveRef this._directiveRef): super(null); - DirectiveRef operator [](name) => name == '.' ? _directiveRef.value : null; + String operator [](name) => name == '.' ? _directiveRef.value : null; void observe(String attributeName, _AttributeChanged notifyFn) { notifyFn(attributeName == '.' ? _directiveRef.value : null); diff --git a/lib/directive/input_select.dart b/lib/directive/input_select.dart index 3b4d8e19d..9736040b1 100644 --- a/lib/directive/input_select.dart +++ b/lib/directive/input_select.dart @@ -105,9 +105,7 @@ class OptionValueDirective implements NgAttachAware, } attach() { - if (_inputSelectDirective != null) { - _inputSelectDirective.dirty(); - } + if (_inputSelectDirective != null) _inputSelectDirective.dirty(); } detach() { @@ -151,8 +149,8 @@ class _SingleSelectMode extends _SelectMode { dom.SelectElement select, NgModel model, this._nullOption, - this._unknownOption - ): super(expando, select, model) { + this._unknownOption) + : super(expando, select, model) { } onViewChange(event) { diff --git a/lib/directive/ng_bind_html.dart b/lib/directive/ng_bind_html.dart index 075f448df..31f47e50e 100644 --- a/lib/directive/ng_bind_html.dart +++ b/lib/directive/ng_bind_html.dart @@ -28,6 +28,6 @@ class NgBindHtmlDirective { * expression is innerHTML'd according to the rules specified in this class' * documentation. */ - set value(value) => element.setInnerHtml(value == null ? '' : value.toString(), - validator: validator); + void set value(value) => element.setInnerHtml( + value == null ? '' : value.toString(), validator: validator); } diff --git a/lib/directive/ng_repeat.dart b/lib/directive/ng_repeat.dart index 209c1731f..046b98aae 100644 --- a/lib/directive/ng_repeat.dart +++ b/lib/directive/ng_repeat.dart @@ -165,6 +165,7 @@ class NgRepeatDirective { _viewPort.insert(view, insertAfter: previousView); }; + // todo(vicb) refactor once GH-774 gets fixed if (_rows == null) { _rows = new List<_Row>(length); for (var i = 0; i < length; i++) { From 38e43c786963d9cc3407fb4ac75204fe38c1884d Mon Sep 17 00:00:00 2001 From: Victor Date: Wed, 19 Mar 2014 18:00:57 +0100 Subject: [PATCH 3/7] feat(ngElement): add support for attributes --- lib/core_dom/ng_element.dart | 63 +++++++++---- test/_specs.dart | 14 ++- test/core_dom/ng_element_spec.dart | 142 +++++++++++++++++++++-------- 3 files changed, 162 insertions(+), 57 deletions(-) diff --git a/lib/core_dom/ng_element.dart b/lib/core_dom/ng_element.dart index 7e01c48ae..5c5be2e1d 100644 --- a/lib/core_dom/ng_element.dart +++ b/lib/core_dom/ng_element.dart @@ -2,38 +2,65 @@ part of angular.core.dom_internal; @NgInjectableService() class NgElement { + static const _TO_BE_REMOVED = const Object(); final dom.Element node; final Scope _scope; final NgAnimate _animate; - final _classes = new Map(); - NgElement(this.node, this._scope, NgAnimate this._animate); + final _classesToUpdate = {}; + final _attributesToUpdate = {}; - addClass(String className) { - if(_classes.isEmpty) { - _listenOnWrite(); - } - _classes[className] = true; + bool _writeScheduled = false; + + NgElement(this.node, this._scope, this._animate); + + void addClass(String className) { + _scheduleDomWrite(); + _classesToUpdate[className] = true; } - removeClass(String className) { - if(_classes.isEmpty) { - _listenOnWrite(); - } - _classes[className] = false; + void removeClass(String className) { + _scheduleDomWrite(); + _classesToUpdate[className] = false; + } + + // todo(vicb) add tests + void setAttribute(String attrName, [value = '']) { + _scheduleDomWrite(); + _attributesToUpdate[attrName] = value == null ? '' : value; } - _listenOnWrite() { - _scope.rootScope.domWrite(() => flush()); + void removeAttribute(String attrName) { + _scheduleDomWrite(); + _attributesToUpdate[attrName] = _TO_BE_REMOVED; } - flush() { - _classes.forEach((className, status) { - status == true + _scheduleDomWrite() { + if (!_writeScheduled) { + _writeScheduled = true; + _scope.rootScope.domWrite(() { + _writeToDom(); + _writeScheduled = false; + }); + } + } + + _writeToDom() { + _classesToUpdate.forEach((String className, bool toBeAdded) { + toBeAdded ? _animate.addClass(node, className) : _animate.removeClass(node, className); }); - _classes.clear(); + _classesToUpdate.clear(); + + _attributesToUpdate.forEach((String attrName, value) { + if (value == _TO_BE_REMOVED) { + node.attributes.remove(attrName); + } else { + node.attributes[attrName] = value; + } + }); + _attributesToUpdate.clear(); } } diff --git a/test/_specs.dart b/test/_specs.dart index 6ff226ab1..b7670e67a 100644 --- a/test/_specs.dart +++ b/test/_specs.dart @@ -78,7 +78,15 @@ class Expect { true, reason: 'method invoked once with correct arguments. (Called ${actual.count} times)'); - toHaveClass(cls) => unit.expect(actual.classes.contains(cls), true, reason: ' Expected ${actual} to have css class ${cls}'); + toHaveClass(cls) => unit.expect(actual.classes.contains(cls), true, reason: 'Expected $actual to have css class "$cls"'); + toHaveAttribute(name, [value = null]) { + unit.expect(actual.attributes.containsKey(name), true, + reason: 'Epxected $actual to have attribute $name'); + if (value != null) { + unit.expect(actual.attributes[name], value, + reason: 'Epxected $actual attribute "$name" to be "$value"'); + } + } toEqualSelect(options) { var actualOptions = []; @@ -162,7 +170,9 @@ class NotExpect { toHaveBeenCalled() => unit.expect(actual.called, false, reason: 'method called'); toThrow() => actual(); - toHaveClass(cls) => unit.expect(actual.classes.contains(cls), false, reason: ' Expected ${actual} to not have css class ${cls}'); + toHaveClass(cls) => unit.expect(actual.classes.contains(cls), false, reason: ' Expected $actual to not have css class "$cls"'); + toHaveAttribute(name) => unit.expect(actual.attributes.containsKey(name), + false, reason: ' Expected $actual to not have attribute "$name"'); toBe(expected) => unit.expect(actual, unit.predicate((actual) => !identical(expected, actual), 'not $expected')); toEqual(expected) => unit.expect(actual, diff --git a/test/core_dom/ng_element_spec.dart b/test/core_dom/ng_element_spec.dart index d497b06bd..3d5d23515 100644 --- a/test/core_dom/ng_element_spec.dart +++ b/test/core_dom/ng_element_spec.dart @@ -6,75 +6,143 @@ import 'dart:html' as dom; void main() { describe('ngElement', () { - it('should add classes on domWrite to the element', - (TestBed _, NgAnimate animate) { + describe('classes', () { + it('should add classes to the element on domWrite', + (TestBed _, NgAnimate animate) { + + var scope = _.rootScope; + var element = e('
'); + var ngElement = new NgElement(element, scope, animate); + + ngElement..addClass('one')..addClass('two three'); + + ['one', 'two', 'three'].forEach((className) { + expect(element).not.toHaveClass(className); + }); + + scope.apply(); + + ['one', 'two', 'three'].forEach((className) { + expect(element).toHaveClass(className); + }); + }); + + it('should remove classes from the element on domWrite', + (TestBed _, NgAnimate animate) { + + var scope = _.rootScope; + var element = e('
'); + var ngElement = new NgElement(element, scope, animate); + + ngElement..removeClass('one') + ..removeClass('two') + ..removeClass('three'); + + ['one', 'two', 'three', 'four'].forEach((className) { + expect(element).toHaveClass(className); + }); + + scope.apply(); + + ['one', 'two', 'three'].forEach((className) { + expect(element).not.toHaveClass(className); + }); + expect(element).toHaveClass('four'); + }); + + it('should always apply the last dom operation on the given className', + (TestBed _, NgAnimate animate) { + + var scope = _.rootScope; + var element = e('
'); + var ngElement = new NgElement(element, scope, animate); + + ngElement..addClass('one') + ..addClass('one') + ..removeClass('one'); + + expect(element).not.toHaveClass('one'); + + scope.apply(); + + expect(element).not.toHaveClass('one'); + + ngElement..removeClass('one') + ..removeClass('one') + ..addClass('one'); + + scope.apply(); + + expect(element.classes.contains('one')).toBe(true); + }); + }); + }); + + describe('attributes', () { + it('should set attributes on domWrite to the element', + (TestBed _, NgAnimate animate) { var scope = _.rootScope; - var element = _.compile('
'); + var element = e('
'); var ngElement = new NgElement(element, scope, animate); - ngElement.addClass('one'); - ngElement.addClass('two three'); + ngElement.setAttribute('id', 'foo'); + ngElement.setAttribute('title', 'bar'); - ['one','two','three'].forEach((className) { - expect(element.classes.contains(className)).toBe(false); + ['id', 'title'].forEach((name) { + expect(element).not.toHaveAttribute(name); }); scope.apply(); - ['one','two','three'].forEach((className) { - expect(element.classes.contains(className)).toBe(true); - }); + expect(element).toHaveAttribute('id', 'foo'); + expect(element).toHaveAttribute('title', 'bar'); }); - it('should remove classes on domWrite to the element', - (TestBed _, NgAnimate animate) { + it('should remove attributes from the element on domWrite ', + (TestBed _, NgAnimate animate) { var scope = _.rootScope; - var element = _.compile('
'); + var element = e('
'); var ngElement = new NgElement(element, scope, animate); - ngElement.removeClass('one'); - ngElement.removeClass('two'); - ngElement.removeClass('three'); + ngElement..removeAttribute('id') + ..removeAttribute('title'); - ['one','two','three'].forEach((className) { - expect(element.classes.contains(className)).toBe(true); - }); - expect(element.classes.contains('four')).toBe(true); + expect(element).toHaveAttribute('id', 'foo'); + expect(element).toHaveAttribute('title', 'bar'); scope.apply(); - ['one','two','three'].forEach((className) { - expect(element.classes.contains(className)).toBe(false); - }); - expect(element.classes.contains('four')).toBe(true); + expect(element).not.toHaveAttribute('id'); + expect(element).not.toHaveAttribute('title'); }); - it('should always apply the last dom operation on the given className', - (TestBed _, NgAnimate animate) { + it('should always apply the last operation on the attribute', + (TestBed _, NgAnimate animate) { var scope = _.rootScope; - var element = _.compile('
'); + var element = e('
'); var ngElement = new NgElement(element, scope, animate); - ngElement.addClass('one'); - ngElement.addClass('one'); - ngElement.removeClass('one'); + ngElement..setAttribute('id', 'foo') + ..setAttribute('id', 'foo') + ..removeAttribute('id'); - expect(element.classes.contains('one')).toBe(false); + expect(element).not.toHaveAttribute('id'); scope.apply(); - expect(element.classes.contains('one')).toBe(false); + expect(element).not.toHaveAttribute('id'); + + ngElement..removeAttribute('id') + ..setAttribute('id', 'foobar') + ..setAttribute('id', 'foo'); - element.classes.add('one'); + scope.apply(); - ngElement.removeClass('one'); - ngElement.removeClass('one'); - ngElement.addClass('one'); + expect(element).toHaveAttribute('id', 'foo'); - expect(element.classes.contains('one')).toBe(true); }); }); } From 99ba3030d668fbb05479be6212a2a87774431f07 Mon Sep 17 00:00:00 2001 From: Victor Date: Wed, 19 Mar 2014 18:01:12 +0100 Subject: [PATCH 4/7] refactor(ngSrcBoolean): Remove usage of NodeAttrs --- lib/directive/ng_src_boolean.dart | 39 ++++++++++++++++++------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/lib/directive/ng_src_boolean.dart b/lib/directive/ng_src_boolean.dart index f2d57ead6..b1c1fabb4 100644 --- a/lib/directive/ng_src_boolean.dart +++ b/lib/directive/ng_src_boolean.dart @@ -5,7 +5,7 @@ part of angular.directive; * * Using `