Skip to content

Commit f26e3ba

Browse files
mheverymhevery
committed
WIP: Issue-399
The issue is that ng-change and input directive can be registered in any order. This makes it possible for on-change event to execute before input event gets a chance to run. Changing from ng-change to on-change fixes the issue, since events are on-change has to bubble to EventHandler before it can run. This is not quite right, since if EventHandler is registered on the same element as input element, then again we don't know which event listener will run first. The separate registration model also means that apply() gets run for each element, which is wasteful. A proper way to fix this is for EventHandler to handle both the change event for both on-change as well as for input element. This means that EventHandler needs to be able to call listener function on InputElement. The only way to do that is for the EventHandler to attach the listener function to the Expando. Currently we are favoring the NgElement over NgPorbe object. So NgElement needs to have a list of listeners from underlying directives which are not expressions on the DOM (as is on-change). These listeners need to be stored in NgElement and retrieved by the EventHandler when event fires.
1 parent d483ffe commit f26e3ba

File tree

8 files changed

+125
-86
lines changed

8 files changed

+125
-86
lines changed

‎example/web/hello_world.dart

+10-5
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,19 @@ import 'package:angular/angular.dart';
22
import 'package:angular/angular_dynamic.dart';
33

44
@NgController(
5-
selector: '[hello-world-controller]',
6-
publishAs: 'ctrl')
7-
class HelloWorldController {
8-
String name = "world";
5+
selector: '[my-controller]',
6+
publishAs: 'ctrl'
7+
)
8+
class MyController {
9+
10+
String currentValue = "aaa";
11+
void selectionChanged() {
12+
print("currentValue $currentValue");
13+
}
914
}
1015

1116
main() {
1217
dynamicApplication()
13-
.addModule(new Module()..type(HelloWorldController))
18+
.addModule(new Module()..type(MyController))
1419
.run();
1520
}

‎example/web/hello_world.html

+11-8
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
<!DOCTYPE html>
2-
<html>
3-
<head>
4-
<title>Hello, World!</title>
5-
</head>
6-
<body hello-world-controller>
7-
8-
<h3>Hello {{ctrl.name}}!</h3>
9-
name: <input type="text" ng-model="ctrl.name">
2+
<html ng-app>
3+
<body>
4+
<div my-controller>
5+
<select
6+
ng-change="ctrl.selectionChanged()"
7+
ng-model="ctrl.currentValue">
8+
<option value="aaa">aaa</option>
9+
<option value="bbb">bbb</option>
10+
<option value="ccc">ccc</option>
11+
</select>
12+
</div>
1013

1114
<script type="application/dart" src="hello_world.dart"></script>
1215
<script src="packages/browser/dart.js"></script>

‎lib/core/scope.dart

+1
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ class Scope {
248248
}
249249

