Added "none" as a label position option and now respects the checkbox's disabled state #13

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
@trebuchetty

I've made a couple useful changes to hold the project over until more formal features have been implemented.

One change is to respect the original checkbox's disabled state. I added a new class to give the checkbox a disabled look by using opacity and made the link unclickable. I think the better approach to the disabled look would be extending the checkbox image to include disabled versions, but I'm not good enough with photoshop to update the psd.

The other addition was to make a label position of none so that a label isn't displayed when it's not required.

I've never done a pull request before, so I hope this works.

@arthurgouveia

This comment has been minimized.

Show comment Hide comment
@arthurgouveia

arthurgouveia Apr 16, 2013

Owner

Hey, man!

I'm not sure why you wouldn't require a label for your input, but I believe this should be encouraged and therefore I'd like to keep the option of not giving the checkable a label outside the master branch.

I've just commited some changes that will add the disabled state for the checkbox and also you now can enable/disable by using methods.

I hope this is ok for you. Let me know and close the issue/pull request if you think this is solved.

Thanks!

Owner

arthurgouveia commented Apr 16, 2013

Hey, man!

I'm not sure why you wouldn't require a label for your input, but I believe this should be encouraged and therefore I'd like to keep the option of not giving the checkable a label outside the master branch.

I've just commited some changes that will add the disabled state for the checkbox and also you now can enable/disable by using methods.

I hope this is ok for you. Let me know and close the issue/pull request if you think this is solved.

Thanks!

@trebuchetty

This comment has been minimized.

Show comment Hide comment
@trebuchetty

trebuchetty Apr 21, 2013

Not requiring a label was more of a custom change that I specifically needed for my project. I have a grid like area where these checkboxes represent days of the week. I didn't need a label as it was explicit that each checkbox represented each day and when an empty string was given for the label, the label still existed, so it would push the checkbox over slightly, making my checkboxes misaligned. I felt adding the "no label" code was cleaner than some css hack to properly re-align the checkboxes.

Your changes look good, so I'll close this. You did leave a console.log('sdf') in the code that should probably be removed though.

Not requiring a label was more of a custom change that I specifically needed for my project. I have a grid like area where these checkboxes represent days of the week. I didn't need a label as it was explicit that each checkbox represented each day and when an empty string was given for the label, the label still existed, so it would push the checkbox over slightly, making my checkboxes misaligned. I felt adding the "no label" code was cleaner than some css hack to properly re-align the checkboxes.

Your changes look good, so I'll close this. You did leave a console.log('sdf') in the code that should probably be removed though.

@arthurgouveia

This comment has been minimized.

Show comment Hide comment
@arthurgouveia

arthurgouveia Apr 21, 2013

Owner

Damn! Thanks for that!
On Apr 21, 2013 2:10 PM, "trebuchetty" notifications@github.com wrote:

Not requiring a label was more of a custom change that I specifically
needed for my project. I have a grid like area where these checkboxes
represent days of the week. I didn't need a label as it was explicit that
each checkbox represented each day and when an empty string was given for
the label, the label still existed, so it would push the checkbox over
slightly, making my checkboxes misaligned. I felt adding the "no label"
code was cleaner than some css hack to properly re-align the checkboxes.

Your changes look good, so I'll close this. You did leave a
console.log('sdf') in the code that should probably be removed though.


Reply to this email directly or view it on GitHubhttps://github.com/arthurgouveia/prettyCheckable/pull/13#issuecomment-16734731
.

Owner

arthurgouveia commented Apr 21, 2013

Damn! Thanks for that!
On Apr 21, 2013 2:10 PM, "trebuchetty" notifications@github.com wrote:

Not requiring a label was more of a custom change that I specifically
needed for my project. I have a grid like area where these checkboxes
represent days of the week. I didn't need a label as it was explicit that
each checkbox represented each day and when an empty string was given for
the label, the label still existed, so it would push the checkbox over
slightly, making my checkboxes misaligned. I felt adding the "no label"
code was cleaner than some css hack to properly re-align the checkboxes.

Your changes look good, so I'll close this. You did leave a
console.log('sdf') in the code that should probably be removed though.


Reply to this email directly or view it on GitHubhttps://github.com/arthurgouveia/prettyCheckable/pull/13#issuecomment-16734731
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment