Skip to content

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

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

Closed
vsavkin wants to merge 1 commit into from
Closed

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

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
37 changes: 4 additions & 33 deletions modules/angular2/docs/core/01_templates.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,33 +130,6 @@ Example:

Example:

`<div on-click="doX()">`
</td>
</tr>
<tr>
<th>Event binding (bubbling)</th>
<td>
`<div (^event)="statement">`

Example:

`<div (^mouseover)="hlite()">`
</td>
<td>
`<div on-bubble-event="statement">`

Example:

`<div on-bubble-mouseover="hlite()">`
</td>
</tr>
<tr>
<th>Declare reference</th>
<td>
`<div #symbol>`

Example:

<pre>
```
<video #player>
Expand Down Expand Up @@ -420,7 +393,7 @@ of the templates occurs. One such example is `ng-for`.

Where:
* `[ng-for]` triggers the for directive.
* `#person` exports the implicit `ng-for` item.
* `#person` exports the implicit `ng-for` item.
* `[ng-for-of]="people"` binds an iterable object to the `ng-for` controller.
* `#i=index` exports item index as `i`.

Expand Down Expand Up @@ -515,18 +488,16 @@ Where:
* `statement` is a valid statement (as defined in section below).
If the execution of the statement returns `false`, then `preventDefault`is applied on the DOM event.

By default, angular only listens to the element on the event, and ignores events which bubble. To listen to bubbled
events (as in the case of clicking on any child) use the bubble option (`(event)` or `on-bubble-event`) as shown
below.
Angular listens to bubbled DOM events (as in the case of clicking on any child), as shown below:

<table>
<tr>
<th>Short form</th>
<td>`<some-element (^some-event)="statement">`</td>
<td>`<some-element (some-event)="statement">`</td>
</tr>
<tr>
<th>Canonical form</th>
<td>`<some-element on-bubble-some-event="statement">`</td>
<td>`<some-element on-some-event="statement">`</td>
</tr>
</table>

