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

Added styles to enable input state toggle buttons #2284

Closed
wants to merge 2 commits into from

Conversation

charettes
Copy link
Contributor

The following styles allow usage of fully functional toggle buttons that rely on inputs instead of js to persist state. IMHO it's a semantically better way of defining toggle buttons.

Before writing any doc/examples I was eager to get some feedback about this approach.

A live example can be seen here http://jsfiddle.net/charettes/SauLj/.

This issue was discussed in #1161

@mdo
Copy link
Member

mdo commented Feb 28, 2012

Instead of placing the input and label as siblings, make the label wrap the checkbox. Less CSS that way.

@charettes
Copy link
Contributor Author

@markdotto this screw the whole point of relying only on css? There's no such selector as label:contains(input:checked)...

@mdo
Copy link
Member

mdo commented Mar 12, 2012

Ah, yes, sorry—forgot about that for the checked state. That said, how do you see this working in IE7? Don't think that selector works there either. Can you double check all supported browsers?

@mdo
Copy link
Member

mdo commented Mar 23, 2012

Looking at this again, we can't merge it in since :checked isn't supported in IE7 and IE8. If you have other ideas on how to do this (perhaps the attribute selectors?) we'll happily take them. Otherwise I'll close this out and punt on it until a future feature release.

@charettes
Copy link
Contributor Author

What about using input[checked] with javascript :checked feature detection that would fallback to

$(document).on('change', 'input:radio, input:checkbox', function(){
  if (this.checked) $(this).attr('checked', 'checked');
  else $(this).removeAttr('checked');
});

@mdo
Copy link
Member

mdo commented Mar 23, 2012

Sure, give it a go. Unsure if IE7/8 will support [checked], but I think it will if I recall. Are you able to test IE7/8/9?

@charettes
Copy link
Contributor Author

I've got a VM with IE7, I'll make sure it works on that awesome browser. I think IE7 has attribute support so it should work. Some one will have to confirm it works for IE8 since I can't get my hands on it without updating the one in my VM, it should already work with IE9.

@charettes
Copy link
Contributor Author

Never mind, I can't get it to work on IE7 nor IE8... it seems like the change event is not triggered when an input is display:none or visibility:hidden ... The only hack I could think of is binding to the label.btn[for]'s click but it looks like there's a race condition when trying to read the checked attribute of the related input.

It's sad but I think the web is not yet ready for this. Curse you IE.

@charettes charettes closed this Mar 23, 2012
@kwilliams
Copy link

I'm working on a project that would benefit from this, after a little trial and error I have a version that works for me in IE7 with a few modifications.

To get around the display:none; and visibility:hidden; issue you can just give the inputs a negative margin. This allows the event to fire. Also, instead of relying on pure css, we can just add the active class in our jQuery listener. This part is a little hacky but it works.

For the working example view: http://jsfiddle.net/kwilliams_texstarweb/sfnY5/

@charettes
Copy link
Contributor Author

@kwilliams Whoa great idea! I gave up too early :D Do you think you can bundle this as a new pull request? If you need help with this maybe I can help.

@kwilliams
Copy link

After a little more fiddling the link below seems to perform well in all of my tests. Now all we need to find is an elegant way to fit this into bootstrap-buttons.js. Any thoughts?

http://jsfiddle.net/kwilliams_texstarweb/B6nqw/12/

@charettes I'd be happy to put together a pull request, but it will probably be this weekend until I can get to it.

@charettes
Copy link
Contributor Author

I think it would be great if this active js shim kick in only on IE so other browsers don't have that extra JS running for no reason. @markdotto What do you think of this idea?

@kwilliams
Copy link

I was able to clean this up quite a bit by iterating through label buttons toggling actives and then binding the change event. I think its ready... Thoughts?

http://jsfiddle.net/kwilliams_texstarweb/LTS5u/4/

@alexgann
Copy link

alexgann commented Jun 7, 2012

Maybe not the right place for this - but I just wanted to say thanks for doing this work - for me, the radio button groups are only useful if attached to an input element. @charettes css only solution is very nice.

And almost certainly not the right place for this, but for the past 3 months IE7 is around 2% browser-share - even at the large slow corporation I work for they're at IE8. Maybe bootstrap 2.1 could drop IE7 support and the workarounds could be cleansed.

@charettes charettes mentioned this pull request Jun 14, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants