Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix event-outside delegate and an error when defining and using `tapoutside` #1608

Open
wants to merge 2 commits into from

3 participants

@dpobel

First, I'm sorry for the pull request mixing two different issues, it happens that I found that the outside delegate does not work at all and does not support filtering with a selector while fixing the tapoutside issue and to properly test it, I need the delegate method to work correctly...

Anyway, this PR does 2 things:

  • fix event-outside delegate to work with and without a filter selector (fix the tests and add new tests)
  • fix the "has no method 'isOutside'" when defining and using tapoutsite (with a regression test)
@tilomitra

Thanks for the PR @dpobel! What browsers did you test this on?

@dpobel

Hi @tilomitra

to be honest before your comment, I tested only with PhantomJS, Firefox (Linux) and Chrome (Linux) without any problem. So, in addition, I've just tested with yeti on Firefox Android, Chrome Android, Chrome Window7, IE8, IE9 and IE10 (that's all I have on my machines at the moment :)) and unfortunately, the unit tests on tapoutside fail for:

  • Firefox Android
  • IE 10

for Firefox Android, I have a message about simulateTouchEvent not being supported but since it's not listed in the supported env. so I guess that's OK for now.

for IE10, my quick manual tests confirm that tapoutside is working as expected (at least with on('tapoutside')), so it seems like the issue lies in the simulateGesture('tap'), is it a known issue ? I've noticed that it's not much used in others unit tests and you usually test the tap event in a different way with the definition of lower level helpers, is it the way to go?

@F21
F21 commented

Any updates on this? I am finding that delegate doesn't work with event outside being a bit of a limitation.

@dpobel

