Closed stevelacey closed 10 years ago
@bluerail @rvanlieshout ?
Hi @stevelacey
I hadn't have any time yet to really look at your submission and even as much as a agree with your changes on the btn-primary I still feel the need to overlook this. I'll give this a go next week!
Thanks for your additions!
Okay no hurry, just checking it was on someone's radar. I am already using a similar custom version in production and have added a few things that may well enhance this, but even so, here is a good starting point.
On Friday, October 25, 2013, Rene van Lieshout wrote:
Hi @stevelacey https://github.com/stevelacey
I hadn't have any time yet to really look at your submission and even as much as a agree with your changes on the btn-primary I still feel the need to overlook this. I'll give this a go next week!
Thanks for your additions!
— Reply to this email directly or view it on GitHubhttps://github.com/bluerail/twitter-bootstrap-rails-confirm/pull/17#issuecomment-27075810 .
Steve Laceyhttp://stevelacey.net@stevelaceyhttp://www.twitter.com/stevelacey / steve@stevelacey.net
Going to fix https://github.com/bluerail/twitter-bootstrap-rails-confirm/issues/18 and wait for confirmation before releasing a new version.
I largely refactored this to make a very basic change - to make it so that setting
data-confirm-proceed-class
overridesbtn-primary
as opposed to appending after it, as this doesn't style the element correctly, for example, a button with classbtn-primary btn-danger
can end up primary styled, they're not for use in conjunction.Other than that, this should work exactly as it did before, just a hell of a lot easier to read (I hope!).
Also, saves re-querying for DOM elements you've already seen - lots and lots of selectors before - I expect this performs faster.