From aa6adc77aef68f555928a385ecd1e0049cd9b6d9 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Tue, 5 Jun 2018 15:00:12 +0200 Subject: [PATCH 1/4] feat(ngRef): add directive to publish controller, or element into scope Thanks to @drpicox for the original implementation: PR #14080 Closes #16511 --- angularFiles.js | 1 + docs/content/error/ngRef/noctrl.ngdoc | 17 + docs/content/error/ngRef/nonassign.ngdoc | 27 ++ src/AngularPublic.js | 2 + src/ng/directive/ngRef.js | 296 ++++++++++++ test/helpers/matchers.js | 1 + test/ng/directive/ngRefSpec.js | 561 +++++++++++++++++++++++ 7 files changed, 905 insertions(+) create mode 100644 docs/content/error/ngRef/noctrl.ngdoc create mode 100644 docs/content/error/ngRef/nonassign.ngdoc create mode 100644 src/ng/directive/ngRef.js create mode 100644 test/ng/directive/ngRefSpec.js diff --git a/angularFiles.js b/angularFiles.js index 0233722adfc4..01d9dfd3f0f3 100644 --- a/angularFiles.js +++ b/angularFiles.js @@ -74,6 +74,7 @@ var angularFiles = { 'src/ng/directive/ngNonBindable.js', 'src/ng/directive/ngOptions.js', 'src/ng/directive/ngPluralize.js', + 'src/ng/directive/ngRef.js', 'src/ng/directive/ngRepeat.js', 'src/ng/directive/ngShowHide.js', 'src/ng/directive/ngStyle.js', diff --git a/docs/content/error/ngRef/noctrl.ngdoc b/docs/content/error/ngRef/noctrl.ngdoc new file mode 100644 index 000000000000..29d19a9ae134 --- /dev/null +++ b/docs/content/error/ngRef/noctrl.ngdoc @@ -0,0 +1,17 @@ +@ngdoc error +@name ngRef:noctrl +@fullName A controller for the value of `ngRefRead` could not be found on the element. +@description + +This error occurs when the {@link ng.ngRef ngRef directive} specifies +a value in `ngRefRead` that cannot be resolved to a directive / component controller. + +Causes for this error can be: + +1. Your `ngRefRead` value has a typo. +2. You have a typo in the *registered* directive / component name. +3. The directive / component does not have a controller. + +Note that `ngRefRead` takes the name of the component / directive, not the name of controller, and +also not the combination of directive and 'Controller'. For example, for a directive called 'myDirective', +the correct declaration is `
`. diff --git a/docs/content/error/ngRef/nonassign.ngdoc b/docs/content/error/ngRef/nonassign.ngdoc new file mode 100644 index 000000000000..9c1c52ee35b7 --- /dev/null +++ b/docs/content/error/ngRef/nonassign.ngdoc @@ -0,0 +1,27 @@ +@ngdoc error +@name ngRef:nonassign +@fullName Non-Assignable Expression +@description + +This error occurs when ngRef defines an expression that is not-assignable. + +In order for ngRef to work, it must be possible to write the reference into the path defined with the expression. + +For example, the following expressions are non-assignable: + +``` + + + + + + + +``` + +To resolve this error, use a path expression that is assignable: + +``` + + +``` diff --git a/src/AngularPublic.js b/src/AngularPublic.js index c18889911a50..dca14bdd6ffd 100644 --- a/src/AngularPublic.js +++ b/src/AngularPublic.js @@ -28,6 +28,7 @@ ngInitDirective, ngNonBindableDirective, ngPluralizeDirective, + ngRefDirective, ngRepeatDirective, ngShowDirective, ngStyleDirective, @@ -194,6 +195,7 @@ function publishExternalAPI(angular) { ngInit: ngInitDirective, ngNonBindable: ngNonBindableDirective, ngPluralize: ngPluralizeDirective, + ngRef: ngRefDirective, ngRepeat: ngRepeatDirective, ngShow: ngShowDirective, ngStyle: ngStyleDirective, diff --git a/src/ng/directive/ngRef.js b/src/ng/directive/ngRef.js new file mode 100644 index 000000000000..4b3c7a746ba4 --- /dev/null +++ b/src/ng/directive/ngRef.js @@ -0,0 +1,296 @@ +'use strict'; + +/** + * @ngdoc directive + * @name ngRef + * @restrict A + * + * @description + * The `ngRef` attribute tells AngularJS to assign the controller of a component (or a directive) + * to the given property in the current scope. It is also possible to add the jqlite-wrapped DOM + * element to the scope. + * + * If the element with `ngRef` is destroyed `null` is assigned to the property. + * + * Note that if you want to assign from a child into the parent scope, you must initialize the + * target property on the parent scope, otherwise `ngRef` will assign on the child scope. + * This commonly happens when assigning elements or components wrapped in {@link ngIf} or + * {@link ngRepeat}. See the second example below. + * + * + * @element ANY + * @param {string} ngRef property name - A valid AngularJS expression identifier to which the + * controller or jqlite-wrapped DOM element will be bound. + * @param {string=} ngRefRead read value - The name of a directive (or component) on this element, + * or the special string `$element`. If a name is provided, `ngRef` will + * assign the matching controller. If `$element` is provided, the element + * itself is assigned (even if a controller is available). + * + * + * @example + * ### Simple toggle + * This example shows how the controller of the component toggle + * is reused in the template through the scope to use its logic. + * + * + * + * + *
+ * You are using a component in the same template to show it. + *
+ * + * + * angular.module('myApp', []) + * .component('myToggle', { + * controller: function ToggleController() { + * var opened = false; + * this.isOpen = function() { return opened; }; + * this.toggle = function() { opened = !opened; }; + * } + * }); + * + * + * it('should publish the toggle into the scope', function() { + * var toggle = element(by.buttonText('Toggle')); + * expect(toggle.evaluate('myToggle.isOpen()')).toEqual(false); + * toggle.click(); + * expect(toggle.evaluate('myToggle.isOpen()')).toEqual(true); + * }); + * + * + * + * @example + * ### ngRef inside scopes + * This example shows how `ngRef` works with child scopes. The `ngRepeat`-ed `myWrapper` components + * are assigned to the scope of `myRoot`, because the `toggles` property has been initialized. + * The repeated `myToggle` components are published to the child scopes created by `ngRepeat`. + * `ngIf` behaves similarly - the assignment of `myToggle` happens in the `ngIf` child scope, + * because the target property has not been initialized on the `myRoot` component controller. + * + * + * + * + * + * + * angular.module('myApp', []) + * .component('myRoot', { + * templateUrl: 'root.html', + * controller: function() { + * this.wrappers = []; // initialize the array so that the wrappers are assigned into the parent scope + * } + * }) + * .component('myToggle', { + * template: 'myToggle', + * transclude: true, + * controller: function ToggleController() { + * var opened = false; + * this.isOpen = function() { return opened; }; + * this.toggle = function() { opened = !opened; }; + * } + * }) + * .component('myWrapper', { + * transclude: true, + * template: 'myWrapper' + + * '
ngRepeatToggle.isOpen(): {{$ctrl.ngRepeatToggle.isOpen() | json}}
' + + * '' + * }); + * + * + * myRoot + * Outer Toggle + *
outerToggle.isOpen(): {{$ctrl.outerToggle.isOpen() | json}}
+ *
wrappers assigned to root
+ *
+ * wrapper.ngRepeatToggle.isOpen(): {{wrapper.ngRepeatToggle.isOpen() | json}} + *
+ * + *
    + *
  • + * ngRepeat + *
    outerToggle.isOpen(): {{$ctrl.outerToggle.isOpen() | json}}
    + * ngRepeat Toggle {{$index + 1}} + *
    • + *
+ * + *
ngIfToggle.isOpen(): {{ngIfToggle.isOpen()}} // This is always undefined because it's + * assigned to the child scope created by ngIf. + *
+ *
+ ngIf + * ngIf Toggle + *
ngIfToggle.isOpen(): {{ngIfToggle.isOpen() | json}}
+ *
outerToggle.isOpen(): {{$ctrl.outerToggle.isOpen() | json}}
+ *
+ * + * + * ul { + * list-style: none; + * padding-left: 0; + * } + * + * li[ng-repeat] { + * background: lightgreen; + * padding: 8px; + * margin: 8px; + * } + * + * [ng-if] { + * background: lightgrey; + * padding: 8px; + * } + * + * my-root { + * background: lightgoldenrodyellow; + * padding: 8px; + * display: block; + * } + * + * my-wrapper { + * background: lightsalmon; + * padding: 8px; + * display: block; + * } + * + * my-toggle { + * background: lightblue; + * padding: 8px; + * display: block; + * } + * + * + * var OuterToggle = function() { + * this.toggle = function() { + * element(by.buttonText('Outer Toggle')).click(); + * }; + * this.isOpen = function() { + * return element.all(by.binding('outerToggle.isOpen()')).first().getText(); + * }; + * }; + * var NgRepeatToggle = function(i) { + * var parent = element.all(by.repeater('(index, value) in [1,2,3]')).get(i - 1); + * this.toggle = function() { + * element(by.buttonText('ngRepeat Toggle ' + i)).click(); + * }; + * this.isOpen = function() { + * return parent.element(by.binding('ngRepeatToggle.isOpen() | json')).getText(); + * }; + * this.isOuterOpen = function() { + * return parent.element(by.binding('outerToggle.isOpen() | json')).getText(); + * }; + * }; + * var NgRepeatToggles = function() { + * var toggles = [1,2,3].map(function(i) { return new NgRepeatToggle(i); }); + * this.forEach = function(fn) { + * toggles.forEach(fn); + * }; + * this.isOuterOpen = function(i) { + * return toggles[i - 1].isOuterOpen(); + * }; + * }; + * var NgIfToggle = function() { + * var parent = element(by.css('[ng-if]')); + * this.toggle = function() { + * element(by.buttonText('ngIf Toggle')).click(); + * }; + * this.isOpen = function() { + * return by.binding('ngIfToggle.isOpen() | json').getText(); + * }; + * this.isOuterOpen = function() { + * return parent.element(by.binding('outerToggle.isOpen() | json')).getText(); + * }; + * }; + * + * it('should toggle the outer toggle', function() { + * var outerToggle = new OuterToggle(); + * expect(outerToggle.isOpen()).toEqual('outerToggle.isOpen(): false'); + * outerToggle.toggle(); + * expect(outerToggle.isOpen()).toEqual('outerToggle.isOpen(): true'); + * }); + * + * it('should toggle all outer toggles', function() { + * var outerToggle = new OuterToggle(); + * var repeatToggles = new NgRepeatToggles(); + * var ifToggle = new NgIfToggle(); + * expect(outerToggle.isOpen()).toEqual('outerToggle.isOpen(): false'); + * expect(repeatToggles.isOuterOpen(1)).toEqual('outerToggle.isOpen(): false'); + * expect(repeatToggles.isOuterOpen(2)).toEqual('outerToggle.isOpen(): false'); + * expect(repeatToggles.isOuterOpen(3)).toEqual('outerToggle.isOpen(): false'); + * expect(ifToggle.isOuterOpen()).toEqual('outerToggle.isOpen(): false'); + * outerToggle.toggle(); + * expect(outerToggle.isOpen()).toEqual('outerToggle.isOpen(): true'); + * expect(repeatToggles.isOuterOpen(1)).toEqual('outerToggle.isOpen(): true'); + * expect(repeatToggles.isOuterOpen(2)).toEqual('outerToggle.isOpen(): true'); + * expect(repeatToggles.isOuterOpen(3)).toEqual('outerToggle.isOpen(): true'); + * expect(ifToggle.isOuterOpen()).toEqual('outerToggle.isOpen(): true'); + * }); + * + * it('should toggle each repeat iteration separately', function() { + * var repeatToggles = new NgRepeatToggles(); + * + * repeatToggles.forEach(function(repeatToggle) { + * expect(repeatToggle.isOpen()).toEqual('ngRepeatToggle.isOpen(): false'); + * expect(repeatToggle.isOuterOpen()).toEqual('outerToggle.isOpen(): false'); + * repeatToggle.toggle(); + * expect(repeatToggle.isOpen()).toEqual('ngRepeatToggle.isOpen(): true'); + * expect(repeatToggle.isOuterOpen()).toEqual('outerToggle.isOpen(): false'); + * }); + * }); + * + * + * + */ + +var ngRefMinErr = minErr('ngRef'); + +var ngRefDirective = ['$parse', function($parse) { + return { + priority: -1, // Needed for compatibility with element transclusion on the same element + restrict: 'A', + compile: function(tElement, tAttrs) { + // Get the expected controller name, converts into "someThing" + var controllerName = directiveNormalize(nodeName_(tElement)); + + // Get the expression for value binding + var getter = $parse(tAttrs.ngRef); + var setter = getter.assign || function() { + throw ngRefMinErr('nonassign', 'Expression in ngRef="{0}" is non-assignable!', tAttrs.ngRef); + }; + + return function(scope, element, attrs) { + var refValue; + + if (attrs.hasOwnProperty('ngRefRead')) { + if (attrs.ngRefRead === '$element') { + refValue = element; + } else { + refValue = element.data('$' + attrs.ngRefRead + 'Controller'); + + if (!refValue) { + throw ngRefMinErr( + 'noctrl', + 'The controller for ngRefRead="{0}" could not be found on ngRef="{1}"', + attrs.ngRefRead, + tAttrs.ngRef + ); + } + } + } else { + refValue = element.data('$' + controllerName + 'Controller'); + } + + refValue = refValue || element; + + setter(scope, refValue); + + // when the element is removed, remove it (nullify it) + element.on('$destroy', function() { + // only remove it if value has not changed, + // because animations (and other procedures) may duplicate elements + if (getter(scope) === refValue) { + setter(scope, null); + } + }); + }; + } + }; +}]; diff --git a/test/helpers/matchers.js b/test/helpers/matchers.js index ac297609e579..5010b212f6d2 100644 --- a/test/helpers/matchers.js +++ b/test/helpers/matchers.js @@ -313,6 +313,7 @@ beforeEach(function() { function generateCompare(isNot) { return function(actual, namespace, code, content) { + var matcher = new MinErrMatcher(isNot, namespace, code, content, { inputType: 'error', expectedAction: 'equal', diff --git a/test/ng/directive/ngRefSpec.js b/test/ng/directive/ngRefSpec.js new file mode 100644 index 000000000000..ef62fae99cad --- /dev/null +++ b/test/ng/directive/ngRefSpec.js @@ -0,0 +1,561 @@ +'use strict'; + +describe('ngRef', function() { + + beforeEach(function() { + jasmine.addMatchers({ + toEqualJq: function(util) { + return { + compare: function(actual, expected) { + // Jquery <= 2.2 objects add a context property that is irrelevant for equality + if (actual && actual.hasOwnProperty('context')) { + delete actual.context; + } + + if (expected && expected.hasOwnProperty('context')) { + delete expected.context; + } + + return { + pass: util.equals(actual, expected) + }; + } + }; + } + }); + }); + + describe('on a component', function() { + + var myComponentController, attributeDirectiveController, $rootScope, $compile; + + beforeEach(module(function($compileProvider) { + $compileProvider.component('myComponent', { + template: 'foo', + controller: function() { + myComponentController = this; + } + }); + + $compileProvider.directive('attributeDirective', function() { + return { + restrict: 'A', + controller: function() { + attributeDirectiveController = this; + } + }; + }); + + })); + + beforeEach(inject(function(_$compile_, _$rootScope_) { + $rootScope = _$rootScope_; + $compile = _$compile_; + })); + + it('should bind in the current scope the controller of a component', function() { + $rootScope.$ctrl = 'undamaged'; + + $compile('')($rootScope); + expect($rootScope.$ctrl).toBe('undamaged'); + expect($rootScope.myComponentRef).toBe(myComponentController); + }); + + it('should throw if the expression is not assignable', function() { + expect(function() { + $compile('')($rootScope); + }).toThrowMinErr('ngRef', 'nonassign', 'Expression in ngRef="\'hello\'" is non-assignable!'); + }); + + it('should work with non:normalized entity name', function() { + $compile('')($rootScope); + expect($rootScope.myComponent1).toBe(myComponentController); + }); + + it('should work with data-non-normalized entity name', function() { + $compile('')($rootScope); + expect($rootScope.myComponent2).toBe(myComponentController); + }); + + it('should work with x-non-normalized entity name', function() { + $compile('')($rootScope); + expect($rootScope.myComponent3).toBe(myComponentController); + }); + + it('should work with data-non-normalized attribute name', function() { + $compile('')($rootScope); + expect($rootScope.myComponent1).toBe(myComponentController); + }); + + it('should work with x-non-normalized attribute name', function() { + $compile('')($rootScope); + expect($rootScope.myComponent2).toBe(myComponentController); + }); + + it('should not bind the controller of an attribute directive', function() { + $compile('')($rootScope); + expect($rootScope.myComponentRef).toBe(myComponentController); + }); + + it('should not leak to parent scopes', function() { + var template = + '
' + + '' + + '
'; + $compile(template)($rootScope); + expect($rootScope.myComponent).toBe(undefined); + }); + + it('should nullify the variable once the component is destroyed', function() { + var template = '
'; + + var element = $compile(template)($rootScope); + expect($rootScope.myComponent).toBe(myComponentController); + + var componentElement = element.children(); + var isolateScope = componentElement.isolateScope(); + componentElement.remove(); + isolateScope.$destroy(); + expect($rootScope.myComponent).toBe(null); + }); + + it('should be compatible with entering/leaving components', inject(function($animate) { + var template = ''; + $rootScope.$ctrl = {}; + var parent = $compile('
')($rootScope); + + var leaving = $compile(template)($rootScope); + var leavingController = myComponentController; + + $animate.enter(leaving, parent); + expect($rootScope.myComponent).toBe(leavingController); + + var entering = $compile(template)($rootScope); + var enteringController = myComponentController; + + $animate.enter(entering, parent); + $animate.leave(leaving, parent); + expect($rootScope.myComponent).toBe(enteringController); + })); + + it('should allow binding to a nested property', function() { + $rootScope.obj = {}; + + $compile('')($rootScope); + expect($rootScope.obj.myComponent).toBe(myComponentController); + }); + + }); + + it('should bind the jqlite wrapped DOM element if there is no component', inject(function($compile, $rootScope) { + + var el = $compile('my text')($rootScope); + + expect($rootScope.mySpan).toEqualJq(el); + expect($rootScope.mySpan[0].textContent).toBe('my text'); + })); + + it('should nullify the expression value if the DOM element is destroyed', inject(function($compile, $rootScope) { + var element = $compile('
my text
')($rootScope); + element.children().remove(); + expect($rootScope.mySpan).toBe(null); + })); + + it('should bind the controller of an element directive', function() { + var myDirectiveController; + + module(function($compileProvider) { + $compileProvider.directive('myDirective', function() { + return { + controller: function() { + myDirectiveController = this; + } + }; + }); + }); + + inject(function($compile, $rootScope) { + $compile('')($rootScope); + + expect($rootScope.myDirective).toBe(myDirectiveController); + }); + }); + + describe('ngRefRead', function() { + + it('should bind the element instead of the controller of a component if ngRefRead="$element" is set', function() { + + module(function($compileProvider) { + + $compileProvider.component('myComponent', { + template: 'my text', + controller: function() {} + }); + }); + + inject(function($compile, $rootScope) { + + var el = $compile('')($rootScope); + expect($rootScope.myEl).toEqualJq(el); + expect($rootScope.myEl[0].textContent).toBe('my text'); + }); + }); + + + it('should bind the element instead an element-directive controller if ngRefRead="$element" is set', function() { + + module(function($compileProvider) { + $compileProvider.directive('myDirective', function() { + return { + restrict: 'E', + template: 'my text', + controller: function() {} + }; + }); + }); + + inject(function($compile, $rootScope) { + var el = $compile('')($rootScope); + + expect($rootScope.myEl).toEqualJq(el); + expect($rootScope.myEl[0].textContent).toBe('my text'); + }); + }); + + + it('should bind an attribute-directive controller if ngRefRead="controllerName" is set', function() { + var attrDirective1Controller; + + module(function($compileProvider) { + $compileProvider.directive('elementDirective', function() { + return { + restrict: 'E', + template: 'my text', + controller: function() {} + }; + }); + + $compileProvider.directive('attributeDirective1', function() { + return { + restrict: 'A', + controller: function() { + attrDirective1Controller = this; + } + }; + }); + + $compileProvider.directive('attributeDirective2', function() { + return { + restrict: 'A', + controller: function() {} + }; + }); + + }); + + inject(function($compile, $rootScope) { + var el = $compile('')($rootScope); + + expect($rootScope.myController).toBe(attrDirective1Controller); + }); + }); + + it('should throw if no controller is found for the ngRefRead value', function() { + + module(function($compileProvider) { + $compileProvider.directive('elementDirective', function() { + return { + restrict: 'E', + template: 'my text', + controller: function() {} + }; + }); + }); + + inject(function($compile, $rootScope) { + + expect(function() { + $compile('')($rootScope); + }).toThrowMinErr('ngRef', 'noctrl', 'The controller for ngRefRead="attribute" could not be found on ngRef="myController"'); + + }); + }); + + }); + + + it('should bind the jqlite element if the controller is on an attribute-directive', function() { + var myDirectiveController; + + module(function($compileProvider) { + $compileProvider.directive('myDirective', function() { + return { + restrict: 'A', + template: 'my text', + controller: function() { + myDirectiveController = this; + } + }; + }); + }); + + inject(function($compile, $rootScope) { + var el = $compile('
')($rootScope); + + expect(myDirectiveController).toBeDefined(); + expect($rootScope.myEl).toEqualJq(el); + expect($rootScope.myEl[0].textContent).toBe('my text'); + }); + }); + + + it('should bind the jqlite element if the controller is on an class-directive', function() { + var myDirectiveController; + + module(function($compileProvider) { + $compileProvider.directive('myDirective', function() { + return { + restrict: 'C', + template: 'my text', + controller: function() { + myDirectiveController = this; + } + }; + }); + }); + + inject(function($compile, $rootScope) { + var el = $compile('
')($rootScope); + + expect(myDirectiveController).toBeDefined(); + expect($rootScope.myEl).toEqualJq(el); + expect($rootScope.myEl[0].textContent).toBe('my text'); + }); + }); + + describe('transclusion', function() { + + it('should work with simple transclusion', function() { + module(function($compileProvider) { + $compileProvider + .component('myComponent', { + transclude: true, + template: '', + controller: function() { + this.text = 'SUCCESS'; + } + }); + }); + + inject(function($compile, $rootScope) { + var template = '{{myComponent.text}}'; + var element = $compile(template)($rootScope); + $rootScope.$apply(); + expect(element.text()).toBe('SUCCESS'); + dealoc(element); + }); + }); + + it('should be compatible with element transclude components', function() { + + module(function($compileProvider) { + $compileProvider + .component('myComponent', { + transclude: 'element', + controller: function($animate, $element, $transclude) { + this.text = 'SUCCESS'; + this.$postLink = function() { + $transclude(function(clone, newScope) { + $animate.enter(clone, $element.parent(), $element); + }); + }; + } + }); + }); + + inject(function($compile, $rootScope) { + var template = + '
' + + '' + + '{{myComponent.text}}' + + '' + + '
'; + var element = $compile(template)($rootScope); + $rootScope.$apply(); + expect(element.text()).toBe('SUCCESS'); + dealoc(element); + }); + }); + + it('should be compatible with ngIf and transclusion on same element', function() { + module(function($compileProvider) { + $compileProvider.component('myComponent', { + template: '', + transclude: true, + controller: function($scope) { + this.text = 'SUCCESS'; + } + }); + }); + + inject(function($compile, $rootScope) { + var template = + '
' + + '' + + '{{myComponent.text}}' + + '' + + '
'; + var element = $compile(template)($rootScope); + + $rootScope.$apply('present = false'); + expect(element.text()).toBe(''); + $rootScope.$apply('present = true'); + expect(element.text()).toBe('SUCCESS'); + $rootScope.$apply('present = false'); + expect(element.text()).toBe(''); + $rootScope.$apply('present = true'); + expect(element.text()).toBe('SUCCESS'); + dealoc(element); + }); + }); + + it('should be compatible with element transclude & destroy components', function() { + var myComponentController; + module(function($compileProvider) { + $compileProvider + .component('myTranscludingComponent', { + transclude: 'element', + controller: function($animate, $element, $transclude) { + myComponentController = this; + + var currentClone, currentScope; + this.transclude = function(text) { + this.text = text; + $transclude(function(clone, newScope) { + currentClone = clone; + currentScope = newScope; + $animate.enter(clone, $element.parent(), $element); + }); + }; + this.destroy = function() { + currentClone.remove(); + currentScope.$destroy(); + }; + } + }); + }); + + inject(function($compile, $rootScope) { + var template = + '
' + + '' + + '{{myComponent.text}}' + + '' + + '
'; + var element = $compile(template)($rootScope); + $rootScope.$apply(); + expect(element.text()).toBe(''); + + myComponentController.transclude('transcludedOk'); + $rootScope.$apply(); + expect(element.text()).toBe('transcludedOk'); + + myComponentController.destroy(); + $rootScope.$apply(); + expect(element.text()).toBe(''); + }); + }); + + it('should be compatible with element transclude directives', function() { + module(function($compileProvider) { + $compileProvider + .directive('myDirective', function($animate) { + return { + transclude: 'element', + controller: function() { + this.text = 'SUCCESS'; + }, + link: function(scope, element, attrs, ctrl, $transclude) { + $transclude(function(clone, newScope) { + $animate.enter(clone, element.parent(), element); + }); + } + }; + }); + }); + + inject(function($compile, $rootScope) { + var template = + '
' + + '' + + '{{myDirective.text}}' + + '' + + '
'; + var element = $compile(template)($rootScope); + $rootScope.$apply(); + expect(element.text()).toBe('SUCCESS'); + dealoc(element); + }); + }); + + }); + + it('should work with components with templates via $http', function() { + module(function($compileProvider) { + $compileProvider.component('httpComponent', { + templateUrl: 'template.html', + controller: function() { + this.me = true; + } + }); + }); + + inject(function($compile, $httpBackend, $rootScope) { + var template = '
'; + var element = $compile(template)($rootScope); + $httpBackend.expect('GET', 'template.html').respond('ok'); + $rootScope.$apply(); + expect($rootScope.controller).toBeUndefined(); + $httpBackend.flush(); + expect($rootScope.controller.me).toBe(true); + dealoc(element); + }); + }); + + + it('should work with ngRepeat-ed components', function() { + var controllers = []; + + module(function($compileProvider) { + $compileProvider.component('myComponent', { + template: 'foo', + controller: function() { + controllers.push(this); + } + }); + }); + + + inject(function($compile, $rootScope) { + $rootScope.elements = [0,1,2,3,4]; + $rootScope.controllers = []; // Initialize the array because ngRepeat creates a child scope + + var template = '
'; + var element = $compile(template)($rootScope); + $rootScope.$apply(); + + expect($rootScope.controllers).toEqual(controllers); + + $rootScope.$apply('elements = []'); + + expect($rootScope.controllers).toEqual([null, null, null, null, null]); + }); + }); + +}); From 0ed36430da8216194e6e228f9e9b5d0a5106bf2b Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Mon, 28 May 2018 01:48:36 +0200 Subject: [PATCH 2/4] docs(ngAnimate): add "animating between value changes" section Add a section which covers use case when users need to animate upon a variable's value changes (not between two states). Refers #16561 Closes #16582 --- src/ngAnimate/module.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/ngAnimate/module.js b/src/ngAnimate/module.js index 0f929f3b0018..d7ef5d873c50 100644 --- a/src/ngAnimate/module.js +++ b/src/ngAnimate/module.js @@ -275,9 +275,22 @@ * .message.ng-enter-prepare { * opacity: 0; * } - * * ``` * + * ### Animating between value changes + * + * Sometimes you need to animate between different expression states, whose values + * don't necessary need to be known or referenced in CSS styles. + * Unless possible with another ["animation aware" directive](#directive-support), that specific + * use case can always be covered with {@link ngAnimate.directive:ngAnimateSwap} as can be seen in + * {@link ngAnimate.directive:ngAnimateSwap#examples this example}. + * + * Note that {@link ngAnimate.directive:ngAnimateSwap} is a *structural directive*, which means it + * creates a new instance of the element (including any other/child directives it may have) and + * links it to a new scope every time *swap* happens. In some cases this might not be desirable + * (e.g. for performance reasons, or when you wish to retain internal state on the original + * element instance). + * * ## JavaScript-based Animations * * ngAnimate also allows for animations to be consumed by JavaScript code. The approach is similar to CSS-based animations (where there is a shared From 03a4782a35e036a6b1b4fc8137fae58f73f3bb6f Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Wed, 6 Jun 2018 15:03:09 +0200 Subject: [PATCH 3/4] feat(errorHandlingConfig): add option to exclude error params from url Specific errors, such as those during nested module loading, can create very long error urls because the error message includes the error stack. These urls create visual clutter in the browser console, are often not clickable, and may be rejected by the docs page because they are simply too long. We've already made improvements to the error display in #16283, which excludes the error url from error parameters, which results in cleaner error messages. Further, modern browsers restrict console message length intelligently. This option can still be useful for older browsers like Internet Explorer, or in general to reduce visual clutter in the console. Closes #14744 Closes #15707 Closes #16283 Closes #16299 Closes #16591 --- src/minErr.js | 19 +++++++++++--- test/minErrSpec.js | 64 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 64 insertions(+), 19 deletions(-) diff --git a/src/minErr.js b/src/minErr.js index a2f0ddc2d544..234d244f544f 100644 --- a/src/minErr.js +++ b/src/minErr.js @@ -7,7 +7,8 @@ */ var minErrConfig = { - objectMaxDepth: 5 + objectMaxDepth: 5, + urlErrorParamsEnabled: true }; /** @@ -30,12 +31,21 @@ var minErrConfig = { * * `objectMaxDepth` **{Number}** - The max depth for stringifying objects. Setting to a * non-positive or non-numeric value, removes the max depth limit. * Default: 5 + * + * * `urlErrorParamsEnabled` **{Boolean}** - Specifies wether the generated error url will + * contain the parameters of the thrown error. Disabling the parameters can be useful if the + * generated error url is very long. + * + * Default: true. When used without argument, it returns the current value. */ function errorHandlingConfig(config) { if (isObject(config)) { if (isDefined(config.objectMaxDepth)) { minErrConfig.objectMaxDepth = isValidObjectMaxDepth(config.objectMaxDepth) ? config.objectMaxDepth : NaN; } + if (isDefined(config.urlErrorParamsEnabled) && isBoolean(config.urlErrorParamsEnabled)) { + minErrConfig.urlErrorParamsEnabled = config.urlErrorParamsEnabled; + } } else { return minErrConfig; } @@ -50,6 +60,7 @@ function isValidObjectMaxDepth(maxDepth) { return isNumber(maxDepth) && maxDepth > 0; } + /** * @description * @@ -113,8 +124,10 @@ function minErr(module, ErrorConstructor) { message += '\n' + url + (module ? module + '/' : '') + code; - for (i = 0, paramPrefix = '?'; i < templateArgs.length; i++, paramPrefix = '&') { - message += paramPrefix + 'p' + i + '=' + encodeURIComponent(templateArgs[i]); + if (minErrConfig.urlErrorParamsEnabled) { + for (i = 0, paramPrefix = '?'; i < templateArgs.length; i++, paramPrefix = '&') { + message += paramPrefix + 'p' + i + '=' + encodeURIComponent(templateArgs[i]); + } } return new ErrorConstructor(message); diff --git a/test/minErrSpec.js b/test/minErrSpec.js index 4319fd88d569..66e018b077c8 100644 --- a/test/minErrSpec.js +++ b/test/minErrSpec.js @@ -2,32 +2,57 @@ describe('errors', function() { var originalObjectMaxDepthInErrorMessage = minErrConfig.objectMaxDepth; + var originalUrlErrorParamsEnabled = minErrConfig.urlErrorParamsEnabled; afterEach(function() { minErrConfig.objectMaxDepth = originalObjectMaxDepthInErrorMessage; + minErrConfig.urlErrorParamsEnabled = originalUrlErrorParamsEnabled; }); describe('errorHandlingConfig', function() { - it('should get default objectMaxDepth', function() { - expect(errorHandlingConfig().objectMaxDepth).toBe(5); - }); + describe('objectMaxDepth',function() { + it('should get default objectMaxDepth', function() { + expect(errorHandlingConfig().objectMaxDepth).toBe(5); + }); + + it('should set objectMaxDepth', function() { + errorHandlingConfig({objectMaxDepth: 3}); + expect(errorHandlingConfig().objectMaxDepth).toBe(3); + }); - it('should set objectMaxDepth', function() { - errorHandlingConfig({objectMaxDepth: 3}); - expect(errorHandlingConfig().objectMaxDepth).toBe(3); + it('should not change objectMaxDepth when undefined is supplied', function() { + errorHandlingConfig({objectMaxDepth: undefined}); + expect(errorHandlingConfig().objectMaxDepth).toBe(originalObjectMaxDepthInErrorMessage); + }); + + they('should set objectMaxDepth to NaN when $prop is supplied', + [NaN, null, true, false, -1, 0], function(maxDepth) { + errorHandlingConfig({objectMaxDepth: maxDepth}); + expect(errorHandlingConfig().objectMaxDepth).toBeNaN(); + } + ); }); - it('should not change objectMaxDepth when undefined is supplied', function() { - errorHandlingConfig({objectMaxDepth: undefined}); - expect(errorHandlingConfig().objectMaxDepth).toBe(originalObjectMaxDepthInErrorMessage); + + describe('urlErrorParamsEnabled',function() { + + it('should get default urlErrorParamsEnabled', function() { + expect(errorHandlingConfig().urlErrorParamsEnabled).toBe(true); + }); + + it('should set urlErrorParamsEnabled', function() { + errorHandlingConfig({urlErrorParamsEnabled: false}); + expect(errorHandlingConfig().urlErrorParamsEnabled).toBe(false); + errorHandlingConfig({urlErrorParamsEnabled: true}); + expect(errorHandlingConfig().urlErrorParamsEnabled).toBe(true); + }); + + it('should not change its value when non-boolean is supplied', function() { + errorHandlingConfig({urlErrorParamsEnabled: 123}); + expect(errorHandlingConfig().urlErrorParamsEnabled).toBe(originalUrlErrorParamsEnabled); + }); }); - they('should set objectMaxDepth to NaN when $prop is supplied', - [NaN, null, true, false, -1, 0], function(maxDepth) { - errorHandlingConfig({objectMaxDepth: maxDepth}); - expect(errorHandlingConfig().objectMaxDepth).toBeNaN(); - } - ); }); describe('minErr', function() { @@ -165,7 +190,6 @@ describe('errors', function() { .toMatch(/^[\s\S]*\?p0=a&p1=b&p2=value%20with%20space$/); }); - it('should strip error reference urls from the error message parameters', function() { var firstError = testError('firstcode', 'longer string and so on'); @@ -177,5 +201,13 @@ describe('errors', function() { '%3A%2F%2Ferrors.angularjs.org%2F%22NG_VERSION_FULL%22%2Ftest%2Ffirstcode'); }); + it('should not generate URL query parameters when urlErrorParamsEnabled is false', function() { + + errorHandlingConfig({urlErrorParamsEnabled: false}); + + expect(testError('acode', 'aproblem', 'a', 'b', 'c').message).toBe('[test:acode] aproblem\n' + + 'https://errors.angularjs.org/"NG_VERSION_FULL"/test/acode'); + }); + }); }); From 84d80be2b4624b9fcfac81204ff047eba108d804 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Wed, 6 Jun 2018 17:40:55 +0200 Subject: [PATCH 4/4] feat(ngMessages): add support for default message add support for showing default message when a truthy value is not matched by an ng-message directive. Closes #12008 Closes #12213 Closes #16587 --- src/ngMessages/messages.js | 176 +++++++++++++++++++++++--------- test/ngMessages/messagesSpec.js | 94 +++++++++++++++++ 2 files changed, 223 insertions(+), 47 deletions(-) diff --git a/src/ngMessages/messages.js b/src/ngMessages/messages.js index 445122b67102..bc0026a14f7c 100644 --- a/src/ngMessages/messages.js +++ b/src/ngMessages/messages.js @@ -18,7 +18,7 @@ var jqLite; * sequencing based on the order of how the messages are defined in the template. * * Currently, the ngMessages module only contains the code for the `ngMessages`, `ngMessagesInclude` - * `ngMessage` and `ngMessageExp` directives. + * `ngMessage`, `ngMessageExp` and `ngMessageDefault` directives. * * ## Usage * The `ngMessages` directive allows keys in a key/value collection to be associated with a child element @@ -257,7 +257,26 @@ var jqLite; * .some-message.ng-leave.ng-leave-active {} * ``` * - * {@link ngAnimate Click here} to learn how to use JavaScript animations or to learn more about ngAnimate. + * {@link ngAnimate See the ngAnimate docs} to learn how to use JavaScript animations or to learn + * more about ngAnimate. + * + * ## Displaying a default message + * If the ngMessages renders no inner ngMessage directive (i.e. when none of the truthy + * keys are matched by a defined message), then it will render a default message + * using the {@link ngMessageDefault} directive. + * Note that matched messages will always take precedence over unmatched messages. That means + * the default message will not be displayed when another message is matched. This is also + * true for `ng-messages-multiple`. + * + * ```html + *
+ *
This field is required
+ *
This field is too short
+ *
This field has an input error
+ *
+ * ``` + * + */ angular.module('ngMessages', [], function initAngularHelpers() { // Access helpers from AngularJS core. @@ -286,8 +305,11 @@ angular.module('ngMessages', [], function initAngularHelpers() { * at a time and this depends on the prioritization of the messages within the template. (This can * be changed by using the `ng-messages-multiple` or `multiple` attribute on the directive container.) * - * A remote template can also be used to promote message reusability and messages can also be - * overridden. + * A remote template can also be used (With {@link ngMessagesInclude}) to promote message + * reusability and messages can also be overridden. + * + * A default message can also be displayed when no `ngMessage` directive is inserted, using the + * {@link ngMessageDefault} directive. * * {@link module:ngMessages Click here} to learn more about `ngMessages` and `ngMessage`. * @@ -298,6 +320,7 @@ angular.module('ngMessages', [], function initAngularHelpers() { * ... * ... * ... + * ... * * * @@ -305,6 +328,7 @@ angular.module('ngMessages', [], function initAngularHelpers() { * ... * ... * ... + * ... * * ``` * @@ -333,6 +357,7 @@ angular.module('ngMessages', [], function initAngularHelpers() { *
You did not enter a field
*
Your field is too short
*
Your field is too long
+ *
This field has an input error
*
* * @@ -370,6 +395,7 @@ angular.module('ngMessages', [], function initAngularHelpers() { var unmatchedMessages = []; var matchedKeys = {}; + var truthyKeys = 0; var messageItem = ctrl.head; var messageFound = false; var totalMessages = 0; @@ -382,13 +408,17 @@ angular.module('ngMessages', [], function initAngularHelpers() { var messageUsed = false; if (!messageFound) { forEach(collection, function(value, key) { - if (!messageUsed && truthy(value) && messageCtrl.test(key)) { - // this is to prevent the same error name from showing up twice - if (matchedKeys[key]) return; - matchedKeys[key] = true; + if (truthy(value) && !messageUsed) { + truthyKeys++; + + if (messageCtrl.test(key)) { + // this is to prevent the same error name from showing up twice + if (matchedKeys[key]) return; + matchedKeys[key] = true; - messageUsed = true; - messageCtrl.attach(); + messageUsed = true; + messageCtrl.attach(); + } } }); } @@ -408,7 +438,16 @@ angular.module('ngMessages', [], function initAngularHelpers() { messageCtrl.detach(); }); - if (unmatchedMessages.length !== totalMessages) { + var messageMatched = unmatchedMessages.length !== totalMessages; + var attachDefault = ctrl.default && !messageMatched && truthyKeys > 0; + + if (attachDefault) { + ctrl.default.attach(); + } else if (ctrl.default) { + ctrl.default.detach(); + } + + if (messageMatched || attachDefault) { $animate.setClass($element, ACTIVE_CLASS, INACTIVE_CLASS); } else { $animate.setClass($element, INACTIVE_CLASS, ACTIVE_CLASS); @@ -428,23 +467,31 @@ angular.module('ngMessages', [], function initAngularHelpers() { } }; - this.register = function(comment, messageCtrl) { - var nextKey = latestKey.toString(); - messages[nextKey] = { - message: messageCtrl - }; - insertMessageNode($element[0], comment, nextKey); - comment.$$ngMessageNode = nextKey; - latestKey++; + this.register = function(comment, messageCtrl, isDefault) { + if (isDefault) { + ctrl.default = messageCtrl; + } else { + var nextKey = latestKey.toString(); + messages[nextKey] = { + message: messageCtrl + }; + insertMessageNode($element[0], comment, nextKey); + comment.$$ngMessageNode = nextKey; + latestKey++; + } ctrl.reRender(); }; - this.deregister = function(comment) { - var key = comment.$$ngMessageNode; - delete comment.$$ngMessageNode; - removeMessageNode($element[0], comment, key); - delete messages[key]; + this.deregister = function(comment, isDefault) { + if (isDefault) { + delete ctrl.default; + } else { + var key = comment.$$ngMessageNode; + delete comment.$$ngMessageNode; + removeMessageNode($element[0], comment, key); + delete messages[key]; + } ctrl.reRender(); }; @@ -647,9 +694,41 @@ angular.module('ngMessages', [], function initAngularHelpers() { * * @param {expression} ngMessageExp|whenExp an expression value corresponding to the message key. */ - .directive('ngMessageExp', ngMessageDirectiveFactory()); + .directive('ngMessageExp', ngMessageDirectiveFactory()) + + /** + * @ngdoc directive + * @name ngMessageDefault + * @restrict AE + * @scope + * + * @description + * `ngMessageDefault` is a directive with the purpose to show and hide a default message for + * {@link ngMessages}, when none of provided messages matches. + * + * More information about using `ngMessageDefault` can be found in the + * {@link module:ngMessages `ngMessages` module documentation}. + * + * @usage + * ```html + * + * + * ... + * ... + * ... + * + * + * + * + * ... + * ... + * ... + * + * + */ + .directive('ngMessageDefault', ngMessageDirectiveFactory(true)); -function ngMessageDirectiveFactory() { +function ngMessageDirectiveFactory(isDefault) { return ['$animate', function($animate) { return { restrict: 'AE', @@ -658,25 +737,28 @@ function ngMessageDirectiveFactory() { terminal: true, require: '^^ngMessages', link: function(scope, element, attrs, ngMessagesCtrl, $transclude) { - var commentNode = element[0]; - - var records; - var staticExp = attrs.ngMessage || attrs.when; - var dynamicExp = attrs.ngMessageExp || attrs.whenExp; - var assignRecords = function(items) { - records = items - ? (isArray(items) - ? items - : items.split(/[\s,]+/)) - : null; - ngMessagesCtrl.reRender(); - }; + var commentNode, records, staticExp, dynamicExp; + + if (!isDefault) { + commentNode = element[0]; + staticExp = attrs.ngMessage || attrs.when; + dynamicExp = attrs.ngMessageExp || attrs.whenExp; + + var assignRecords = function(items) { + records = items + ? (isArray(items) + ? items + : items.split(/[\s,]+/)) + : null; + ngMessagesCtrl.reRender(); + }; - if (dynamicExp) { - assignRecords(scope.$eval(dynamicExp)); - scope.$watchCollection(dynamicExp, assignRecords); - } else { - assignRecords(staticExp); + if (dynamicExp) { + assignRecords(scope.$eval(dynamicExp)); + scope.$watchCollection(dynamicExp, assignRecords); + } else { + assignRecords(staticExp); + } } var currentElement, messageCtrl; @@ -701,7 +783,7 @@ function ngMessageDirectiveFactory() { // If the message element was removed via a call to `detach` then `currentElement` will be null // So this handler only handles cases where something else removed the message element. if (currentElement && currentElement.$$attachId === $$attachId) { - ngMessagesCtrl.deregister(commentNode); + ngMessagesCtrl.deregister(commentNode, isDefault); messageCtrl.detach(); } newScope.$destroy(); @@ -716,14 +798,14 @@ function ngMessageDirectiveFactory() { $animate.leave(elm); } } - }); + }, isDefault); // We need to ensure that this directive deregisters itself when it no longer exists // Normally this is done when the attached element is destroyed; but if this directive // gets removed before we attach the message to the DOM there is nothing to watch // in which case we must deregister when the containing scope is destroyed. scope.$on('$destroy', function() { - ngMessagesCtrl.deregister(commentNode); + ngMessagesCtrl.deregister(commentNode, isDefault); }); } }; diff --git a/test/ngMessages/messagesSpec.js b/test/ngMessages/messagesSpec.js index b86f764f37d0..527a577b1f18 100644 --- a/test/ngMessages/messagesSpec.js +++ b/test/ngMessages/messagesSpec.js @@ -661,6 +661,100 @@ describe('ngMessages', function() { ); + describe('default message', function() { + it('should render a default message when no message matches', inject(function($rootScope, $compile) { + element = $compile('
' + + '
Message is set
' + + '
Default message is set
' + + '
')($rootScope); + $rootScope.$apply(function() { + $rootScope.col = { unexpected: false }; + }); + + $rootScope.$digest(); + + expect(element.text().trim()).toBe(''); + expect(element).not.toHaveClass('ng-active'); + + $rootScope.$apply(function() { + $rootScope.col = { unexpected: true }; + }); + + expect(element.text().trim()).toBe('Default message is set'); + expect(element).toHaveClass('ng-active'); + + $rootScope.$apply(function() { + $rootScope.col = { unexpected: false }; + }); + + expect(element.text().trim()).toBe(''); + expect(element).not.toHaveClass('ng-active'); + + $rootScope.$apply(function() { + $rootScope.col = { val: true, unexpected: true }; + }); + + expect(element.text().trim()).toBe('Message is set'); + expect(element).toHaveClass('ng-active'); + })); + + it('should not render a default message with ng-messages-multiple if another error matches', + inject(function($rootScope, $compile) { + element = $compile('
' + + '
Message is set
' + + '
Other message is set
' + + '
Default message is set
' + + '
')($rootScope); + + expect(element.text().trim()).toBe(''); + + $rootScope.$apply(function() { + $rootScope.col = { val: true, other: false, unexpected: false }; + }); + + expect(element.text().trim()).toBe('Message is set'); + + $rootScope.$apply(function() { + $rootScope.col = { val: true, other: true, unexpected: true }; + }); + + expect(element.text().trim()).toBe('Message is set Other message is set'); + + $rootScope.$apply(function() { + $rootScope.col = { val: false, other: false, unexpected: true }; + }); + + expect(element.text().trim()).toBe('Default message is set'); + }) + ); + + it('should handle a default message with ngIf', inject(function($rootScope, $compile) { + element = $compile('
' + + '
Message is set
' + + '
Default message is set
' + + '
')($rootScope); + $rootScope.default = true; + $rootScope.col = {unexpected: true}; + $rootScope.$digest(); + + expect(element.text().trim()).toBe('Default message is set'); + + $rootScope.$apply('default = false'); + + expect(element.text().trim()).toBe(''); + + $rootScope.$apply('default = true'); + + expect(element.text().trim()).toBe('Default message is set'); + + $rootScope.$apply(function() { + $rootScope.col = { val: true }; + }); + + expect(element.text().trim()).toBe('Message is set'); + })); + }); + describe('when including templates', function() { they('should work with a dynamic collection model which is managed by ngRepeat', {'
': '
' +