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

Ignore events with prevented defaults #379

Merged
merged 1 commit into from May 12, 2014

Conversation

marcandre
Copy link
Contributor

If a click event has a prevented default (say because some other JS has already handled the event doing something else) then pjax should ignore the event.

It seems obvious to me, but if further justification is needed, not ignoring the event behaves differently than the fallback behavior (non pushState browsers or if calling $.pjax.disable()).

ok(false, "Event should have been ignored")
})
frame.$("a[href='/dinosaurs.html']").on("click", function(event) {
event.preventDefault()
Copy link
Collaborator

Choose a reason for hiding this comment

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

The test should be unpaused here after a timeout, to allow potential async stuff to happen (even though we don't expect it).

So, instead of a start() at the end of the test, you shoud add something like setTimeout(start, 10) here in the click handler. This way we can be sure that the click actually happened.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, @marcandre, this will be good to go as soon as you improve this test in the manner that I described. Let me know if you won't have a chance to do it. Thanks!

@mislav
Copy link
Collaborator

mislav commented May 6, 2014

Looks alright. @josh do you think this is in order?

@josh
Copy link
Contributor

josh commented May 6, 2014

👍

@marcandre
Copy link
Contributor Author

Sure, test improved.

})

frame.$("a[href='/dinosaurs.html']").click()
setTimeout(function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Close, but not there yet. What I now want you to do is move this entire setTimeout block to inside of the click handler for the a[href='/dinosaurs.html'] element. This is so we can be sure that the click handler has actually ran.

Also, you can reduce the timeout from 100ms to something smaller like 10ms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

mislav added a commit that referenced this pull request May 12, 2014
Ignore events with prevented defaults
@mislav mislav merged commit 97bccf9 into defunkt:master May 12, 2014
@mislav
Copy link
Collaborator

mislav commented May 12, 2014

Thanks!

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.

None yet

3 participants