Skip to content

Commit

Permalink
[BUGFIX release] Fix component action bubbling semantics.
Browse files Browse the repository at this point in the history
When a component action returns `true` we should not bubble the action
upwards (this is a significant difference between `Component#send` and
`Controller#send` / `Route#send`). The only way to get bubbling from
within a component is to call `Component#sendAction`.

This removes an invalid fallback in `ActionSupport` mixin that allowed
all component actions to bubble, and adds a few tests to help ensure we
do not regress in future refactors.
  • Loading branch information
Robert Jackson committed Sep 14, 2016
1 parent 3139bbf commit ea1eeb6
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2725,4 +2725,57 @@ moduleFor('Components test: curly components', class extends RenderingTest {
this.render('{{foo-bar}}');
}, /didInitAttrs called/);
}

['@test returning `true` from an action does not bubble if `target` is not specified (GH#14275)'](assert) {
this.registerComponent('display-toggle', {
ComponentClass: Component.extend({
actions: {
show() {
assert.ok(true, 'display-toggle show action was called');
return true;
}
}
}),

template: `<button {{action 'show'}}>Show</button>`
});

this.render(`{{display-toggle}}`, {
send() {
assert.notOk(true, 'send should not be called when action is not "subscribed" to');
}
});

this.assertText('Show');

this.runTask(() => this.$('button').click());
}

['@test returning `true` from an action bubbles to the `target` if specified'](assert) {
assert.expect(4);

this.registerComponent('display-toggle', {
ComponentClass: Component.extend({
actions: {
show() {
assert.ok(true, 'display-toggle show action was called');
return true;
}
}
}),

template: `<button {{action 'show'}}>Show</button>`
});

this.render(`{{display-toggle target=this}}`, {
send(actionName) {
assert.ok(true, 'send should be called when action is "subscribed" to');
assert.equal(actionName, 'show');
}
});

this.assertText('Show');

this.runTask(() => this.$('button').click());
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,30 @@ moduleFor('Components test: sendAction', class extends RenderingTest {
}

['@test calling sendAction on a component within a block sends to the outer scope GH#14216'](assert) {
this.registerTemplate('components/action-delegate', strip`
{{#component-a}}
{{component-b bar="derp"}}
{{/component-a}}
`);
let testContext = this;
// overrides default action-delegate so actions can be added
this.registerComponent('action-delegate', {
ComponentClass: Component.extend({
init() {
this._super();
testContext.delegate = this;
this.name = 'action-delegate';
},

actions: {
derp(arg1) {
assert.ok(true, 'action called on action-delgate');
assert.equal(arg1, 'something special', 'argument passed through properly');
}
}
}),

template: strip`
{{#component-a}}
{{component-b bar="derp"}}
{{/component-a}}
`
});

this.registerComponent('component-a', {
ComponentClass: Component.extend({
Expand Down Expand Up @@ -238,10 +257,7 @@ moduleFor('Components test: sendAction', class extends RenderingTest {

this.renderDelegate();

this.runTask(() => innerChild.sendAction('bar'));

this.assertSendCount(1);
this.assertNamedSendCount('derp', 1);
this.runTask(() => innerChild.sendAction('bar', 'something special'));
}
});

Expand Down
2 changes: 1 addition & 1 deletion packages/ember-views/lib/mixins/action_support.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export default Mixin.create({
if (!shouldBubble) { return; }
}

target = get(this, 'target') || get(this, '_targetObject');
target = get(this, 'target');

if (target) {
assert(
Expand Down

0 comments on commit ea1eeb6

Please sign in to comment.