I had not the time to work on this in the mean time and the fact the tests are failing in IE10 is not a big source of motivation;-)
More seriously, I'll try to find some time this week to at least dig what is wrong in IE10 and simulateGesture.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
22 src/event/js/outside.js
@@ -66,7 +66,7 @@ Y.Event.defineOutside = function (event, name) {
on: function (node, sub, notifier) {
sub.handle = Y.one('doc').on(event, function(e) {
- if (this.isOutside(node, e.target)) {
+ if (config.isOutside(node, e.target)) {
e.currentTarget = node;
notifier.fire(e);
}
@@ -78,17 +78,27 @@ Y.Event.defineOutside = function (event, name) {
},
delegate: function (node, sub, notifier, filter) {
- sub.handle = Y.one('doc').delegate(event, function (e) {
- if (this.isOutside(node, e.target)) {
+ sub.handle = Y.one('doc').on(event, function (e) {
+ if (config.isOutside(node, e.target, filter)) {
+ e.currentTarget = node;
notifier.fire(e);
}
- }, filter, this);
+ }, this);
},
- isOutside: function (node, target) {
- return target !== node && !target.ancestor(function (p) {
+ isOutside: function (node, target, filter) {
+ var parents;
+
+ if ( !filter ) {
+ return target !== node && !target.ancestor(function (p) {
return p === node;
});
+ } else {
+ parents = node.all(filter);
+ return parents.indexOf(target) === -1 && !target.ancestor(function (p) {
+ return parents.indexOf(p) !== -1;
+ });
+ }
}
};
config.detachDelegate = config.detach;
View
116 src/event/tests/unit/assets/outside-tests.js
@@ -7,37 +7,131 @@ YUI.add('outside-tests', function(Y) {
node = Y.one('#tester');
- //Only testing "clickoutside" since it's all the same logic.
+ // Only testing the full logic with "clickoutside" as it's always the same
+ // "tapoutside" regression test just checks that the "has no method
+ // 'isOutside'" error is fixed.
suite.add(new Y.Test.Case({
name: 'outside events',
'test: on': function() {
- var fired;
- var handle = node.on('clickoutside', function(e) {
+ var fired, handle;
+
+ handle = node.on('clickoutside', function(e) {
fired = true;
Assert.areSame('clickoutside', e.type);
Assert.areSame(node, e.currentTarget);
Assert.areSame(body, e.target);
- handle.detach();
});
body.simulate('click');
-
Assert.isTrue(fired);
+
+ fired = false;
+ node.one('.included').simulate('click');
+ Assert.isFalse(fired);
+
+ handle.detach();
},
- 'test: delegate': function() {
- var fired;
- var handle = node.delegate('clickoutside', function(e) {
+
+ 'test: delegate without selector': function() {
+ var fired, handle;
+
+ handle = node.delegate('clickoutside', function(e) {
fired = true;
Assert.areSame('clickoutside', e.type);
- Assert.areSame(node, e.currentTarget);
- Assert.areSame(body, e.target);
- handle.detach();
+ Assert.areSame(node, e.currentTarget, 'e.currentTarget should be the node');
+ Assert.areSame(body, e.target, 'e.target should be the body element');
});
body.simulate('click');
+ Assert.isTrue(fired);
+
+ fired = false;
+ node.simulate('click');
+ Assert.isFalse(fired);
+
+ fired = false;
+ node.one('.included').simulate('click');
+ Assert.isFalse(fired);
+
+ handle.detach();
+ },
+
+ 'test: delegate with selector': function() {
+ var fired, handle, clickTarget,
+ filter = '.included',
+ included = node.one(filter),
+ inIncludedDiv = node.one('.included div'),
+ outIncludedDiv = node.one('.excluded div');
+
+ handle = node.delegate('clickoutside', function(e) {
+ fired = true;
+ Assert.areSame('clickoutside', e.type);
+ Assert.areSame(node, e.currentTarget, 'e.currentTarget should be the node');
+ Assert.areSame(clickTarget, e.target, 'e.target should be the click target');
+ }, filter);
+
+ clickTarget = outIncludedDiv;
+ outIncludedDiv.simulate('click');
+ Assert.isTrue(fired);
+
+ fired = false;
+ clickTarget = node;
+ node.simulate('click');
+ Assert.isTrue(fired);
+ fired = false;
+ included.simulate('click');
+ Assert.isFalse(fired);
+
+ fired = false;
+ inIncludedDiv.simulate('click');
+ Assert.isFalse(fired);
+
+ handle.detach();
+ },
+ }));
+
+ suite.add(new Y.Test.Case({
+ name: 'tapoutside event regression test',
+
+ setUp: function () {
+ Y.Event.defineOutside('tap');
+ },
+
+ 'test: on': function() {
+ var handle, that = this;
+
+ handle = node.on('tapoutside', function (e) {
+ that.resume(function () {
+ Assert.areSame('tapoutside', e.type);
+ Assert.areSame(node, e.currentTarget);
+ Assert.areSame(body, e.target);
+ handle.detach();
+ });
+ });
+
+ body.simulateGesture('tap');
+
+ this.wait();
+ },
+
+ 'test: delegate': function() {
+ var handle, that = this;
+
+ handle = node.delegate('tapoutside', function (e) {
+ that.resume(function () {
+ Assert.areSame('tapoutside', e.type);
+ Assert.areSame(node, e.currentTarget);
+ Assert.areSame(body, e.target);
+ handle.detach();
+ });
+ });
+
+ body.simulateGesture('tap');
+ this.wait();
}
+
}));
Y.Test.Runner.add(suite);
View
13 src/event/tests/unit/event-outside.html
@@ -7,7 +7,16 @@
<div id="log"></div>
-<div id="tester"></div>
+<div id="tester">
+ <div class="included">
+ Included
+ <div>Child included</div>
+ </div>
+ <div class="excluded">
+ Excluded
+ <div>Child excluded</div>
+ </div>
+</div>
<script type="text/javascript" src="../../../../build/yui/yui.js"></script>
<script>
@@ -17,7 +26,7 @@
modules: {
'outside-tests': {
fullpath: './assets/outside-tests.js',
- requires: [ 'event-outside', 'node-event-simulate', 'test-console', 'test' ]
+ requires: [ 'event-outside', 'node-event-simulate', 'event-tap', 'test-console', 'test' ]
}
}
}).use('outside-tests', function(Y) {
Something went wrong with that request. Please try again.