Styles for [input:disabled] on checkboxes and radio. Issue #17918 #17921

markau · 2015-10-14T02:34:26Z

cvrebert · 2015-10-14T04:48:36Z

If possible, exclude dist/css/bootstrap.css* and docs.min.css* from the PR.

cvrebert · 2015-10-14T04:52:04Z

Also, it's not required, but you might want to associate the email address you used for your commit with your GitHub account so that you'll get credit for it in your GitHub profile page.

markau · 2015-10-14T05:32:56Z

Thanks for the Git help, @cvrebert, much appreciated. I have added the email to my GitHub profile (and updated git config with the preferred one so this doesn't happen again).

I tried to git rm the files, then commit with the --ammend flag to correct the PR and remove those files. I think I just made a mess of my local repo though. Should I try some more? (also, would a push following an ammended commit update the email address used for the commit?)

I'd like to get this right, but don't want to waste your time if it's going to be complicated.

Thank you!

markau · 2015-10-14T05:47:13Z

Ok I realized the commits had not been pushed. I think the commit is now correct (i.e. rm'd the dist\css files). Interested in how it looks to you, @cvrebert. Thank you!

cvrebert · 2015-10-14T05:55:20Z

Ok I realized the commits had not been pushed. I think the commit is now correct (i.e. rm'd the dist\css files). Interested in how it looks to you

Doesn't quite look right. (The files I mentioned shouldn't be in the diff at all.)

Should I try some more?

Nah, that's all right. We can fix it on our end when merging this.
(We'll use "git rebase" to do the fixing; you can google around for info on rebasing if you're interested, but it's somewhat advanced.)

(also, would a push following an amended commit update the email address used for the commit?)

To update the author info, you'd need to do git commit --amend --reset-author , and then do git push --force (which should be used with caution since it can "delete" (I'm oversimplifying) commits, but should be safe in this case).

markau · 2015-10-14T07:26:51Z

Thanks for the info, Chris. I will have a go at updating the email tomorrow when back in the office.

cvrebert · 2015-10-15T23:37:56Z

scss/_custom-forms.scss

@@ -83,6 +83,10 @@
    background-image: url(data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4NCjwhLS0gR2VuZXJhdG9yOiBBZG9iZSBJbGx1c3RyYXRvciAxNy4xLjAsIFNWRyBFeHBvcnQgUGx1Zy1JbiAuIFNWRyBWZXJzaW9uOiA2LjAwIEJ1aWxkIDApICAtLT4NCjwhRE9DVFlQRSBzdmcgUFVCTElDICItLy9XM0MvL0RURCBTVkcgMS4xLy9FTiIgImh0dHA6Ly93d3cudzMub3JnL0dyYXBoaWNzL1NWRy8xLjEvRFREL3N2ZzExLmR0ZCI+DQo8c3ZnIHZlcnNpb249IjEuMSIgaWQ9IkxheWVyXzEiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgeG1sbnM6eGxpbms9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGxpbmsiIHg9IjBweCIgeT0iMHB4Ig0KCSB3aWR0aD0iOHB4IiBoZWlnaHQ9IjhweCIgdmlld0JveD0iMCAwIDggOCIgZW5hYmxlLWJhY2tncm91bmQ9Im5ldyAwIDAgOCA4IiB4bWw6c3BhY2U9InByZXNlcnZlIj4NCjxwYXRoIGZpbGw9IiNGRkZGRkYiIGQ9Ik0wLDN2Mmg4VjNIMHoiLz4NCjwvc3ZnPg0K);
    @include box-shadow(none);
  }
