Skip to content

feat(core): remove the (^ syntax and make all DOM events bubbling - #3864

Closed
vsavkin wants to merge 1 commit into
angular:masterfrom
vsavkin:remove_bubbling
Closed

feat(core): remove the (^ syntax and make all DOM events bubbling#3864
vsavkin wants to merge 1 commit into
angular:masterfrom
vsavkin:remove_bubbling

Conversation

@vsavkin

@vsavkin vsavkin commented Aug 27, 2015

Copy link
Copy Markdown
Contributor

BREAKING CHANGE

Before

<div (^click)="onEventHandler()">
  <button></button>
</div>

After

<div (click)="onEventHandler()">
  <button></button>
</div>

@vsavkin vsavkin added comp: core action: review The PR is still awaiting reviews from at least one requested reviewer effort1: hours labels Aug 27, 2015
@vsavkin vsavkin added this to the alpha-37 milestone Aug 27, 2015
@pkozlowski-opensource

pkozlowski-opensource commented Aug 27, 2015

Copy link
Copy Markdown
Member

Yes! Out of curiosity - what were the trade-offs here? I mean, why we've decided to not catch bubbling events by default?

@mhevery

mhevery commented Aug 27, 2015

Copy link
Copy Markdown
Contributor

@pkozlowski-opensource The change is that event decides if it is bubbling or not. In practice it turns out that you want all user events (mouse, keys, etc) to bubble, but non of the app events (open,close,onChange, etc) to bubble.

So we are going the route of event declaring if it wants to bubble or not.

@mhevery

mhevery commented Aug 27, 2015

Copy link
Copy Markdown
Contributor

Related: #2296

@mhevery mhevery added pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 27, 2015
@mhevery mhevery assigned vsavkin and unassigned mhevery Aug 27, 2015
vicb
vicb reviewed Aug 27, 2015
View reviewed changes
Comment thread modules/angular2/src/core/render/dom/events/hammer_gestures.dart

@vicb vicb Aug 27, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hammer events do not bubble by default - because they are not using DOM events.

It is possible to configure Hammer events to bubble (by using DOM event, it comes with a perf penalty) ? This could peobably be done in a follow up PR.

@0x-r4bbit 0x-r4bbit mentioned this pull request Aug 30, 2015
Closed
@0x-r4bbit

0x-r4bbit commented Aug 30, 2015

Copy link
Copy Markdown
Contributor

Will custom events bubble up by default too then?

@0x-r4bbit 0x-r4bbit mentioned this pull request Aug 31, 2015
Closed
@vsavkin

vsavkin commented Aug 31, 2015

Copy link
Copy Markdown
Contributor Author

@PascalPrecht custom dom events can bubble, but synthetic events won't.

@vsavkin vsavkin added the action: merge The PR is ready for merge by the caretaker label Aug 31, 2015
@mary-poppins

mary-poppins commented Aug 31, 2015

Copy link
Copy Markdown

Merging PR #3864 on behalf of @vsavkin to branch presubmit-vsavkin-pr-3864.

@mary-poppins mary-poppins removed the action: merge The PR is ready for merge by the caretaker label Aug 31, 2015
@vsavkin
feat(core): remove the (^ syntax and make all DOM events bubbling
d2f74d0 
BREAKING CHANGE

Before
<div (^click)="onEventHandler()">
  <button></button>
</div>

After
<div (click)="onEventHandler()">
  <button></button>
</div>
@vsavkin vsavkin added the action: merge The PR is ready for merge by the caretaker label Sep 1, 2015
@mary-poppins

mary-poppins commented Sep 1, 2015

Copy link
Copy Markdown

Merging PR #3864 on behalf of @vsavkin to branch presubmit-vsavkin-pr-3864.

@mary-poppins mary-poppins removed the action: merge The PR is ready for merge by the caretaker label Sep 1, 2015
@vsavkin vsavkin closed this in 60ce884 Sep 1, 2015
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 7, 2019

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@vsavkin vsavkin

Labels

cla: yes effort1: hours

Projects

None yet

Milestone

alpha-37

Development

Successfully merging this pull request may close these issues.

7 participants

@vsavkin @pkozlowski-opensource @mhevery @0x-r4bbit @mary-poppins @vicb @googlebot