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

Properly hide checkbox and radio inputs in button groups #14559

Merged
merged 1 commit into from Sep 8, 2014

Conversation

Projects
None yet
3 participants
@hnrch02
Copy link
Member

hnrch02 commented Sep 7, 2014

pointer-events: none; for modern browsers (including IE11+), clip: rect(1px, 1px, 1px, 1px); for everything else.

Maybe @mdo can give some input on clip, I found various resources online promoting 1px, 1px, 1px, 1px instead of 0, 0, 0, 0.

Fixes #14137.
Per #14137 (comment).

@cvrebert cvrebert added the css label Sep 7, 2014

@cvrebert cvrebert added this to the v3.2.1 milestone Sep 7, 2014

@cvrebert

This comment has been minimized.

Copy link
Member

cvrebert commented Sep 8, 2014

Elsewhere, we're currently using clip: rect(0,0,0,0);
Also, switching to clipping to 0 would make the opacity setting unnecessary, right?
Might need to update the explanatory comment that precedes this chunk of declarations.

@hnrch02

This comment has been minimized.

Copy link
Member Author

hnrch02 commented Sep 8, 2014

Elsewhere, we're currently using clip: rect(0,0,0,0);

Interestingly, the article linked in the explanatory comment for the .sr-only class also uses clip: rect(1px, 1px, 1px, 1px);.

Also, switching to clipping to 0 would make the opacity setting unnecessary, right?

I suppose so, yes.

Might need to update the explanatory comment that precedes this chunk of declarations.

Will do.

Properly hide checkbox and radio inputs in button groups
`pointer-events: none;` for modern browsers (including IE11+), `clip: rect(1px, 1px, 1px, 1px);` for everything else.

Fixes #14137

@hnrch02 hnrch02 force-pushed the hnrch02:properly-hide-btngroup-checkbox-n-radio branch to 475152a Sep 8, 2014

@mdo

This comment has been minimized.

Copy link
Member

mdo commented Sep 8, 2014

Nice, this sounds reasonable to me. Thanks for updating that comment, too.

I'd love to hear more about the clip differences, but that's not hugely important for this PR. So long as they're the same, I'm good.

@hnrch02

This comment has been minimized.

Copy link
Member Author

hnrch02 commented Sep 8, 2014

I don't have any experience with clip, just reciting what I've read online.

@mdo

This comment has been minimized.

Copy link
Member

mdo commented Sep 8, 2014

Just one more question—this means we don't need to revert the commit mentioned in #14137 (comment), right?

If not, merge away @hnrch02!

@hnrch02

This comment has been minimized.

Copy link
Member Author

hnrch02 commented Sep 8, 2014

Nope, that would just have been a dirty workaround.

@hnrch02

This comment has been minimized.

Copy link
Member Author

hnrch02 commented Sep 8, 2014

@cvrebert Comment wording LGTY?

@mdo

This comment has been minimized.

Copy link
Member

mdo commented Sep 8, 2014

Comment looks great to me. :shipit:

hnrch02 added a commit that referenced this pull request Sep 8, 2014

Merge pull request #14559 from hnrch02/properly-hide-btngroup-checkbo…
…x-n-radio

Properly hide checkbox and radio inputs in button groups

@hnrch02 hnrch02 merged commit 285feb4 into twbs:master Sep 8, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@hnrch02 hnrch02 deleted the hnrch02:properly-hide-btngroup-checkbox-n-radio branch Sep 8, 2014

@hnrch02

This comment has been minimized.

Copy link
Member Author

hnrch02 commented Sep 8, 2014

Woohoo, first merge 🎉

@mdo

This comment has been minimized.

Copy link
Member

mdo commented Sep 8, 2014

Aww yeah.

@mdo

This comment has been minimized.

Copy link
Member

mdo commented Sep 8, 2014

@hnrch02 Can you add a note to #13952 under the appropriate section? Just issue number and basic commit message/summary.

@hnrch02

This comment has been minimized.

Copy link
Member Author

hnrch02 commented Sep 8, 2014

Sure, was wondering how that is handled. Always thought it was a bot due to the insane speed of @cvrebert.

@cvrebert cvrebert referenced this pull request Sep 8, 2014

Closed

v3.3.0 ship list #13952

@mdo

This comment has been minimized.

Copy link
Member

mdo commented Sep 8, 2014

The cross reference in the timeline here will always show up as @cvrebert since he opened that issue. Anytime you merge something worth noting, you should update the ship list.

And on the note of it being a bot, I'd love it if it was done by us at GitHub, but that's for another time :).

@hnrch02

This comment has been minimized.

Copy link
Member Author

hnrch02 commented Sep 8, 2014

Got it. And looking forward to more GitHub awesomeness in the future. 😁

stempler pushed a commit to stempler/bootstrap that referenced this pull request Nov 4, 2014

(less) Merge pull request twbs#14559 from hnrch02/properly-hide-btngr…
…oup-checkbox-n-radio

Properly hide checkbox and radio inputs in button groups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.