+
+  input:disabled ~ .c-indicator {
+    background-color: #bbb;


Should probably use a variable for this color

markau · 2015-10-15T23:50:00Z

I did wonder that and I'll take a look. Thanks for your patience, Chris. I have used open source software for years and this is my first meaningful contribution to a project, so I am happy to make the effort to get it right.

If I make a change to use a variable, would it be a new commit/push (referencing this issue number?)

cvrebert · 2015-10-16T00:18:38Z

No problem! 😄 Thanks for giving back to the community ❤️

The variable declaration should be added to _variables.scss

If I make a change to use a variable, would it be a new commit/push (referencing this issue number?)

Yes, that'd be fine.

markau · 2015-10-16T00:56:21Z

I'll include the doc changes in the commit, but that is probably the most subjective part because you might not like the wording. Ideally, I would show you a diff of the changes for review prior to committing. How best to achieve this?

cvrebert · 2015-10-16T01:07:53Z

Eh, you might as well just commit your suggestion and then depending on the feedback either you can push more fixup commits or we can fix it ourselves when we go to merge it. In any case, we'll squash it into a single commit on our end when we merge this PR.

markau · 2015-10-16T03:24:34Z

Thanks once again, Chris. I'll add the doco commit later when I can test (Jekyll setup is problematic on this machine.)

houndci-bot · 2015-10-17T15:11:24Z

scss/_custom-forms.scss

@@ -83,6 +83,11 @@
    background-image: url(data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4NCjwhLS0gR2VuZXJhdG9yOiBBZG9iZSBJbGx1c3RyYXRvciAxNy4xLjAsIFNWRyBFeHBvcnQgUGx1Zy1JbiAuIFNWRyBWZXJzaW9uOiA2LjAwIEJ1aWxkIDApICAtLT4NCjwhRE9DVFlQRSBzdmcgUFVCTElDICItLy9XM0MvL0RURCBTVkcgMS4xLy9FTiIgImh0dHA6Ly93d3cudzMub3JnL0dyYXBoaWNzL1NWRy8xLjEvRFREL3N2ZzExLmR0ZCI+DQo8c3ZnIHZlcnNpb249IjEuMSIgaWQ9IkxheWVyXzEiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgeG1sbnM6eGxpbms9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGxpbmsiIHg9IjBweCIgeT0iMHB4Ig0KCSB3aWR0aD0iOHB4IiBoZWlnaHQ9IjhweCIgdmlld0JveD0iMCAwIDggOCIgZW5hYmxlLWJhY2tncm91bmQ9Im5ldyAwIDAgOCA4IiB4bWw6c3BhY2U9InByZXNlcnZlIj4NCjxwYXRoIGZpbGw9IiNGRkZGRkYiIGQ9Ik0wLDN2Mmg4VjNIMHoiLz4NCjwvc3ZnPg0K);
    @include box-shadow(none);
  }
