Add the correct classes and styles for disabled buttons in Bartik

"ie8" is an actual tag to help out IE8 module :)

@oriol_e9g I was trying to test this issue, but I was unable to find any references to this class. It appears the css class should have been converted to 'image-button.is-disabled' earlier in issue #1986074: Buttons style update, but it never was.

Oh the issue for this existed already here #1282982: Clean up CSS for buttons in Bartik, but as this one is defined so nicely and has patches to review I will close the other one.

The code looks good, is there a page or use case we can use to make sure this works?

@LewisNyman The only 'image-button' currently in core is the cogwheel on the Field UI. I am not aware of any way to make this button disabled.

@oriol_e9g The classes apply correctly, but the styling suggests there is still an action available. See this screen capture for illustration:

Can you reset the cursor and background-color to its default for disabled buttons? For an example see seven/components/buttons.theme.css:

.button:disabled,
.button:disabled:active,
.button.is-disabled,
.button.is-disabled:active {
  border-color: #d4d4d4;
  background: #ededed;
  box-shadow: none;
  color: #5c5c5c;
  font-weight: normal;
  cursor: default;
  text-shadow: 0 1px hsla(0, 0%, 100%, 0.6);
}

Obligatory screenshots:

Buttons before:

Buttons after:

Ah yes, that will do. These styles should be located in buttons.css, not in form.css

I will try to resolve this issue by checking what is the current state of this issue in current repository of Drupal 8.0.x for the Drupal global Sprint Weekend 17.01.2015.

First, by searching though whole Drupal 8.x project, it seems that initial post still holds,

1. The only place in whole Drupal project where is mentioned class form-button-disabled is in Bartik, so it is not referenced anyhere (except if, maybe somehow constructed in code somewhere) — therefore, I can pretty safely assume that CSS may be changed to accommodate new class is-disabled
2. Although semantically, is-disabled does look more like active test than as state, there is disabled class in theme seven but not anywhere else and it is used in parallel with is-disabled in theme seven just to be safe — this could maybe be adopted in
3. The class is-disabled appears in following files so it would not be much of a hassle to standardize on disabled which makes more sense to me (but that is the case for reopening #1986074: Buttons style update:
   1. core/lib/Drupal/Core/Render/Element/Button.php
   2. core/lib/Drupal/Core/Render/Element/ImageButton.php
   3. core/modules/system/src/Tests/Form/FormTest.php

I've checked things and done what LewisNyman suggested, moved buttons from form.css to buttons.css so it seems to me it can be tested and committed.

1. +++ b/core/themes/bartik/css/components/buttons.css
   @@ -23,3 +23,20 @@
   +.button.is-disabled,
   +.button.is-disabled:hover,
   +.button.is-disabled:active,
   +.button.is-disabled:focus
   + {
   

   The curly bracket can go on the line with the last selector: .button.is-disabled: focus {

2. +++ b/core/themes/bartik/css/components/buttons.css
   @@ -23,3 +23,20 @@
   +.image-button.is-disabled,
   +.image-button.is-disabled:hover,
   +.image-button.is-disabled:active,
   +.image-button.is-disabled:focus {
   +  opacity: .5;
   +  cursor: default;
   +}
   

   The last 3 selectors are superfluous as they don't override any existing properties

3. Could you also reset the background on disabled image buttons as I mentioned in #8?
   

You are right, also in form.css there was curly bracket left on next line, both corrected, added background (if I understood well?)

1. +++ b/core/themes/bartik/css/components/buttons.css
   @@ -23,3 +23,20 @@
   +.image-button.is-disabled:hover,
   +.image-button.is-disabled:active,
   +.image-button.is-disabled:focus {
   

   These selectors can still be removed :)

2. +++ b/core/themes/bartik/css/components/buttons.css
   @@ -23,3 +23,20 @@
   +  background: #ededed;
   

   Sorry, I was being unclear. By background reset I mean the item should have no background, eg. background: none.

3. +++ b/core/themes/bartik/css/components/form.css
   @@ -138,23 +138,11 @@ input.form-submit:focus {
   +  cursor: default;
   

   The cursor property is no longer necessary, since the remaining selectors have no css property for cursor.

I'm a bit harder to follow but I'll assume I got it right this time :)

1. +++ b/core/themes/bartik/css/components/buttons.css
   @@ -21,5 +21,19 @@
    .button:focus {
      text-decoration: none;
      color: #5a5a5a;
   -  background: #dedede;
   +  background: none;
   

   We are affecting all buttons here, I don't think this is an intentional change

2. +++ b/core/themes/bartik/css/components/buttons.css
   @@ -21,5 +21,19 @@
   +.button.is-disabled,
   +.button.is-disabled:hover,
   +.button.is-disabled:active,
   +.button.is-disabled:focus {
   

   Idebr mentioned before that these could be removed

3. +++ b/core/themes/bartik/css/components/buttons.css
   @@ -21,5 +21,19 @@
   +  opacity: .5;
   

   I think we always add a 0 to the beginning of these decimal points

With #3 I agree completely, I've missed that out.

As for #2, Idebr didn't say about that specifically but seems logical to remove if the same for image button was removed.

But for #1 I am not so sure. It is affecting but whole selector is affecting and it does make sense as it signifies the change of the state so it should be left there. IF you look at the previous selector in the file .buton it is very logical that next one (the one you argue in #1) is dealing with states.

Can you be more specific with some example why this shouldn't be?

+++ b/core/themes/bartik/css/components/buttons.css
@@ -21,5 +21,16 @@
-  background: #dedede;
+  background: none;
+}
...
+.image-button.is-disabled {
+  background: #ededed;

It appears I was very unclear, terribly sorry about that :(

The background: none should applied to image-button.is-disabled to prevent the background-color from appearing on disabled image-buttons (see the behavior in #8 for reference)

OK, dear friends I hope this one will fix everything and satisfy everyone :)

First, background: none doesn't clear the assigned color so we need transparent to override assigned colors.

On the other hand I haven't seen how could I make cog or button appear as disabled so I fabricated in browser to simulate and check behavior.

I hope I understood what you all said and made the right patch. This should be fixed and we should move on to more important tasks so let us all make an effort and conclude this ASAP so please be constructive as much as possible with comments and lets close this once,

I tested the current patch and noticed the following things.

1. Looking at disabled image-buttons they work great now...
   

 

2. We now need to also add back the disabled hover styles for regular disabled buttons, which were lost when the form-button-disabled selector's styles. Currently disabled buttons with the class .is-disabled have a hover state. See below...
   
    
   

 

3. Also one code error in the patch...

   +++ b/core/themes/bartik/css/components/buttons.css
   @@ -21,5 +21,21 @@
   +.image-button.is-disabled:hover,
   +.image-button.is-disabled:active,
   +.image-button.is-disabled:focus {
   +  background: transparent;;
   

   There is a double ;; after transparent.

I've rechecked and bit optimized CSS here with disabled buttons.

Please: how do you make those animated screen capture?

+++ b/core/themes/bartik/css/components/buttons.css
@@ -19,7 +19,24 @@
 .button:hover,
 .button:active,
 .button:focus {
-  text-decoration: none;
+  background: transparent;
   color: #5a5a5a;
-  background: #dedede;
+  text-decoration: none;
+}

Why did you change these styles? Can you put this back as normal 'a' element links that have the class button need these styles on hover etc.
See this example on the admin/content page....

 
Otherwise the disabled button styles now work great, see latest gif below...
 

Fixed background color for hover, active and focus states for buttons, as said on comment #30.

Testing "+Add content button

1) I visited /admin/content to test the new CSS class.
2) I simulated the disabled state of the button adding the class using Chrome Dev Tools. Please, see next code:

<a href="/node/add" class="button button-action is-disabled" data-drupal-link-system-path="node/add">Add content</a>

3) Following images are tests I did.

Add content enabled

Add content enabled / hover

Add content disabled

Add content disabled / hover

Testing "Save and Publish" button

1) I went to this path to review the new "is-disabled" class: /node/add/article
2) In the Chrome Dev Tools I added the CSS class to simulate the expected behaviour. Please, see following line:

<input type="submit" name="op" value="Save and publish" class="button form-submit is-disabled">

3) Following images show the manual tests I did.

Save and publish enabled

Save and publish hover

looks like need the disabled screenshot also (for the preview example)

Beautiful! +1

Here is an interdiff between the patches on #29 and #32.

Testing "+Add content" button

1) I visited /admin/content to test the new CSS class.
2) I simulated the disabled state of the button adding the class using Chrome Dev Tools. Please, see next code:

