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

jquery__pointernative: pointerenter / pointerleave events bubble up to the document root #802

Merged
merged 1 commit into from
Jan 19, 2015

Conversation

narqo
Copy link
Member

@narqo narqo commented Jan 18, 2015

fix #801

/cc @dfilatov

@narqo
Copy link
Member Author

narqo commented Jan 18, 2015

Note, the only browsers I've tested it are Chrome and Fx.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) when pulling 82ba2cd on issues/801@v2 into 8fa8beb on v2.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) when pulling 6949d0f on issues/801@v2 into 8fa8beb on v2.

@dfilatov
Copy link
Member

Could you add spec for pointerleave?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.12%) when pulling 30a8570 on issues/801@v2 into 8fa8beb on v2.

@narqo
Copy link
Member Author

narqo commented Jan 19, 2015

@dfilatov we already had simple tests for pointerleave / pointerleave events. But we didn't test event bubbling.

🆙

@narqo
Copy link
Member Author

narqo commented Jan 19, 2015

BTW: It seems that no one of currently popular Pointer Events Polyfils (MS, Polymer or jquery's PEP) would pass this test case, as they use native bubbling = false and dispatch the event on the last nested target only 🤘

var target = event.target,
relatedTarget = event.relatedTarget;

while(target && !this.contains(target, relatedTarget)) {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it expensive? contains has inner cycle as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, yep, haven't thought about it :(

What if I rewrite it like this:

if(!this.contains(target, relatedTarget)) {
  do {
    event.target = target;
    fn.call(this, event);  // we need to emit non bubbling events here on the subtree from target to relatedTarget
  } while(target = target.parentNode && target !== relatedTarget)
}

?

Copy link
Member

Choose a reason for hiding this comment

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

imho looks better

@narqo
Copy link
Member Author

narqo commented Jan 19, 2015

🆙

this.enterLeave(event, this.enter);
},

enterLeave : function(event, fn) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't like the name of the method, but I couldn't decide how to call it :(

@dfilatov
Copy link
Member

👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.12%) when pulling 26bce3a on issues/801@v2 into 96fc868 on v2.

@dfilatov
Copy link
Member

@narqo Don't forget do the same PR to v3

narqo pushed a commit that referenced this pull request Jan 19, 2015
jquery__pointernative: pointerenter / pointerleave events bubble up to the document root
@narqo narqo merged commit f58bfc5 into v2 Jan 19, 2015
@narqo narqo deleted the issues/801@v2 branch January 19, 2015 16:36
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.

3 participants