+
+  input:disabled ~ .c-indicator {
+    background-color: $custom-form-bg-color-disabled;


Properties should be ordered color, background-color

houndci-bot · 2015-10-18T09:49:56Z

scss/_custom-forms.scss

+    }
+    ~ .c-indicator-label {
+      color: $custom-form-label-color-disabled;
+      cursor:not-allowed;


Colon after property should be followed by at least one space

houndci-bot · 2015-10-18T09:49:57Z

scss/_custom-forms.scss

+
+  input:disabled {
+    ~ .c-indicator {
+      background-color: $custom-form-bg-color-disabled;


Properties should be ordered cursor, background-color

houndci-bot · 2015-10-18T10:23:00Z

scss/_custom-forms.scss

+      background-color: $custom-form-bg-color-disabled;
+    }
+    ~ .c-indicator-label {
+      cursor: not-allowed;


Properties should be ordered color, cursor

markau · 2015-10-18T22:28:13Z

Hello Chris, me again. While I have tested every change prior to commit, I clearly haven't linted. Sorry for the hound spam. Lesson learned.

Can I just check whether you are likely to be happy with wrapping the checkbox/radio label in a span? This is in order to apply the cursor: none; style. I ask because it adds a relatively large chunk of characters to the html (every time an element is used) for what is a small detail (and only pertinent when the element is disabled). i.e.

before:

<label class="c-input c-checkbox">
    <input type="checkbox" checked="checked">
    <span class="c-indicator"></span>
    Can toggle this
</label>

after:

<label class="c-input c-checkbox">
    <input type="checkbox" checked="checked">
    <span class="c-indicator"></span>
    <span class="c-indicator-label">Can toggle this</span>
</label>

I tend to think the cleaner the html the better, but I see the value either way. I'm curious which way you'd want to go.

cvrebert · 2015-10-18T23:05:58Z

Can I just check whether you are likely to be happy with wrapping the checkbox/radio label in a span?

Those sorts of design specifics regarding the custom form widgets are @mdo's department rather than mine.

mdo · 2016-01-06T07:59:17Z

Changes from here are now part of #18771 (squashed and rebased there). Thanks!

markau · 2016-01-06T09:03:17Z

Thanks @mdo, it's great to have made even a small contribution. Will I be famous and show up as a contributor? (I'm a simple man 😊)

mdo · 2016-01-06T21:58:42Z

Hah, yeah, you'll be on the contributors list once the v4-dev branch makes it into master :).

Styles for [input:disabled] on checkboxes and radio elements. Issue #…

f55ac26

…17918

cvrebert added feature css v4 labels Oct 14, 2015

Styles for [input:disabled] on checkboxes and radio. Issue #17918

42efb76

Styles for [input:disabled] on checkboxes and radio. Issue #17918

64ee17c

cvrebert reviewed Oct 15, 2015
View reviewed changes

Using a variable for the disabled element color. Issue #17918

c6f276f

Color variables for both states. Improved colours. Issue #17918

fe52a43

houndci-bot reviewed Oct 17, 2015
View reviewed changes

markau added 3 commits October 18, 2015 01:45

Debugging removed. Issue #17918

1112678

Label color now reflects element state. Issue #17918

8a47dbf

Adding cursor:not-allowed to element and label. Issue #17918

cf7e5a1

houndci-bot reviewed Oct 18, 2015
View reviewed changes

Keeping the hound at bay. Fixed formatting. Issue #17918

ed1cf27

houndci-bot reviewed Oct 18, 2015
View reviewed changes

Formatting for hound. Issue #17918

748311d

Formatting - trailing whitespace. #17918

ba7b716

mdo mentioned this pull request Nov 13, 2015

added disabled state for custom form elements #17815

Closed

mdo mentioned this pull request Jan 6, 2016

v4: Custom forms (disabled state, revamp, etc) #18771

Merged

mdo closed this Jan 6, 2016

mdo added this to the v4.0.0-alpha.3 milestone Jan 6, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Styles for [input:disabled] on checkboxes and radio. Issue #17918 #17921

Styles for [input:disabled] on checkboxes and radio. Issue #17918 #17921

markau commented Oct 14, 2015

cvrebert commented Oct 14, 2015

cvrebert commented Oct 14, 2015

markau commented Oct 14, 2015

markau commented Oct 14, 2015

cvrebert commented Oct 14, 2015

markau commented Oct 14, 2015

cvrebert Oct 15, 2015

markau commented Oct 15, 2015

cvrebert commented Oct 16, 2015

markau commented Oct 16, 2015

cvrebert commented Oct 16, 2015

markau commented Oct 16, 2015

houndci-bot Oct 17, 2015

houndci-bot Oct 18, 2015

houndci-bot Oct 18, 2015

houndci-bot Oct 18, 2015

houndci-bot Oct 18, 2015

markau commented Oct 18, 2015

cvrebert commented Oct 18, 2015

mdo commented Jan 6, 2016

markau commented Jan 6, 2016

mdo commented Jan 6, 2016

Styles for [input:disabled] on checkboxes and radio. Issue #17918 #17921

Styles for [input:disabled] on checkboxes and radio. Issue #17918 #17921

Conversation

markau commented Oct 14, 2015

cvrebert commented Oct 14, 2015

cvrebert commented Oct 14, 2015

markau commented Oct 14, 2015

markau commented Oct 14, 2015

cvrebert commented Oct 14, 2015

markau commented Oct 14, 2015

cvrebert Oct 15, 2015

Choose a reason for hiding this comment

markau commented Oct 15, 2015

cvrebert commented Oct 16, 2015

markau commented Oct 16, 2015

cvrebert commented Oct 16, 2015

markau commented Oct 16, 2015

houndci-bot Oct 17, 2015

Choose a reason for hiding this comment

houndci-bot Oct 18, 2015

Choose a reason for hiding this comment

houndci-bot Oct 18, 2015

Choose a reason for hiding this comment

houndci-bot Oct 18, 2015

Choose a reason for hiding this comment

houndci-bot Oct 18, 2015

Choose a reason for hiding this comment

markau commented Oct 18, 2015

cvrebert commented Oct 18, 2015

mdo commented Jan 6, 2016

markau commented Jan 6, 2016

mdo commented Jan 6, 2016