<a href="/node/add" class="button button-action is-disabled" data-drupal-link-system-path="node/add">Add content</a>

3) Following images are tests I did.

Add content enabled

Add content enabled / hover

Add content disabled

Add content disabled / hover

Testing "Save and Publish" button

1) I went to this path to review the new "is-disabled" class: /node/add/article
2) In the Chrome Dev Tools I added the CSS class to simulate the expected behaviour. Please, see following line:

<input type="submit" name="op" value="Save and publish" class="button form-submit is-disabled">

3) Following images show the manual tests I did.

Save and publish enabled

Save and publish enabled / hover

Save and publish disabled

Save and publish disabled / hover

Testing "Preview" button

1) I went to this path to review the new "is-disabled" class: /node/add/article
2) In the Chrome Dev Tools I added the CSS class to simulate the expected behaviour. Please, see following line:

<input type="submit" id="edit-preview" name="op" value="Preview" class="button form-submit is-disabled">

3) Following images show the manual tests I did.

Preview enabled

Preview enabled / hover

Preview disabled

Preview disabled / hover

I'm not clear on why this is a bug.
If it is a task, a beta evaluation would be super.
https://www.drupal.org/core/beta-changes

I have looked at the screenshots in #27 and #39 and compared them with head. None of the elements affected by this work are visually different between head and the patch in #32. The code for this work is all CSS code which has been placed in the correct files and meet coding standards. Buttons are functioning correctly when disabled. Plus we checked other states that may have been affected by this work, this is shown in the screenshots in #39. RTBC++

Thank you! Committed at the code sprint in Bogota.