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.

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

Don't spin up extra run-loops when bubbling events. Fixes #11540 #12075

Merged
merged 1 commit into from
Sep 9, 2015

Conversation

runspired
Copy link
Contributor

Fixes #11540

@rwjblue
Copy link
Member

rwjblue commented Aug 13, 2015

Seems good to me, @stefanpenner r?

@stefanpenner
Copy link
Member

I plan to review in the morning. Unless there is s big rush tonight. If so I can gladly look into shortly

@rwjblue
Copy link
Member

rwjblue commented Aug 13, 2015

No huge rush. Can go into [BUGFIX beta] tomorrow (for 2.1).

@rwjblue
Copy link
Member

rwjblue commented Aug 15, 2015

I kicked sauce labs.

@stefanpenner - Whenever you have a chance, I'd love your 👍 / 👎 here...

@rwjblue
Copy link
Member

rwjblue commented Aug 22, 2015

@stefanpenner - r?

@rwjblue
Copy link
Member

rwjblue commented Sep 9, 2015

@stefanpenner - 👍 / 👎?

@stefanpenner
Copy link
Member

This fixes up the immediate concern so 👍

This doesn't prevent bubbling from spawning many loops per user action.

For example:

given the following hierarchy root < a < b < c

if a and b have click handlers, and c returns true, a will gets its own run-loop. This isn't desirable.

@runspired I believe due to how we currently integrate with jQuery prevent ^ in the same micro-task is tricky/not possible. To sort this out, I suspect taking your approach re: home-grown event-dispatcher is key.

stefanpenner added a commit that referenced this pull request Sep 9, 2015
Don't spin up extra run-loops when bubbling events.  Fixes #11540
@stefanpenner stefanpenner merged commit 60ab022 into emberjs:master Sep 9, 2015
@asakusuma
Copy link
Contributor

Thanks @runspired!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants