-
Notifications
You must be signed in to change notification settings - Fork 34
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
click handler returns false #6
click handler returns false #6
Conversation
Awesome! Thank you for your feedback. Could you rebase this since i've just merged your previous pull request. Otherwise I'll just copy your change when I have some time to give it a quick run. |
Rebased. Thanks! |
I manually made this change to my setup and this did not fix the issue. Any ideas? |
Man does this project need a test suite. I could've sworn it was working out here. I'll look into it when I get a chance... |
Exactly. It's still on my TODO-list with the one you suggested in pull request 4 |
No ideas, @jonathansimmons. I just retested it in one of my apps in the latest Safari, Chrome and Firefox and it really seems to be working. Didn't test IE because I didn't feel like warming up my VM, and I've never had problems with this behavior on IE (...or any browser, really). We'll know more when there's a test suite. @rvanlieshout, I was hoping to surprise you with a konacha test suite PR, but all of a sudden I'm swamped. Probably won't crawl out from under it for another 2+ weeks. |
@taavo, feeling better already? Has anybody got the time to test this in IE yet? |
Yeah, it's still working out here. Just tested IE8, and it works. I've had this branch running on a pretty active staging server since I issued the pull request, and haven't heard of any problems. Is it working in your tests, @rvanlieshout? (And, yeah, I'm around. But I may not have the time to set up a test suite for you guys for a while.) |
click handler returns false
Then let's get this merged. A new branch is created with a first attempt to get this gem tested using Jasmine. I've tried konacha, but it seems to force me to either create a separate testing app or include Rails in this gem. |
If you have a link with
remote: true
, currently confirming it with twitter-bootstrap-rails-confirm causes a "#" to be added to the location. This is because the ".proceed" click handler returns true, allowing the default "a" click handler to fire, so the href "#" gets visited. Nothing breaks if we return false, however, because that's what thecallback()
method is for.I've tested this on one of my apps, and it appears to work fine for both
remote: true
andremote: false
.