From 89884c5bfaad5e38a2f29fc98228ac1c0fc0c54e Mon Sep 17 00:00:00 2001 From: Joel Kang Date: Thu, 19 May 2016 15:05:52 -0700 Subject: [PATCH 1/3] [Glimmer2] Add support for target actions and migrate tests --- packages/ember-glimmer/lib/component.js | 2 + .../lib/ember-views/target-action-support.js | 223 ++++++++ .../lib/syntax/curly-component.js | 8 + .../components/dynamic-components-test.js | 2 +- .../components/target-action-test.js | 536 ++++++++++++++++++ .../tests/utils/abstract-test-case.js | 14 + 6 files changed, 784 insertions(+), 1 deletion(-) create mode 100644 packages/ember-glimmer/lib/ember-views/target-action-support.js create mode 100644 packages/ember-glimmer/tests/integration/components/target-action-test.js diff --git a/packages/ember-glimmer/lib/component.js b/packages/ember-glimmer/lib/component.js index 389cd86d9d5..6a67b7b0145 100644 --- a/packages/ember-glimmer/lib/component.js +++ b/packages/ember-glimmer/lib/component.js @@ -5,6 +5,7 @@ import ViewStateSupport from 'ember-views/mixins/view_state_support'; import InstrumentationSupport from 'ember-views/mixins/instrumentation_support'; import AriaRoleSupport from 'ember-views/mixins/aria_role_support'; import ViewMixin from 'ember-views/mixins/view_support'; +import TargetActionSupport from './ember-views/target-action-support'; import EmberView from 'ember-views/views/view'; import symbol from 'ember-metal/symbol'; import EmptyObject from 'ember-metal/empty_object'; @@ -36,6 +37,7 @@ const Component = CoreView.extend( ClassNamesSupport, InstrumentationSupport, AriaRoleSupport, + TargetActionSupport, ViewMixin, { isComponent: true, layoutName: null, diff --git a/packages/ember-glimmer/lib/ember-views/target-action-support.js b/packages/ember-glimmer/lib/ember-views/target-action-support.js new file mode 100644 index 00000000000..1614538dc24 --- /dev/null +++ b/packages/ember-glimmer/lib/ember-views/target-action-support.js @@ -0,0 +1,223 @@ +import { Mixin } from 'ember-metal/mixin'; +import TargetActionSupport from 'ember-runtime/mixins/target_action_support'; +import alias from 'ember-metal/alias'; +import { computed } from 'ember-metal/computed'; +import { assert } from 'ember-metal/debug'; +import { inspect } from 'ember-metal/utils'; +import isNone from 'ember-metal/is_none'; +import { get } from 'ember-metal/property_get'; +import { ARGS } from '../component'; + +/** +`Ember.ComponentTargetActionSupport` is a mixin that can be included in a +component class to add a `triggerAction` method with semantics similar to +the Handlebars `{{action}}` helper. By default the action's target is the +component itself, or the controller of the component if it is an outlet. + +Note: In normal Ember usage, the `{{action}}` helper is usually the best +choice. This mixin is most often useful when you are doing more complex +event handling in custom component subclasses. + +For example: + +```javascript +App.SaveButtonComponent = Ember.Component.extend(Ember.ComponentTargetActionSupport, { + action: 'save', + click: function() { + this.triggerAction(); // Sends the `save` action, along with the current context + // to the current controller + } +}); +``` + +The `action` can be provided as properties of an optional object argument +to `triggerAction` as well. + +```javascript +App.SaveButtonComponent = Ember.Component.extend(Ember.ComponentTargetActionSupport, { + click: function() { + this.triggerAction({ + action: 'save' + }); // Sends the `save` action, along with the current context + // to the current controller + } +}); +``` + +@class ComponentTargetActionSupport +@namespace Ember +@extends Ember.TargetActionSupport +@private +*/ +export default Mixin.create(TargetActionSupport, { + /** + @property target + @private + */ + target: computed('outletState', function () { + let outletState = get(this, 'outletState'); + return outletState ? getCurrentOutletState(outletState).render.controller : undefined; + }), + + // @override + send(actionName, ...args) { + var target; + var action = this.actions && this.actions[actionName]; + + if (action) { + var shouldBubble = action.apply(this, args) === true; + if (!shouldBubble) { return; } + } + + if (target = get(this, 'target')) { + assert( + 'The `target` for ' + this + ' (' + target + + ') does not have a `send` method', + typeof target.send === 'function' + ); + target.send(...arguments); + } else { + if (!action) { + throw new Error(inspect(this) + ' had no action handler for: ' + actionName); + } + } + }, + /** + TODO: This looks like it's not even used by the view layer. Deprecate and remove? + @property actionContext + @private + */ + actionContext: alias('target'), + /** + If the component is currently inserted into the DOM of a parent component, this + property will point to the parent's controller, if present, or the parent itself. + + @property targetObject + @type Ember.Controller + @default null + @private + */ + targetObject: computed('parentView', function(key) { + let targetObjectAttr = get(this, '_targetObject'); + if (targetObjectAttr) { + return targetObjectAttr; + } + let parentComponent = get(this, 'parentView'); + if (parentComponent) { + let outletState = get(parentComponent, 'outletState'); + return outletState ? getCurrentOutletState(outletState).render.controller : parentComponent; + } + return null; + }), + + /** + Calls a action passed to a component. + For example a component for playing or pausing music may translate click events + into action notifications of "play" or "stop" depending on some internal state + of the component: + ```javascript + // app/components/play-button.js + export default Ember.Component.extend({ + click() { + if (this.get('isPlaying')) { + this.sendAction('play'); + } else { + this.sendAction('stop'); + } + } + }); + ``` + The actions "play" and "stop" must be passed to this `play-button` component: + ```handlebars + {{! app/templates/application.hbs }} + {{play-button play=(action "musicStarted") stop=(action "musicStopped")}} + ``` + When the component receives a browser `click` event it translate this + interaction into application-specific semantics ("play" or "stop") and + calls the specified action. + ```javascript + // app/controller/application.js + export default Ember.Controller.extend({ + actions: { + musicStarted() { + // called when the play button is clicked + // and the music started playing + }, + musicStopped() { + // called when the play button is clicked + // and the music stopped playing + } + } + }); + ``` + If no action is passed to `sendAction` a default name of "action" + is assumed. + ```javascript + // app/components/next-button.js + export default Ember.Component.extend({ + click() { + this.sendAction(); + } + }); + ``` + ```handlebars + {{! app/templates/application.hbs }} + {{next-button action=(action "playNextSongInAlbum")}} + ``` + ```javascript + // app/controllers/application.js + App.ApplicationController = Ember.Controller.extend({ + actions: { + playNextSongInAlbum() { + ... + } + } + }); + ``` + @method sendAction + @param [action] {String} the action to call + @param [params] {*} arguments for the action + @public + */ + sendAction(action, ...contexts) { + let actionName; + + // Send the default action + if (action === undefined) { + action = 'action'; + } + actionName = get(this, `${ARGS}.${action}`) || get(this, action); + actionName = validateAction(this, actionName); + + // If no action name for that action could be found, just abort. + if (actionName === undefined) { return; } + + if (typeof actionName === 'function') { + actionName(...contexts); + } else { + this.triggerAction({ + action: actionName, + actionContext: contexts + }); + } + } +}); + +function getCurrentOutletState(outletState) { + let { outlets, render } = outletState; + return (Object.keys(outlets).length > 0) ? getCurrentOutletState(outlets[render.outlet]) : outletState; +} + +function validateAction(component, actionName) { + //TODO: How to check if this is a reference? + if (actionName && typeof actionName.value === 'function') { + return actionName.value(); + } + + assert( + 'The default action was triggered on the component ' + component.toString() + + ', but the action name (' + actionName + ') was not a string.', + isNone(actionName) || typeof actionName === 'string' || typeof actionName === 'function' + ); + return actionName; +} diff --git a/packages/ember-glimmer/lib/syntax/curly-component.js b/packages/ember-glimmer/lib/syntax/curly-component.js index 0551a0d86c5..b803dd21c62 100644 --- a/packages/ember-glimmer/lib/syntax/curly-component.js +++ b/packages/ember-glimmer/lib/syntax/curly-component.js @@ -37,6 +37,13 @@ function applyAttributeBindings(attributeBindings, component, operations) { } } +function privatizeTargetObject(args, props) { + if (args.named.has('targetObject')) { + props._targetObject = props.targetObject; + delete props.targetObject; + } +} + export class CurlyComponentSyntax extends StatementSyntax { constructor({ args, definition, templates }) { super(); @@ -69,6 +76,7 @@ class CurlyComponentManager { let { attrs, props } = processedArgs.value(); aliasIdToElementId(args, props); + privatizeTargetObject(args, props); props.renderer = parentView.renderer; props[HAS_BLOCK] = hasBlock; diff --git a/packages/ember-glimmer/tests/integration/components/dynamic-components-test.js b/packages/ember-glimmer/tests/integration/components/dynamic-components-test.js index 4d160ef025b..5f1773c5cf5 100644 --- a/packages/ember-glimmer/tests/integration/components/dynamic-components-test.js +++ b/packages/ember-glimmer/tests/integration/components/dynamic-components-test.js @@ -399,7 +399,7 @@ moduleFor('Components test: dynamic components', class extends RenderingTest { this.assertText('foo-bar Caracas Caracas arepas!'); } - ['@htmlbars component helper with actions'](assert) { + ['@test component helper with actions'](assert) { this.registerComponent('inner-component', { template: 'inner-component {{yield}}', ComponentClass: Component.extend({ diff --git a/packages/ember-glimmer/tests/integration/components/target-action-test.js b/packages/ember-glimmer/tests/integration/components/target-action-test.js new file mode 100644 index 00000000000..9789f861251 --- /dev/null +++ b/packages/ember-glimmer/tests/integration/components/target-action-test.js @@ -0,0 +1,536 @@ +import { moduleFor, RenderingTest, ApplicationTest } from '../../utils/test-case'; +import { set } from 'ember-metal/property_set'; +import { Component } from '../../utils/helpers'; +import assign from 'ember-metal/assign'; +import Controller from 'ember-runtime/controllers/controller'; +import { Mixin } from 'ember-metal/mixin'; +import Route from 'ember-routing/system/route'; +import EmberObject from 'ember-runtime/system/object'; + +moduleFor('Components test: sendAction', class extends RenderingTest { + + constructor() { + super(); + this.actionCounts = {}; + this.sendCount = 0; + this.actionArguments = null; + + var self = this; + + this.registerComponent('action-delegate', { + ComponentClass: Component.extend({ + init() { + this._super(); + self.delegate = this; + } + }) + }); + } + + renderDelegate(template = '{{action-delegate}}', context = {}) { + let root = this; + context = assign(context, { + send(actionName) { + root.sendCount++; + root.actionCounts[actionName] = root.actionCounts[actionName] || 0; + root.actionCounts[actionName]++; + root.actionArguments = Array.prototype.slice.call(arguments, 1); + } + }); + this.render(template, context); + } + + assertSendCount(count) { + this.assert.equal(this.sendCount, count, `Send was called ${count} time(s)`); + } + + assertNamedSendCount(actionName, count) { + this.assert.equal(this.actionCounts[actionName], count, `An action named '${actionName}' was sent ${count} times`); + } + + assertSentWithArgs(expected, message = 'arguments were sent with the action') { + this.assert.deepEqual(this.actionArguments, expected, message); + } + + ['@test Calling sendAction on a component without an action defined does nothing']() { + this.renderDelegate(); + + this.runTask(() => this.delegate.sendAction()); + + this.assertSendCount(0); + } + + ['@test Calling sendAction on a component with an action defined calls send on the controller']() { + this.renderDelegate(); + + this.runTask(() => { + set(this.delegate, 'action', 'addItem'); + this.delegate.sendAction(); + }); + + this.assertSendCount(1); + this.assertNamedSendCount('addItem', 1); + } + + ['@test Calling sendAction on a component with a function calls the function']() { + this.assert.expect(1); + + this.renderDelegate(); + + this.runTask(() => { + set(this.delegate, 'action', () => this.assert.ok(true, 'function is called')); + this.delegate.sendAction(); + }); + } + + ['@test Calling sendAction on a component with a function calls the function with arguments']() { + this.assert.expect(1); + let argument = {}; + + this.renderDelegate(); + + this.runTask(() => { + set(this.delegate, 'action', (actualArgument) => { + this.assert.deepEqual(argument, actualArgument, 'argument is passed'); + }); + this.delegate.sendAction('action', argument); + }); + } + + ['@glimmer Calling sendAction on a component with a reference attr calls the function with arguments']() { + this.renderDelegate('{{action-delegate playing=playing}}', { + playing: null + }); + + this.runTask(() => this.delegate.sendAction()); + + this.assertSendCount(0); + + this.runTask(() => { + set(this.context, 'playing', 'didStartPlaying'); + this.delegate.sendAction('playing'); + }); + + this.assertSendCount(1); + this.assertNamedSendCount('didStartPlaying', 1); + } + + ['@htmlbars Calling sendAction on a component with a {{mut}} attr calls the function with arguments']() { + this.renderDelegate('{{action-delegate playing=(mut playing)}}', { + playing: null + }); + + this.runTask(() => this.delegate.sendAction('playing')); + + this.assertSendCount(0); + + this.runTask(() => this.delegate.attrs.playing.update('didStartPlaying')); + this.runTask(() => this.delegate.sendAction('playing')); + + this.assertSendCount(1); + this.assertNamedSendCount('didStartPlaying', 1); + } + + ['@test Calling sendAction with a named action uses the component\'s property as the action name']() { + this.renderDelegate(); + + let component = this.delegate; + + this.runTask(() => { + set(this.delegate, 'playing', 'didStartPlaying'); + component.sendAction('playing'); + }); + + this.assertSendCount(1); + this.assertNamedSendCount('didStartPlaying', 1); + + this.runTask(() => component.sendAction('playing')); + + this.assertSendCount(2); + this.assertNamedSendCount('didStartPlaying', 2); + + this.runTask(() => { + set(component, 'action', 'didDoSomeBusiness'); + component.sendAction(); + }); + + this.assertSendCount(3); + this.assertNamedSendCount('didDoSomeBusiness', 1); + } + + ['@test Calling sendAction when the action name is not a string raises an exception']() { + this.renderDelegate(); + + this.runTask(() => { + set(this.delegate, 'action', {}); + set(this.delegate, 'playing', {}); + }); + + this.assert.throws(() => this.delegate.sendAction()); + this.assert.throws(() => this.delegate.sendAction('playing')); + } + + ['@test Calling sendAction on a component with contexts']() { + this.renderDelegate(); + + let testContext = { song: 'She Broke My Ember' }; + let firstContext = { song: 'She Broke My Ember' }; + let secondContext = { song: 'My Achey Breaky Ember' }; + + this.runTask(() => { + set(this.delegate, 'playing', 'didStartPlaying'); + this.delegate.sendAction('playing', testContext); + }); + + this.assertSendCount(1); + this.assertNamedSendCount('didStartPlaying', 1); + this.assertSentWithArgs([testContext], 'context was sent with the action'); + + this.runTask(() => { + this.delegate.sendAction('playing', firstContext, secondContext); + }); + + this.assertSendCount(2); + this.assertNamedSendCount('didStartPlaying', 2); + this.assertSentWithArgs([firstContext, secondContext], 'multiple contexts were sent to the action'); + } + + ['@test Calling sendAction on a component with a targetObject specified'](assert) { + assert.expect(3); + this.registerComponent('wrapper-component', { + ComponentClass: Component.extend({ + mockTarget: EmberObject.create({ + send(actionName, actionContext) { + assert.equal(actionName, 'poke', 'targetObject received the sendAction request'); + assert.equal(actionContext, 'me', 'targetObject received the arguments for the sendAction request'); + } + }) + }), + template: '{{action-delegate poke="poke" targetObject=mockTarget}}' + }); + + this.render('{{wrapper-component}}'); + + this.runTask(() => this.delegate.sendAction('poke', 'me')); + + this.assertSendCount(0); + } +}); + +moduleFor('Components test: sendAction to a controller', class extends ApplicationTest { + + ['@test sendAction should trigger an action on the parent component\'s controller if it exists'](assert) { + assert.expect(7); + + let component; + + this.router.map(function () { + this.route('withNestedController', function () { + this.route('nestedWithController'); + this.route('nestedWithoutController'); + }); + this.route('withController'); + this.route('withoutController'); + }); + + this.registerComponent('foo-bar', { + ComponentClass: Component.extend({ + init() { + this._super(...arguments); + component = this; + } + }) + }); + + this.registerTemplate('withNestedController', '{{foo-bar poke="poke"}}{{outlet}}'); + this.registerTemplate('withController', '{{foo-bar poke="poke"}}'); + this.registerTemplate('withoutController', '{{foo-bar poke="poke"}}'); + this.registerTemplate('withNestedController.nestedWithController', '{{foo-bar poke="poke"}}'); + this.registerTemplate('withNestedController.nestedWithoutController', '{{foo-bar poke="poke"}}'); + + this.registerController('withController', Controller.extend({ + send(actionName, actionContext) { + assert.equal(actionName, 'poke', 'send() method was invoked from a top level controller'); + assert.equal(actionContext, 'top', 'action arguments were passed into the top level controller'); + } + })); + + this.registerController('withNestedControllerNestedWithController', Controller.extend({ + send(actionName, actionContext) { + assert.equal(actionName, 'poke', 'send() method was invoked from a nested controller'); + assert.equal(actionContext, 'nested', 'action arguments were passed into the nested controller'); + } + })); + + this.registerRoute('withController', Route); + this.registerRoute('withNestedController', Route); + this.registerRoute('withNestedController.nestedWithController', Route); + this.registerRoute('withNestedController.nestedWithOutController', Route); + this.registerRoute('withoutController', Route); + + return this.visit('/withController') + .then(() => component.sendAction('poke', 'top')) + .then(() => this.visit('withoutController')) + .then(() => assert.throws(() => component.sendAction('poke'))) + // withNestedController.index does not have a controller specified, so it should throw + .then(() => this.visit('/withNestedController')) + .then(() => assert.throws(() => component.sendAction('poke'))) + .then(() => this.visit('/withNestedController/nestedWithController')) + .then(() => component.sendAction('poke', 'nested')) + .then(() => this.visit('/withNestedController/nestedWithoutController')) + .then(() => assert.throws(() => component.sendAction('poke'))); + } + + ['@test sendAction should not trigger an action an outlet\'s controller if a parent component handles it'](assert) { + assert.expect(1); + + let component; + + this.registerComponent('x-parent', { + ComponentClass: Component.extend({ + actions: { + poke() { + assert.ok(true, 'parent component handled the aciton'); + } + } + }), + template: '{{x-child poke="poke"}}' + }); + + this.registerComponent('x-child', { + ComponentClass: Component.extend({ + init() { + this._super(...arguments); + component = this; + } + }) + }); + + this.registerTemplate('application', '{{x-parent}}'); + this.registerController('application', Controller.extend({ + send(actionName) { + throw new Error('controller action should not be called'); + } + })); + + return this.visit('/').then(() => component.sendAction('poke')); + } + +}); + +moduleFor('Components test: sendAction of a closure action', class extends RenderingTest { + + ['@test action should be called'](assert) { + this.assert.expect(1); + let component; + + this.registerComponent('inner-component', { + ComponentClass: Component.extend({ + init() { + this._super(...arguments); + component = this; + } + }), + template: 'inner' + }); + + this.registerComponent('outer-component', { + ComponentClass: Component.extend({ + outerSubmit() { + assert.ok(true, 'outerSubmit called'); + } + }), + template: '{{inner-component submitAction=(action outerSubmit)}}' + }); + + this.render('{{outer-component}}'); + + this.runTask(() => component.sendAction('submitAction')); + } + + ['@test contexts passed to sendAction are appended to the bound arguments on a closure action']() { + let first = 'mitch'; + let second = 'martin'; + let third = 'matt'; + let fourth = 'wacky wycats'; + + let innerComponent; + let actualArgs; + + this.registerComponent('inner-component', { + ComponentClass: Component.extend({ + init() { + this._super(...arguments); + innerComponent = this; + } + }), + template: 'inner' + }); + + this.registerComponent('outer-component', { + ComponentClass: Component.extend({ + third, + actions: { + outerSubmit() { + actualArgs = [...arguments]; + } + } + }), + template: `{{inner-component innerSubmit=(action (action "outerSubmit" "${first}") "${second}" third)}}` + }); + + this.render('{{outer-component}}'); + + this.runTask(() => innerComponent.sendAction('innerSubmit', fourth)); + + this.assert.deepEqual(actualArgs, [first, second, third, fourth], 'action has the correct args'); + } +}); + +moduleFor('Components test: send', class extends RenderingTest { + ['@test sending to undefined actions triggers an error'](assert) { + assert.expect(2); + + let component; + + this.registerComponent('foo-bar', { + ComponentClass: Component.extend({ + init() { + this._super(); + component = this; + }, + actions: { + foo(message) { + assert.equal('bar', message); + } + } + }) + }); + + this.render('{{foo-bar}}'); + + this.runTask(() => component.send('foo', 'bar')); + + assert.throws(function () { + component.send('baz', 'bar'); + }, /had no action handler for: baz/); + } + + ['@test `send` will call send from a target if it is defined']() { + let component; + let target = { + send: (message, payload) => { + this.assert.equal('foo', message); + this.assert.equal('baz', payload); + } + }; + + this.registerComponent('foo-bar', { + ComponentClass: Component.extend({ + init() { + this._super(); + component = this; + }, + target + }) + }); + + this.render('{{foo-bar}}'); + + this.runTask(() => component.send('foo', 'baz')); + } + + ['@test a handled action can be bubbled to the target for continued processing']() { + this.assert.expect(2); + + let component; + + this.registerComponent('foo-bar', { + ComponentClass: Component.extend({ + init() { + this._super(...arguments); + component = this; + }, + actions: { + poke: () => { + this.assert.ok(true, 'component action called'); + return true; + } + }, + target: Controller.extend({ + actions: { + poke: () => { + this.assert.ok(true, 'action bubbled to controller'); + } + } + }).create() + }) + }); + + this.render('{{foo-bar poke="poke"}}'); + + this.runTask(() => component.send('poke')); + } + + ['@test action can be handled by a superclass\' actions object'](assert) { + this.assert.expect(4); + + let component; + + let SuperComponent = Component.extend({ + actions: { + foo() { + assert.ok(true, 'foo'); + }, + bar(msg) { + assert.equal(msg, 'HELLO'); + } + } + }); + + let BarViewMixin = Mixin.create({ + actions: { + bar(msg) { + assert.equal(msg, 'HELLO'); + this._super(msg); + } + } + }); + + this.registerComponent('x-index', { + ComponentClass: SuperComponent.extend(BarViewMixin, { + init() { + this._super(...arguments); + component = this; + }, + actions: { + baz() { + assert.ok(true, 'baz'); + } + } + }) + }); + + this.render('{{x-index}}'); + + this.runTask(() => { + component.send('foo'); + component.send('bar', 'HELLO'); + component.send('baz'); + }); + } + + ['@test actions cannot be provided at create time'](assert) { + assert.throws(() => Component.create({ + actions: { + foo() { + assert.ok(true, 'foo'); + } + } + })); + // but should be OK on an object that doesn't mix in Ember.ActionHandler + EmberObject.create({ + actions: ['foo'] + }); + } +}); diff --git a/packages/ember-glimmer/tests/utils/abstract-test-case.js b/packages/ember-glimmer/tests/utils/abstract-test-case.js index 0767d169a74..53ea09c7f03 100644 --- a/packages/ember-glimmer/tests/utils/abstract-test-case.js +++ b/packages/ember-glimmer/tests/utils/abstract-test-case.js @@ -313,6 +313,20 @@ export class ApplicationTest extends TestCase { registerController(name, controller) { this.application.register(`controller:${name}`, controller); } + + registerComponent(name, { ComponentClass = null, template = null }) { + let { application } = this; + + if (ComponentClass) { + application.register(`component:${name}`, ComponentClass); + } + + if (typeof template === 'string') { + application.register(`template:components/${name}`, compile(template, { + moduleName: `components/${name}` + })); + } + } } export class RenderingTest extends TestCase { From 06948d4ce8c61f12f5beaf6f35e17cb25dbf89d3 Mon Sep 17 00:00:00 2001 From: Joel Kang Date: Thu, 19 May 2016 15:14:44 -0700 Subject: [PATCH 2/3] Add targetObject privatization to update too and remove targetObject test since it is private --- .../lib/ember-views/target-action-support.js | 11 ++--- .../lib/syntax/curly-component.js | 9 ++-- .../components/target-action-test.js | 43 +++++++------------ 3 files changed, 26 insertions(+), 37 deletions(-) diff --git a/packages/ember-glimmer/lib/ember-views/target-action-support.js b/packages/ember-glimmer/lib/ember-views/target-action-support.js index 1614538dc24..5bc20184107 100644 --- a/packages/ember-glimmer/lib/ember-views/target-action-support.js +++ b/packages/ember-glimmer/lib/ember-views/target-action-support.js @@ -54,15 +54,15 @@ export default Mixin.create(TargetActionSupport, { @property target @private */ - target: computed('outletState', function () { + target: computed(function () { let outletState = get(this, 'outletState'); return outletState ? getCurrentOutletState(outletState).render.controller : undefined; }), // @override send(actionName, ...args) { - var target; - var action = this.actions && this.actions[actionName]; + let target; + let action = this.actions && this.actions[actionName]; if (action) { var shouldBubble = action.apply(this, args) === true; @@ -98,10 +98,7 @@ export default Mixin.create(TargetActionSupport, { @private */ targetObject: computed('parentView', function(key) { - let targetObjectAttr = get(this, '_targetObject'); - if (targetObjectAttr) { - return targetObjectAttr; - } + if (this._targetObject) { return this._targetObject; } let parentComponent = get(this, 'parentView'); if (parentComponent) { let outletState = get(parentComponent, 'outletState'); diff --git a/packages/ember-glimmer/lib/syntax/curly-component.js b/packages/ember-glimmer/lib/syntax/curly-component.js index b803dd21c62..b728a7e467d 100644 --- a/packages/ember-glimmer/lib/syntax/curly-component.js +++ b/packages/ember-glimmer/lib/syntax/curly-component.js @@ -37,8 +37,10 @@ function applyAttributeBindings(attributeBindings, component, operations) { } } -function privatizeTargetObject(args, props) { - if (args.named.has('targetObject')) { +// Use `_targetObject` to avoid stomping on a CP +// that exists in the component +function privatizeTargetObject(props) { + if (props.targetObject) { props._targetObject = props.targetObject; delete props.targetObject; } @@ -76,7 +78,7 @@ class CurlyComponentManager { let { attrs, props } = processedArgs.value(); aliasIdToElementId(args, props); - privatizeTargetObject(args, props); + privatizeTargetObject(props); props.renderer = parentView.renderer; props[HAS_BLOCK] = hasBlock; @@ -206,6 +208,7 @@ class CurlyComponentManager { bucket.argsRevision = args.tag.value(); let { attrs, props } = args.value(); + privatizeTargetObject(props); let oldAttrs = component.attrs; let newAttrs = attrs; diff --git a/packages/ember-glimmer/tests/integration/components/target-action-test.js b/packages/ember-glimmer/tests/integration/components/target-action-test.js index 9789f861251..9dd28c7004b 100644 --- a/packages/ember-glimmer/tests/integration/components/target-action-test.js +++ b/packages/ember-glimmer/tests/integration/components/target-action-test.js @@ -166,8 +166,8 @@ moduleFor('Components test: sendAction', class extends RenderingTest { set(this.delegate, 'playing', {}); }); - this.assert.throws(() => this.delegate.sendAction()); - this.assert.throws(() => this.delegate.sendAction('playing')); + expectAssertion(() => this.delegate.sendAction()); + expectAssertion(() => this.delegate.sendAction('playing')); } ['@test Calling sendAction on a component with contexts']() { @@ -195,26 +195,6 @@ moduleFor('Components test: sendAction', class extends RenderingTest { this.assertSentWithArgs([firstContext, secondContext], 'multiple contexts were sent to the action'); } - ['@test Calling sendAction on a component with a targetObject specified'](assert) { - assert.expect(3); - this.registerComponent('wrapper-component', { - ComponentClass: Component.extend({ - mockTarget: EmberObject.create({ - send(actionName, actionContext) { - assert.equal(actionName, 'poke', 'targetObject received the sendAction request'); - assert.equal(actionContext, 'me', 'targetObject received the arguments for the sendAction request'); - } - }) - }), - template: '{{action-delegate poke="poke" targetObject=mockTarget}}' - }); - - this.render('{{wrapper-component}}'); - - this.runTask(() => this.delegate.sendAction('poke', 'me')); - - this.assertSendCount(0); - } }); moduleFor('Components test: sendAction to a controller', class extends ApplicationTest { @@ -268,17 +248,26 @@ moduleFor('Components test: sendAction to a controller', class extends Applicati this.registerRoute('withNestedController.nestedWithOutController', Route); this.registerRoute('withoutController', Route); + function expectSendActionToThrow() { + let fn = () => component.sendAction('poke'); + if (EmberDev && EmberDev.runningProdBuild) { + expectAssertion(fn); + } else { + assert.throws(fn); + } + } + return this.visit('/withController') .then(() => component.sendAction('poke', 'top')) .then(() => this.visit('withoutController')) - .then(() => assert.throws(() => component.sendAction('poke'))) + .then(expectSendActionToThrow) // withNestedController.index does not have a controller specified, so it should throw .then(() => this.visit('/withNestedController')) - .then(() => assert.throws(() => component.sendAction('poke'))) + .then(expectSendActionToThrow) .then(() => this.visit('/withNestedController/nestedWithController')) .then(() => component.sendAction('poke', 'nested')) .then(() => this.visit('/withNestedController/nestedWithoutController')) - .then(() => assert.throws(() => component.sendAction('poke'))); + .then(expectSendActionToThrow); } ['@test sendAction should not trigger an action an outlet\'s controller if a parent component handles it'](assert) { @@ -411,7 +400,7 @@ moduleFor('Components test: send', class extends RenderingTest { this.runTask(() => component.send('foo', 'bar')); - assert.throws(function () { + expectAssertion(function () { component.send('baz', 'bar'); }, /had no action handler for: baz/); } @@ -521,7 +510,7 @@ moduleFor('Components test: send', class extends RenderingTest { } ['@test actions cannot be provided at create time'](assert) { - assert.throws(() => Component.create({ + expectAssertion(() => Component.create({ actions: { foo() { assert.ok(true, 'foo'); From 0378e2a9fd5ebffa5a243ac97e1d1e31f041b201 Mon Sep 17 00:00:00 2001 From: Joel Kang Date: Fri, 20 May 2016 20:22:26 -0700 Subject: [PATCH 3/3] Make view layer send() method user asser instead of throwing to allow tests to properly expect it --- .../lib/ember-views/target-action-support.js | 9 +-- .../lib/syntax/curly-component.js | 2 +- .../components/target-action-test.js | 72 +++++++++++-------- .../tests/utils/abstract-test-case.js | 14 ---- 4 files changed, 46 insertions(+), 51 deletions(-) diff --git a/packages/ember-glimmer/lib/ember-views/target-action-support.js b/packages/ember-glimmer/lib/ember-views/target-action-support.js index 5bc20184107..3f65d8cc169 100644 --- a/packages/ember-glimmer/lib/ember-views/target-action-support.js +++ b/packages/ember-glimmer/lib/ember-views/target-action-support.js @@ -71,16 +71,14 @@ export default Mixin.create(TargetActionSupport, { if (target = get(this, 'target')) { assert( - 'The `target` for ' + this + ' (' + target + + 'The `target` for ' + inspect(this) + ' (' + target + ') does not have a `send` method', typeof target.send === 'function' ); target.send(...arguments); - } else { - if (!action) { - throw new Error(inspect(this) + ' had no action handler for: ' + actionName); - } + return; } + assert(`${inspect(this)} had no action handler for: ${actionName}`, action); }, /** TODO: This looks like it's not even used by the view layer. Deprecate and remove? @@ -106,7 +104,6 @@ export default Mixin.create(TargetActionSupport, { } return null; }), - /** Calls a action passed to a component. For example a component for playing or pausing music may translate click events diff --git a/packages/ember-glimmer/lib/syntax/curly-component.js b/packages/ember-glimmer/lib/syntax/curly-component.js index b728a7e467d..54813f9de88 100644 --- a/packages/ember-glimmer/lib/syntax/curly-component.js +++ b/packages/ember-glimmer/lib/syntax/curly-component.js @@ -42,7 +42,7 @@ function applyAttributeBindings(attributeBindings, component, operations) { function privatizeTargetObject(props) { if (props.targetObject) { props._targetObject = props.targetObject; - delete props.targetObject; + props.targetObject = undefined; } } diff --git a/packages/ember-glimmer/tests/integration/components/target-action-test.js b/packages/ember-glimmer/tests/integration/components/target-action-test.js index 9dd28c7004b..edc498d69fa 100644 --- a/packages/ember-glimmer/tests/integration/components/target-action-test.js +++ b/packages/ember-glimmer/tests/integration/components/target-action-test.js @@ -200,17 +200,17 @@ moduleFor('Components test: sendAction', class extends RenderingTest { moduleFor('Components test: sendAction to a controller', class extends ApplicationTest { ['@test sendAction should trigger an action on the parent component\'s controller if it exists'](assert) { - assert.expect(7); + assert.expect(10); let component; this.router.map(function () { + this.route('withController'); + this.route('withoutController'); this.route('withNestedController', function () { this.route('nestedWithController'); this.route('nestedWithoutController'); }); - this.route('withController'); - this.route('withoutController'); }); this.registerComponent('foo-bar', { @@ -222,52 +222,64 @@ moduleFor('Components test: sendAction to a controller', class extends Applicati }) }); - this.registerTemplate('withNestedController', '{{foo-bar poke="poke"}}{{outlet}}'); - this.registerTemplate('withController', '{{foo-bar poke="poke"}}'); - this.registerTemplate('withoutController', '{{foo-bar poke="poke"}}'); - this.registerTemplate('withNestedController.nestedWithController', '{{foo-bar poke="poke"}}'); - this.registerTemplate('withNestedController.nestedWithoutController', '{{foo-bar poke="poke"}}'); - + this.registerRoute('withController', Route); this.registerController('withController', Controller.extend({ send(actionName, actionContext) { assert.equal(actionName, 'poke', 'send() method was invoked from a top level controller'); assert.equal(actionContext, 'top', 'action arguments were passed into the top level controller'); } })); + this.registerTemplate('withController', '{{foo-bar poke="poke"}}'); + + this.registerRoute('withoutController', Route.extend({ + actions: { + poke(actionContext) { + assert.ok(true, 'Unhandled action sent to route'); + assert.equal(actionContext, 'top no controller'); + } + } + })); + this.registerTemplate('withoutController', '{{foo-bar poke="poke"}}'); + this.registerRoute('withNestedController', Route.extend({ + actions: { + poke(actionContext) { + assert.ok(true, 'Unhandled action sent to route'); + assert.equal(actionContext, 'top with nested no controller'); + } + } + })); + this.registerTemplate('withNestedController', '{{foo-bar poke="poke"}}{{outlet}}'); + + this.registerRoute('withNestedController.nestedWithController', Route); this.registerController('withNestedControllerNestedWithController', Controller.extend({ send(actionName, actionContext) { assert.equal(actionName, 'poke', 'send() method was invoked from a nested controller'); assert.equal(actionContext, 'nested', 'action arguments were passed into the nested controller'); } })); + this.registerTemplate('withNestedController.nestedWithController', '{{foo-bar poke="poke"}}'); - this.registerRoute('withController', Route); - this.registerRoute('withNestedController', Route); - this.registerRoute('withNestedController.nestedWithController', Route); - this.registerRoute('withNestedController.nestedWithOutController', Route); - this.registerRoute('withoutController', Route); - - function expectSendActionToThrow() { - let fn = () => component.sendAction('poke'); - if (EmberDev && EmberDev.runningProdBuild) { - expectAssertion(fn); - } else { - assert.throws(fn); + this.registerRoute('withNestedController.nestedWithoutController', Route.extend({ + actions: { + poke(actionContext) { + assert.ok(true, 'Unhandled action sent to route'); + assert.equal(actionContext, 'nested no controller'); + } } - } + })); + this.registerTemplate('withNestedController.nestedWithoutController', '{{foo-bar poke="poke"}}'); return this.visit('/withController') .then(() => component.sendAction('poke', 'top')) - .then(() => this.visit('withoutController')) - .then(expectSendActionToThrow) - // withNestedController.index does not have a controller specified, so it should throw + .then(() => this.visit('/withoutController')) + .then(() => component.sendAction('poke', 'top no controller')) .then(() => this.visit('/withNestedController')) - .then(expectSendActionToThrow) + .then(() => component.sendAction('poke', 'top with nested no controller')) .then(() => this.visit('/withNestedController/nestedWithController')) .then(() => component.sendAction('poke', 'nested')) .then(() => this.visit('/withNestedController/nestedWithoutController')) - .then(expectSendActionToThrow); + .then(() => component.sendAction('poke', 'nested no controller')); } ['@test sendAction should not trigger an action an outlet\'s controller if a parent component handles it'](assert) { @@ -310,7 +322,7 @@ moduleFor('Components test: sendAction to a controller', class extends Applicati moduleFor('Components test: sendAction of a closure action', class extends RenderingTest { ['@test action should be called'](assert) { - this.assert.expect(1); + assert.expect(1); let component; this.registerComponent('inner-component', { @@ -400,8 +412,8 @@ moduleFor('Components test: send', class extends RenderingTest { this.runTask(() => component.send('foo', 'bar')); - expectAssertion(function () { - component.send('baz', 'bar'); + expectAssertion(() => { + return component.send('baz', 'bar'); }, /had no action handler for: baz/); } diff --git a/packages/ember-glimmer/tests/utils/abstract-test-case.js b/packages/ember-glimmer/tests/utils/abstract-test-case.js index 53ea09c7f03..0767d169a74 100644 --- a/packages/ember-glimmer/tests/utils/abstract-test-case.js +++ b/packages/ember-glimmer/tests/utils/abstract-test-case.js @@ -313,20 +313,6 @@ export class ApplicationTest extends TestCase { registerController(name, controller) { this.application.register(`controller:${name}`, controller); } - - registerComponent(name, { ComponentClass = null, template = null }) { - let { application } = this; - - if (ComponentClass) { - application.register(`component:${name}`, ComponentClass); - } - - if (typeof template === 'string') { - application.register(`template:components/${name}`, compile(template, { - moduleName: `components/${name}` - })); - } - } } export class RenderingTest extends TestCase {