Expand Down
36 changes: 16 additions & 20 deletions modules/angular2/src/core/render/dom/compiler/property_binding_parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@ import {dashCaseToCamelCase} from '../util';
// Group 1 = "bind-"
// Group 2 = "var-" or "#"
// Group 3 = "on-"
// Group 4 = "onbubble-"
// Group 5 = "bindon-"
// Group 6 = the identifier after "bind-", "var-/#", or "on-"
// Group 7 = idenitifer inside [()]
// Group 8 = idenitifer inside []
// Group 9 = identifier inside ()
// Group 4 = "bindon-"
// Group 5 = the identifier after "bind-", "var-/#", or "on-"
// Group 6 = idenitifer inside [()]
// Group 7 = idenitifer inside []
// Group 8 = identifier inside ()
var BIND_NAME_REGEXP =
/^(?:(?:(?:(bind-)|(var-|#)|(on-)|(onbubble-)|(bindon-))(.+))|\[\(([^\)]+)\)\]|\[([^\]]+)\]|\(([^\)]+)\))$/g;
/^(?:(?:(?:(bind-)|(var-|#)|(on-)|(bindon-))(.+))|\[\(([^\)]+)\)\]|\[([^\]]+)\]|\(([^\)]+)\))$/g;
/**
* Parses the property bindings on a single element.
*/
Expand All @@ -39,33 +38,30 @@ export class PropertyBindingParser implements CompileStep {
var bindParts = RegExpWrapper.firstMatch(BIND_NAME_REGEXP, attrName);
if (isPresent(bindParts)) {
if (isPresent(bindParts[1])) { // match: bind-prop
this._bindProperty(bindParts[6], attrValue, current, newAttrs);
this._bindProperty(bindParts[5], attrValue, current, newAttrs);

} else if (isPresent(
bindParts[2])) { // match: var-name / var-name="iden" / #name / #name="iden"
var identifier = bindParts[6];
var identifier = bindParts[5];
var value = attrValue == '' ? '\$implicit' : attrValue;
this._bindVariable(identifier, value, current, newAttrs);

} else if (isPresent(bindParts[3])) { // match: on-event
this._bindEvent(bindParts[6], attrValue, current, newAttrs);
this._bindEvent(bindParts[5], attrValue, current, newAttrs);

} else if (isPresent(bindParts[4])) { // match: onbubble-event
this._bindEvent('^' + bindParts[6], attrValue, current, newAttrs);
} else if (isPresent(bindParts[4])) { // match: bindon-prop
this._bindProperty(bindParts[5], attrValue, current, newAttrs);
this._bindAssignmentEvent(bindParts[5], attrValue, current, newAttrs);

} else if (isPresent(bindParts[5])) { // match: bindon-prop
} else if (isPresent(bindParts[6])) { // match: [(expr)]
this._bindProperty(bindParts[6], attrValue, current, newAttrs);
this._bindAssignmentEvent(bindParts[6], attrValue, current, newAttrs);

} else if (isPresent(bindParts[7])) { // match: [(expr)]
} else if (isPresent(bindParts[7])) { // match: [expr]
this._bindProperty(bindParts[7], attrValue, current, newAttrs);
this._bindAssignmentEvent(bindParts[7], attrValue, current, newAttrs);

} else if (isPresent(bindParts[8])) { // match: [expr]
this._bindProperty(bindParts[8], attrValue, current, newAttrs);

} else if (isPresent(bindParts[9])) { // match: (event)
this._bindEvent(bindParts[9], attrValue, current, newAttrs);
} else if (isPresent(bindParts[8])) { // match: (event)
this._bindEvent(bindParts[8], attrValue, current, newAttrs);
}
} else {
var expr = this._parser.parseInterpolation(attrValue, current.elementDescription);
Expand Down
61 changes: 12 additions & 49 deletions modules/angular2/src/core/render/dom/events/event_manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import {isBlank, BaseException, isPresent, StringWrapper} from 'angular2/src/cor
import {DOM} from 'angular2/src/core/dom/dom_adapter';
import {NgZone} from 'angular2/src/core/zone/ng_zone';

const BUBBLE_SYMBOL = '^';

export class EventManager {
constructor(public _plugins: EventManagerPlugin[], public _zone: NgZone) {
for (var i = 0; i < _plugins.length; i++) {
Expand All @@ -12,17 +10,13 @@ export class EventManager {
}

addEventListener(element: HTMLElement, eventName: string, handler: Function) {
var withoutBubbleSymbol = this._removeBubbleSymbol(eventName);
var plugin = this._findPluginFor(withoutBubbleSymbol);
plugin.addEventListener(element, withoutBubbleSymbol, handler,
withoutBubbleSymbol != eventName);
var plugin = this._findPluginFor(eventName);
plugin.addEventListener(element, eventName, handler);
}

addGlobalEventListener(target: string, eventName: string, handler: Function): Function {
var withoutBubbleSymbol = this._removeBubbleSymbol(eventName);
var plugin = this._findPluginFor(withoutBubbleSymbol);
return plugin.addGlobalEventListener(target, withoutBubbleSymbol, handler,
withoutBubbleSymbol != eventName);
var plugin = this._findPluginFor(eventName);
return plugin.addGlobalEventListener(target, eventName, handler);
}

getZone(): NgZone { return this._zone; }
Expand All @@ -37,28 +31,19 @@ export class EventManager {
}
throw new BaseException(`No event manager plugin found for event ${eventName}`);
}

_removeBubbleSymbol(eventName: string): string {
return eventName[0] == BUBBLE_SYMBOL ? StringWrapper.substring(eventName, 1) : eventName;
}
}

export class EventManagerPlugin {
manager: EventManager;

// We are assuming here that all plugins support bubbled and non-bubbled events.
// That is equivalent to having supporting $event.target
// The bubbling flag (currently ^) is stripped before calling the supports and
// addEventListener methods.
supports(eventName: string): boolean { return false; }

addEventListener(element: HTMLElement, eventName: string, handler: Function,
shouldSupportBubble: boolean) {
addEventListener(element: HTMLElement, eventName: string, handler: Function) {
throw "not implemented";
}

addGlobalEventListener(element: string, eventName: string, handler: Function,
shouldSupportBubble: boolean): Function {
addGlobalEventListener(element: string, eventName: string, handler: Function): Function {
throw "not implemented";
}
}
Expand All @@ -70,39 +55,17 @@ export class DomEventsPlugin extends EventManagerPlugin {
// events.
supports(eventName: string): boolean { return true; }

addEventListener(element: HTMLElement, eventName: string, handler: Function,
shouldSupportBubble: boolean) {
var outsideHandler =
this._getOutsideHandler(shouldSupportBubble, element, handler, this.manager._zone);
addEventListener(element: HTMLElement, eventName: string, handler: Function) {
var zone = this.manager._zone;
var outsideHandler = (event) => zone.run(() => handler(event));
this.manager._zone.runOutsideAngular(() => { DOM.on(element, eventName, outsideHandler); });
}

addGlobalEventListener(target: string, eventName: string, handler: Function,
shouldSupportBubble: boolean): Function {
addGlobalEventListener(target: string, eventName: string, handler: Function): Function {
var element = DOM.getGlobalEventTarget(target);
var outsideHandler =
this._getOutsideHandler(shouldSupportBubble, element, handler, this.manager._zone);
var zone = this.manager._zone;
var outsideHandler = (event) => zone.run(() => handler(event));
return this.manager._zone.runOutsideAngular(
() => { return DOM.onAndCancel(element, eventName, outsideHandler); });
}

_getOutsideHandler(shouldSupportBubble: boolean, element: HTMLElement, handler: Function,
zone: NgZone) {
return shouldSupportBubble ? DomEventsPlugin.bubbleCallback(element, handler, zone) :
DomEventsPlugin.sameElementCallback(element, handler, zone);
}

static sameElementCallback(element: HTMLElement, handler: Function, zone: NgZone):
(event: Event) => void {
return (event) => {
if (event.target === element) {
zone.run(() => handler(event));
}
};
}

static bubbleCallback(element: HTMLElement, handler: Function, zone: NgZone):
(event: Event) => void {
return (event) => zone.run(() => handler(event));
}
}
5 changes: 1 addition & 4 deletions modules/angular2/src/core/render/dom/events/hammer_gestures.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ class HammerGesturesPlugin extends HammerGesturesPluginCommon {
return true;
}

addEventListener(Element element, String eventName, Function handler,
bool shouldSupportBubble) {
if (shouldSupportBubble) throw new BaseException(
'Hammer.js plugin does not support bubbling gestures.');
addEventListener(Element element, String eventName, Function handler) {

@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.

var zone = this.manager.getZone();
eventName = eventName.toLowerCase();

Expand Down
5 changes: 1 addition & 4 deletions modules/angular2/src/core/render/dom/events/hammer_gestures.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ export class HammerGesturesPlugin extends HammerGesturesPluginCommon {
return true;
}

addEventListener(element: HTMLElement, eventName: string, handler: Function,
shouldSupportBubble: boolean) {
if (shouldSupportBubble)
throw new BaseException('Hammer.js plugin does not support bubbling gestures.');
addEventListener(element: HTMLElement, eventName: string, handler: Function) {
var zone = this.manager.getZone();
eventName = eventName.toLowerCase();

Expand Down
15 changes: 6 additions & 9 deletions modules/angular2/src/core/render/dom/events/key_events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,11 @@ export class KeyEventsPlugin extends EventManagerPlugin {
return isPresent(KeyEventsPlugin.parseEventName(eventName));
}

addEventListener(element: HTMLElement, eventName: string, handler: (Event: any) => any,
shouldSupportBubble: boolean) {
addEventListener(element: HTMLElement, eventName: string, handler: (Event: any) => any) {
var parsedEvent = KeyEventsPlugin.parseEventName(eventName);

var outsideHandler = KeyEventsPlugin.eventCallback(element, shouldSupportBubble,
StringMapWrapper.get(parsedEvent, 'fullKey'),
handler, this.manager.getZone());
var outsideHandler = KeyEventsPlugin.eventCallback(
element, StringMapWrapper.get(parsedEvent, 'fullKey'), handler, this.manager.getZone());

this.manager.getZone().runOutsideAngular(() => {
DOM.on(element, StringMapWrapper.get(parsedEvent, 'domEventName'), outsideHandler);
Expand Down Expand Up @@ -91,11 +89,10 @@ export class KeyEventsPlugin extends EventManagerPlugin {
return fullKey;
}

static eventCallback(element: HTMLElement, shouldSupportBubble: boolean, fullKey: any,
handler: (Event) => any, zone: NgZone): (event: Event) => void {
static eventCallback(element: HTMLElement, fullKey: any, handler: (Event) => any, zone: NgZone):
(event: Event) => void {
return (event) => {
var correctElement = shouldSupportBubble || event.target === element;
if (correctElement && StringWrapper.equals(KeyEventsPlugin.getEventFullKey(event), fullKey)) {
if (StringWrapper.equals(KeyEventsPlugin.getEventFullKey(event), fullKey)) {
zone.run(() => handler(event));
}
};
Expand Down
2 changes: 1 addition & 1 deletion modules/angular2/src/router/router_link.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {Instruction, stringifyInstruction} from './instruction';
selector: '[router-link]',
properties: ['routeParams: routerLink'],
host: {
'(^click)': 'onClick()',
'(click)': 'onClick()',
'[attr.href]': 'visibleHref',
'[class.router-link-active]': 'isRouteActive'
}
Expand Down
27 changes: 14 additions & 13 deletions modules/angular2/test/core/compiler/integration_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,9 @@ export function main() {

dispatchEvent(tc.nativeElement, 'domEvent');

expect(listener.eventType).toEqual('domEvent');
expect(listener.eventTypes)
.toEqual(
['domEvent', 'body_domEvent', 'document_domEvent', 'window_domEvent']);

async.done();
});
Expand All @@ -874,16 +876,16 @@ export function main() {
var tc = rootTC.componentViewChildren[0];
var listener = tc.inject(DirectiveListeningDomEvent);
dispatchEvent(DOM.getGlobalEventTarget("window"), 'domEvent');
expect(listener.eventType).toEqual('window_domEvent');
expect(listener.eventTypes).toEqual(['window_domEvent']);

listener = tc.inject(DirectiveListeningDomEvent);
listener.eventTypes = [];
dispatchEvent(DOM.getGlobalEventTarget("document"), 'domEvent');
expect(listener.eventType).toEqual('document_domEvent');
expect(listener.eventTypes).toEqual(['document_domEvent', 'window_domEvent']);

rootTC.destroy();
listener = tc.inject(DirectiveListeningDomEvent);
listener.eventTypes = [];
dispatchEvent(DOM.getGlobalEventTarget("body"), 'domEvent');
expect(listener.eventType).toEqual('');
expect(listener.eventTypes).toEqual([]);

async.done();
});
Expand Down Expand Up @@ -983,7 +985,7 @@ export function main() {
var listener = tc.inject(DirectiveListeningDomEvent);
var listenerother = tc.inject(DirectiveListeningDomEventOther);
dispatchEvent(DOM.getGlobalEventTarget("window"), 'domEvent');
expect(listener.eventType).toEqual('window_domEvent');
expect(listener.eventTypes).toEqual(['window_domEvent']);
expect(listenerother.eventType).toEqual('other_domEvent');
expect(globalCounter).toEqual(1);

Expand Down Expand Up @@ -1851,12 +1853,11 @@ class DirectiveListeningEvent {
})
@Injectable()
class DirectiveListeningDomEvent {
eventType: string;
constructor() { this.eventType = ''; }
onEvent(eventType: string) { this.eventType = eventType; }
onWindowEvent(eventType: string) { this.eventType = "window_" + eventType; }
onDocumentEvent(eventType: string) { this.eventType = "document_" + eventType; }
onBodyEvent(eventType: string) { this.eventType = "body_" + eventType; }
eventTypes: string[] = [];
onEvent(eventType: string) { this.eventTypes.push(eventType); }
onWindowEvent(eventType: string) { this.eventTypes.push("window_" + eventType); }
onDocumentEvent(eventType: string) { this.eventTypes.push("document_" + eventType); }
onBodyEvent(eventType: string) { this.eventTypes.push("body_" + eventType); }
}

var globalCounter = 0;
Expand Down
14 changes: 0 additions & 14 deletions modules/angular2/test/core/render/dom/compiler/property_binding_parser_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,20 +182,6 @@ export function main() {
expect(eventBinding.fullName).toEqual('click');
});

it('should detect onbubble- syntax', () => {
var results = process(el('<div onbubble-click="b()"></div>'));
var eventBinding = results[0].eventBindings[0];
expect(eventBinding.source.source).toEqual('b()');
expect(eventBinding.fullName).toEqual('^click');
});

it('should detect onbubble- syntax with data- prefix', () => {
var results = process(el('<div data-onbubble-click="b()"></div>'));
var eventBinding = results[0].eventBindings[0];
expect(eventBinding.source.source).toEqual('b()');
expect(eventBinding.fullName).toEqual('^click');
});

it('should parse event handlers using on- syntax as actions', () => {
var results = process(el('<div on-click="foo=bar"></div>'));
var eventBinding = results[0].eventBindings[0];
Expand Down
Loading