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

Avoid appending custom proceed class to btn-primary, refactored dialog JS #17

Merged
merged 2 commits into from
Oct 29, 2013
Merged

Conversation

stevelacey
Copy link
Contributor

I largely refactored this to make a very basic change - to make it so that setting data-confirm-proceed-class overrides btn-primary as opposed to appending after it, as this doesn't style the element correctly, for example, a button with class btn-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.

I largely refactored this to make a very basic change - to make it so that setting `data-confirm-proceed-class` overrides `btn-primary` as opposed to appending after it, as this doesn't style the element correctly, for example, a button with class `btn-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!).
I am impressed this even worked.
@stevelacey
Copy link
Contributor Author

@bluerail @rvanlieshout ?

@rvanlieshout
Copy link
Member

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!

@stevelacey
Copy link
Contributor Author

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//pull/17#issuecomment-27075810
.

Steve Laceyhttp://stevelacey.net@stevelaceyhttp://www.twitter.com/stevelacey
/ steve@stevelacey.net

rvanlieshout added a commit that referenced this pull request Oct 29, 2013
Avoid appending custom proceed class to btn-primary, refactored dialog JS
@rvanlieshout rvanlieshout merged commit df0c5ff into bluerail:master Oct 29, 2013
@rvanlieshout
Copy link
Member

Going to fix #18 and wait for confirmation before releasing a new version.

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.

2 participants