250250
dynamic apply([expression, Map locals]) {
251+
print('APPLY');
251252
_assertInternalStateConsistency();
252253
rootScope._transitionState(null, RootScope.STATE_APPLY);
253254
try {

‎lib/core_dom/event_handler.dart

+15-7
Original file line numberDiff line numberDiff line change
@@ -28,26 +28,34 @@ typedef void EventFunction(event);
2828
*/
2929
@NgInjectableService()
3030
class EventHandler {
31-
dom.Node _rootNode;
31+
final dom.Node _rootNode;
32+
final RootScope _scope;
3233
final Expando _expando;
3334
final ExceptionHandler _exceptionHandler;
34-
final _listeners = <String, Function>{};
35+
final _listeners = <String, Sream>{};
36+
dom.EventListener _eventListener;
3537

36-
EventHandler(this._rootNode, this._expando, this._exceptionHandler);
38+
EventHandler(this._rootNode, this._scope, this._expando, this._exceptionHandler) {
39+
_eventListener = _eventListenerMethod;
40+
}
3741

3842
/**
3943
* Register an event. This makes sure that an event (of the specified name)
4044
* which bubbles to this node, gets processed by this [EventHandler].
4145
*/
4246
void register(String eventName) {
4347
_listeners.putIfAbsent(eventName, () {
44-
dom.EventListener eventListener = this._eventListener;
45-
_rootNode.on[eventName].listen(eventListener);
46-
return eventListener;
48+
print('EventHandler.register($eventName)');
49+
return _rootNode.on[eventName].listen(_eventListener);;
4750
});
4851
}
4952

50-
void _eventListener(dom.Event event) {
53+
void release() {
54+
//TODO(misko): release all of the listeners. Needs to be called from afterEach() in test;
55+
}
56+
57+
void _eventListenerMethod(dom.Event event) {
58+
print('EventHandler.listener($event)');
5159
dom.Node element = event.target;
5260
while (element != null && element != _rootNode) {
5361
var expression;

‎lib/directive/input_select.dart

+2-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ class InputSelect implements NgAttachAware {
5454
_mode.onModelChange(_model.viewValue);
5555
});
5656

57-
_selectElement.onChange.listen((event) => _mode.onViewChange(event));
57+
print('Input on change');
58+
_selectElement.onChange.listen((event){ print('INPUT EVENT'); _mode.onViewChange(event);});
5859
_model.render = (value) {
5960
// TODO(misko): this hack need to delay the rendering until after domRead
6061
// because the modelChange reads from the DOM. We should be able to render

‎lib/directive/ng_events.dart

+57-58
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,11 @@ class NgEvent {
134134
// object? One would pretty much only assign to one or two of those
135135
// properties. I'm opting for the map since it's less boilerplate code.
136136
var listeners = {};
137+
final EventHandler eventHandler;
137138
final dom.Element element;
138139
final Scope scope;
139140

140-
NgEvent(this.element, this.scope);
141+
NgEvent(this.element, this.scope, this.eventHandler);
141142

142143
// NOTE: Do not use the element.on['some_event'].listen(...) syntax. Doing so
143144
// has two downsides:
@@ -147,63 +148,61 @@ class NgEvent {
147148
// platform specific event name automatically but you're on your own if
148149
// you use the on[] syntax. This also applies to $dom_addEventListener.
149150
// Ref: http://api.dartlang.org/docs/releases/latest/dart_html/Events.html
150-
initListener(var stream, var handler) {
151-
int key = stream.hashCode;
152-
if (!listeners.containsKey(key)) {
153-
listeners[key] = handler;
154-
stream.listen((event) => handler({r"$event": event}));
155-
}
151+
_initListener(name, stream, handler) {
152+
print("DEPRECATED: ng-$name is depricated use on-$name instead.");
153+
element.attributes['on-$name'] = element.attributes['ng-$name'];
154+
eventHandler.register(name);
156155
}
157156

158-
set onAbort(value) => initListener(element.onAbort, value);
159-
set onBeforeCopy(value) => initListener(element.onBeforeCopy, value);
160-
set onBeforeCut(value) => initListener(element.onBeforeCut, value);
161-
set onBeforePaste(value) => initListener(element.onBeforePaste, value);
162-
set onBlur(value) => initListener(element.onBlur, value);
163-
set onChange(value) => initListener(element.onChange, value);
164-
set onClick(value) => initListener(element.onClick, value);
165-
set onContextMenu(value) => initListener(element.onContextMenu, value);
166-
set onCopy(value) => initListener(element.onCopy, value);
167-
set onCut(value) => initListener(element.onCut, value);
168-
set onDoubleClick(value) => initListener(element.onDoubleClick, value);
169-
set onDrag(value) => initListener(element.onDrag, value);
170-
set onDragEnd(value) => initListener(element.onDragEnd, value);
171-
set onDragEnter(value) => initListener(element.onDragEnter, value);
172-
set onDragLeave(value) => initListener(element.onDragLeave, value);
173-
set onDragOver(value) => initListener(element.onDragOver, value);
174-
set onDragStart(value) => initListener(element.onDragStart, value);
175-
set onDrop(value) => initListener(element.onDrop, value);
176-
set onError(value) => initListener(element.onError, value);
177-
set onFocus(value) => initListener(element.onFocus, value);
178-
set onFullscreenChange(value) => initListener(element.onFullscreenChange, value);
179-
set onFullscreenError(value) => initListener(element.onFullscreenError, value);
180-
set onInput(value) => initListener(element.onInput, value);
181-
set onInvalid(value) => initListener(element.onInvalid, value);
182-
set onKeyDown(value) => initListener(element.onKeyDown, value);
183-
set onKeyPress(value) => initListener(element.onKeyPress, value);
184-
set onKeyUp(value) => initListener(element.onKeyUp, value);
185-
set onLoad(value) => initListener(element.onLoad, value);
186-
set onMouseDown(value) => initListener(element.onMouseDown, value);
187-
set onMouseEnter(value) => initListener(element.onMouseEnter, value);
188-
set onMouseLeave(value) => initListener(element.onMouseLeave, value);
189-
set onMouseMove(value) => initListener(element.onMouseMove, value);
190-
set onMouseOut(value) => initListener(element.onMouseOut, value);
191-
set onMouseOver(value) => initListener(element.onMouseOver, value);
192-
set onMouseUp(value) => initListener(element.onMouseUp, value);
193-
set onMouseWheel(value) => initListener(element.onMouseWheel, value);
194-
set onPaste(value) => initListener(element.onPaste, value);
195-
set onReset(value) => initListener(element.onReset, value);
196-
set onScroll(value) => initListener(element.onScroll, value);
197-
set onSearch(value) => initListener(element.onSearch, value);
198-
set onSelect(value) => initListener(element.onSelect, value);
199-
set onSelectStart(value) => initListener(element.onSelectStart, value);
200-
// set onSpeechChange(value) => initListener(element.onSpeechChange, value);
201-
set onSubmit(value) => initListener(element.onSubmit, value);
202-
set onTouchCancel(value) => initListener(element.onTouchCancel, value);
203-
set onTouchEnd(value) => initListener(element.onTouchEnd, value);
204-
set onTouchEnter(value) => initListener(element.onTouchEnter, value);
205-
set onTouchLeave(value) => initListener(element.onTouchLeave, value);
206-
set onTouchMove(value) => initListener(element.onTouchMove, value);
207-
set onTouchStart(value) => initListener(element.onTouchStart, value);
208-
set onTransitionEnd(value) => initListener(element.onTransitionEnd, value);
157+
set onAbort(value) => _initListener('abort', element.onAbort, value);
158+
set onBeforeCopy(value) => _initListener('beforecopy', element.onBeforeCopy, value);
159+
set onBeforeCut(value) => _initListener('beforecut', element.onBeforeCut, value);
160+
set onBeforePaste(value) => _initListener('beforepaste', element.onBeforePaste, value);
161+
set onBlur(value) => _initListener('blur', element.onBlur, value);
162+
set onChange(value) => _initListener('change', element.onChange, value);
163+
set onClick(value) => _initListener('click', element.onClick, value);
164+
set onContextMenu(value) => _initListener('contextmenu', element.onContextMenu, value);
165+
set onCopy(value) => _initListener('copy', element.onCopy, value);
166+
set onCut(value) => _initListener('cut', element.onCut, value);
167+
set onDoubleClick(value) => _initListener('doubleclick', element.onDoubleClick, value);
168+
set onDrag(value) => _initListener('drag', element.onDrag, value);
169+
set onDragEnd(value) => _initListener('dragend', element.onDragEnd, value);
170+
set onDragEnter(value) => _initListener('dragenter', element.onDragEnter, value);
171+
set onDragLeave(value) => _initListener('dragleave', element.onDragLeave, value);
172+
set onDragOver(value) => _initListener('dragover', element.onDragOver, value);
173+
set onDragStart(value) => _initListener('dragstart', element.onDragStart, value);
174+
set onDrop(value) => _initListener('drop', element.onDrop, value);
175+
set onError(value) => _initListener('error', element.onError, value);
176+
set onFocus(value) => _initListener('focus', element.onFocus, value);
177+
set onFullscreenChange(value) => _initListener('fullscreenchange',element.onFullscreenChange, value);
178+
set onFullscreenError(value) => _initListener('fullscreenerror', element.onFullscreenError, value);
179+
set onInput(value) => _initListener('input', element.onInput, value);
180+
set onInvalid(value) => _initListener('invalid', element.onInvalid, value);
181+
set onKeyDown(value) => _initListener('keydown', element.onKeyDown, value);
182+
set onKeyPress(value) => _initListener('keypress', element.onKeyPress, value);
183+
set onKeyUp(value) => _initListener('keyup', element.onKeyUp, value);
184+
set onLoad(value) => _initListener('load', element.onLoad, value);
185+
set onMouseDown(value) => _initListener('mousedown', element.onMouseDown, value);
186+
set onMouseEnter(value) => _initListener('mouseenter', element.onMouseEnter, value);
187+
set onMouseLeave(value) => _initListener('mouseleave', element.onMouseLeave, value);
188+
set onMouseMove(value) => _initListener('mousemove', element.onMouseMove, value);
189+
set onMouseOut(value) => _initListener('mouseout', element.onMouseOut, value);
190+
set onMouseOver(value) => _initListener('mouseover', element.onMouseOver, value);
191+
set onMouseUp(value) => _initListener('mouseup', element.onMouseUp, value);
192+
set onMouseWheel(value) => _initListener('mousewheel', element.onMouseWheel, value);
193+
set onPaste(value) => _initListener('paste', element.onPaste, value);
194+
set onReset(value) => _initListener('reset', element.onReset, value);
195+
set onScroll(value) => _initListener('scroll', element.onScroll, value);
196+
set onSearch(value) => _initListener('search', element.onSearch, value);
197+
set onSelect(value) => _initListener('select', element.onSelect, value);
198+
set onSelectStart(value) => _initListener('selectstart', element.onSelectStart, value);
199+
//set onSpeechChange(value) => _initListener('speechchange', element.onSpeechChange, value);
200+
set onSubmit(value) => _initListener('submit', element.onSubmit, value);
201+
set onTouchCancel(value) => _initListener('touchcancel', element.onTouchCancel, value);
202+
set onTouchEnd(value) => _initListener('touchend', element.onTouchEnd, value);
203+
set onTouchEnter(value) => _initListener('touchenter', element.onTouchEnter, value);
204+
set onTouchLeave(value) => _initListener('touchleave', element.onTouchLeave, value);
205+
set onTouchMove(value) => _initListener('touchmove', element.onTouchMove, value);
206+
set onTouchStart(value) => _initListener('touchstart', element.onTouchStart, value);
207+
set onTransitionEnd(value) => _initListener('transitionend', element.onTransitionEnd, value);
209208
}

‎lib/mock/test_bed.dart

+25-4
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,36 @@ class TestBed {
7070
}
7171

7272
/**
73-
* Triggern a specific DOM element on a given node to test directives
73+
* Trigger specific DOM element on a given node to test directives
7474
* which listen to events.
7575
*/
76-
triggerEvent(element, name, [type='MouseEvent']) {
77-
element.dispatchEvent(new Event.eventType(type, name));
78-
// Since we are manually triggering event we need to simpulate apply();
76+
triggerEvent(Element element, name, [type='MouseEvent']) {
77+
var event = new Event.eventType(type, name);
78+
var isAttached = _isRenderTreeAttached(element);
79+
if (!isAttached) {
80+
var topElement = element;
81+
while(topElement.parentNode != null) topElement = topElement.parentNode;
82+
Element appElement = injector.get(Node);
83+
appElement.append(topElement);
84+
element.dispatchEvent(event);
85+
topElement.remove();
86+
} else {
87+
element.dispatchEvent(event);
88+
}
89+
90+
// Since we are manually triggering event we need to simulate apply();
7991
rootScope.apply();
8092
}
8193

94+
bool _isRenderTreeAttached(Node node) {
95+
var owner = window.document;
96+
while (node != null) {
97+
if (node == owner) return true;
98+
node = node.parentNode;
99+
}
100+
return false;
101+
}
102+
82103
/**
83104
* Select an [OPTION] in a [SELECT] with a given name and trigger the
84105
* appropriate DOM event. Used when testing [SELECT] controlls in forms.

‎test/directive/input_select_spec.dart

+4-3
Original file line numberDiff line numberDiff line change
@@ -475,12 +475,13 @@ main() {
475475
expect(element).toEqualSelect(['not me', ['me!'], 'nah']);
476476
});
477477

478-
it('should fire ng-change event.', () {
478+
iit('should fire ng-change event only when user action', () {
479479
var log = '';
480480
compile(
481481
'<select name="select" ng-model="selection" ng-change="change()">' +
482482
'<option value=""></option>' +
483483
'<option value="c">C</option>' +
484+
'<option value="d">D</option>' +
484485
'</select>');
485486

486487
scope.context['change'] = () {
@@ -491,9 +492,9 @@ main() {
491492
scope.context['selection'] = 'c';
492493
});
493494

494-
element.value = 'c';
495+
element.value = 'd';
495496
_.triggerEvent(element, 'change');
496-
expect(log).toEqual('change:c;');
497+
expect(log).toEqual('change:d;');
497498
});
498499

499500

0 commit comments

Comments
 (0)