Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BUGFIX beta] Fix no-jQuery EventDispatcher to allow handling multiple element-space actions #17876

Merged
merged 1 commit into from
Apr 7, 2019
Merged

[BUGFIX beta] Fix no-jQuery EventDispatcher to allow handling multiple element-space actions #17876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions packages/@ember/-internals/glimmer/tests/integration/helpers/element-action-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,45 @@ moduleFor(
this.assert.equal(originalHandlerWasCalled, true, 'the click handler was called');
}

['@test multiple actions with bubbles=false for same event are called but prevent bubbling']() {
let clickAction1WasCalled = false;
let clickAction2WasCalled = false;
let eventHandlerWasCalled = false;

let ExampleComponent = Component.extend({
actions: {
clicked1() {
clickAction1WasCalled = true;
},
clicked2() {
clickAction2WasCalled = true;
},
},
});

this.registerComponent('example-component', {
ComponentClass: ExampleComponent,
template: strip`
<a href="#"
{{action "clicked1" on="click" bubbles=false}}
{{action "clicked2" on="click" bubbles=false}}
>click me</a>`,
click() {
eventHandlerWasCalled = true;
},
});

this.render('{{example-component}}');

runTask(() => {
this.$('a').trigger('click');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A question about this (as @nlfurniss and I are working on upgrading an app): this PR says it is fixing the no-jQuery eventDispatcher, but this test appears (and perhaps I'm simply misunderstanding this?) to be adding tests here and below for jQuery-based event trigger. Is this using jQuery only for event triggering, but not for dispatch? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chriskrycho You mean because of that $() call looking like jQuery?

If so, it is actually not jQuery here. Was also confused when I first saw this. But there is some test infrastructure that adds this without actually using jQuery. Probably helped migrating tests away from jQuery back then (my guess). See

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We thought that might be the case but wanted to double check. Thank you very much for clarifying!

});

this.assert.ok(clickAction1WasCalled, 'the first clicked action was called');
this.assert.ok(clickAction2WasCalled, 'the second clicked action was called');
this.assert.notOk(eventHandlerWasCalled, 'event did not bubble up');
}

['@test it should work properly in an #each block']() {
let editHandlerWasCalled = false;

Expand Down Expand Up @@ -1359,6 +1398,40 @@ moduleFor(
this.assert.ok(doubleClickActionWasCalled, 'the doubleClicked action was called');
}

['@test allows multiple actions for same event on a single element']() {
let clickAction1WasCalled = false;
let clickAction2WasCalled = false;

let ExampleComponent = Component.extend({
actions: {
clicked1() {
clickAction1WasCalled = true;
},
clicked2() {
clickAction2WasCalled = true;
},
},
});

this.registerComponent('example-component', {
ComponentClass: ExampleComponent,
template: strip`
<a href="#"
{{action "clicked1" on="click"}}
{{action "clicked2" on="click"}}
>click me</a>`,
});

this.render('{{example-component}}');

runTask(() => {
this.$('a').trigger('click');
});

this.assert.ok(clickAction1WasCalled, 'the first clicked action was called');
this.assert.ok(clickAction2WasCalled, 'the second clicked action was called');
}

['@test it should respect preventDefault option if provided']() {
let ExampleComponent = Component.extend({
actions: {
Expand Down
5 changes: 4 additions & 1 deletion packages/@ember/-internals/views/lib/system/event_dispatcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,16 @@ export default EmberObject.extend({
return;
}

let result = true;
for (let index = 0; index < actions.length; index++) {
let action = actions[index];

if (action && action.eventName === eventName) {
return action.handler(event);
// return false if any of the action handlers returns false
result = action.handler(event) && result;
}
}
return result;
};

// Special handling of events that don't bubble (event delegation does not work).
Expand Down