Fix missing v-disabled for RadioButtonGroup and CheckBoxGroup options (#10332)

* Fix missing v-disabled for RadioButtonGroup
* Fix missing v-disabled for CheckBoxGroup

Resolves #9258

6 files changed, 231 insertions, 2 deletions

diff --git a/client/src/main/java/com/vaadin/client/ui/VCheckBoxGroup.java b/client/src/main/java/com/vaadin/client/ui/VCheckBoxGroup.java
index d393b1c3ce..b1132a487c 100644
--- a/client/src/main/java/com/vaadin/client/ui/VCheckBoxGroup.java
+++ b/client/src/main/java/com/vaadin/client/ui/VCheckBoxGroup.java
@@ -31,6 +31,7 @@ import com.google.gwt.user.client.ui.HasEnabled;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.ApplicationConnection;
 import com.vaadin.client.BrowserInfo;
+import com.vaadin.client.StyleConstants;
 import com.vaadin.client.WidgetUtil;
 import com.vaadin.client.widgets.FocusableFlowPanelComposite;
 import com.vaadin.shared.Registration;
@@ -188,6 +189,9 @@ public class VCheckBoxGroup extends FocusableFlowPanelComposite
                 .getBoolean(ListingJsonConstants.JSONKEY_ITEM_DISABLED);
         boolean enabled = optionEnabled && !isReadonly() && isEnabled();
         checkBox.setEnabled(enabled);
+        // #9258 apply the v-disabled class when disabled for UX
+        checkBox.setStyleName(StyleConstants.DISABLED,
+                !isEnabled() || !optionEnabled);
     }
 
     public boolean isHtmlContentAllowed() {
diff --git a/client/src/main/java/com/vaadin/client/ui/VRadioButtonGroup.java b/client/src/main/java/com/vaadin/client/ui/VRadioButtonGroup.java
index a382758409..89f8eb16e3 100644
--- a/client/src/main/java/com/vaadin/client/ui/VRadioButtonGroup.java
+++ b/client/src/main/java/com/vaadin/client/ui/VRadioButtonGroup.java
@@ -34,6 +34,7 @@ import com.google.gwt.user.client.ui.RadioButton;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.ApplicationConnection;
 import com.vaadin.client.BrowserInfo;
+import com.vaadin.client.StyleConstants;
 import com.vaadin.client.WidgetUtil;
 import com.vaadin.client.widgets.FocusableFlowPanelComposite;
 import com.vaadin.shared.Registration;
@@ -155,6 +156,10 @@ public class VRadioButtonGroup extends FocusableFlowPanelComposite
                 .getBoolean(ListingJsonConstants.JSONKEY_ITEM_DISABLED);
         boolean enabled = optionEnabled && !isReadonly() && isEnabled();
         button.setEnabled(enabled);
+        // #9258 apply the v-disabled class when disabled for UX
+        button.setStyleName(StyleConstants.DISABLED,
+                !isEnabled() || !optionEnabled);
+
         String key = item.getString(DataCommunicatorConstants.KEY);
 
         if (requireInitialization) {
@@ -205,9 +210,12 @@ public class VRadioButtonGroup extends FocusableFlowPanelComposite
                 .entrySet()) {
             RadioButton radioButton = entry.getKey();
             JsonObject value = entry.getValue();
-            Boolean isOptionEnabled = !value
+            boolean optionEnabled = !value
                     .getBoolean(ListingJsonConstants.JSONKEY_ITEM_DISABLED);
-            radioButton.setEnabled(radioButtonEnabled && isOptionEnabled);
+            radioButton.setEnabled(radioButtonEnabled && optionEnabled);
+            // #9258 apply the v-disabled class when disabled for UX
+            radioButton.setStyleName(StyleConstants.DISABLED,
+                    !isEnabled() || !optionEnabled);
         }
     }
 
diff --git a/uitest/src/main/java/com/vaadin/tests/components/checkbox/CheckBoxGroupTestUI.java b/uitest/src/main/java/com/vaadin/tests/components/checkbox/CheckBoxGroupTestUI.java
index e7ee89f662..c3533f2506 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/checkbox/CheckBoxGroupTestUI.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/checkbox/CheckBoxGroupTestUI.java
@@ -16,8 +16,10 @@
 package com.vaadin.tests.components.checkbox;
 
 import java.util.LinkedHashMap;
+import java.util.Objects;
 
 import com.vaadin.icons.VaadinIcons;
+import com.vaadin.server.SerializablePredicate;
 import com.vaadin.tests.components.abstractlisting.AbstractMultiSelectTestUI;
 import com.vaadin.ui.CheckBoxGroup;
 import com.vaadin.ui.DescriptionGenerator;
@@ -53,6 +55,7 @@ public class CheckBoxGroupTestUI
         super.createActions();
         createItemIconGenerator();
         createItemDescriptionGeneratorMenu();
+        createItemEnabledProviderMenu();
     }
 
     private void createItemIconGenerator() {
@@ -75,6 +78,19 @@ public class CheckBoxGroupTestUI
                 }, true);
     }
 
+    private void createItemEnabledProviderMenu() {
+        LinkedHashMap<String, SerializablePredicate<Object>> options = new LinkedHashMap<>();
+        options.put("Disable Item 0", o -> !Objects.equals(o, "Item 0"));
+        options.put("Disable Item 3", o -> !Objects.equals(o, "Item 3"));
+        options.put("Disable Item 5", o -> !Objects.equals(o, "Item 5"));
+
+        createSelectAction("Item Enabled Provider", "Item Enabled Provider",
+                options, "None", (checkBoxGroup, generator, data) -> {
+                    checkBoxGroup.setItemEnabledProvider(generator);
+                    checkBoxGroup.getDataProvider().refreshAll();
+                }, true);
+    }
+
     private void useItemIconProvider(CheckBoxGroup<Object> group,
             boolean activate, Object data) {
         if (activate) {
diff --git a/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java b/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java
index 87f265cf99..585c734c7d 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/radiobutton/RadioButtonGroupTestUI.java
@@ -16,9 +16,11 @@
 package com.vaadin.tests.components.radiobutton;
 
 import java.util.LinkedHashMap;
+import java.util.Objects;
 import java.util.stream.IntStream;
 
 import com.vaadin.icons.VaadinIcons;
+import com.vaadin.server.SerializablePredicate;
 import com.vaadin.tests.components.abstractlisting.AbstractListingTestUI;
 import com.vaadin.ui.DescriptionGenerator;
 import com.vaadin.ui.ItemCaptionGenerator;
@@ -48,6 +50,7 @@ public class RadioButtonGroupTestUI
         createItemIconGeneratorMenu();
         createItemCaptionGeneratorMenu();
         createItemDescriptionGeneratorMenu();
+        createItemEnabledProviderMenu();
     }
 
     protected void createSelectionMenu() {
@@ -104,6 +107,18 @@ public class RadioButtonGroupTestUI
                 "Item Description Generator", options, "None",
                 (radioButtonGroup, generator, data) -> {
                     radioButtonGroup.setItemDescriptionGenerator(generator);
+                }, true);
+    }
+
+    private void createItemEnabledProviderMenu() {
+        LinkedHashMap<String, SerializablePredicate<Object>> options = new LinkedHashMap<>();
+        options.put("Disable Item 0", o -> !Objects.equals(o, "Item 0"));
+        options.put("Disable Item 3", o -> !Objects.equals(o, "Item 3"));
+        options.put("Disable Item 5", o -> !Objects.equals(o, "Item 5"));
+
+        createSelectAction("Item Enabled Provider", "Item Enabled Provider",
+                options, "None", (radioButtonGroup, generator, data) -> {
+                    radioButtonGroup.setItemEnabledProvider(generator);
                     radioButtonGroup.getDataProvider().refreshAll();
                 }, true);
     }
diff --git a/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java b/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java
index 294026f06e..66589956b9 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/checkboxgroup/CheckBoxGroupTest.java
@@ -27,6 +27,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.stream.Collectors;
 
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.openqa.selenium.By;
@@ -291,9 +292,105 @@ public class CheckBoxGroupTest extends MultiBrowserTest {
         }
     }
 
+    @Test // #9258
+    public void disabled_correctClassNamesApplied() {
+        openTestURL("theme=valo");
+        selectMenuPath("Component", "State", "Enabled");
+
+        List<WebElement> options = getSelect().findElements(By.tagName("span"));
+        Assert.assertTrue(options.size() > 0);
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(className -> verifyCheckboxDisabledClassNames(
+                        className, true));
+
+        selectMenuPath("Component", "State", "Enabled");
+
+        options = getSelect().findElements(By.tagName("span"));
+        Assert.assertTrue(options.size() > 0);
+        verifyCheckboxDisabledClassNames(
+                options.remove(10).getAttribute("className"), true);
+
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(className -> verifyCheckboxDisabledClassNames(
+                        className, false));
+    }
+
+    @Test // #9258
+    public void itemDisabledWithEnabledProvider_correctClassNamesApplied() {
+        openTestURL("theme=valo");
+
+        List<WebElement> options = getSelect().findElements(By.tagName("span"));
+
+        Assert.assertTrue(options.size() > 0);
+        verifyCheckboxDisabledClassNames(
+                options.remove(10).getAttribute("className"), true);
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(cs -> verifyCheckboxDisabledClassNames(cs, false));
+
+        selectMenuPath("Component", "Item Enabled Provider",
+                "Item Enabled Provider", "Disable Item 0");
+
+        options = getSelect().findElements(By.tagName("span"));
+        String className = options.get(0).getAttribute("className");
+        verifyCheckboxDisabledClassNames(className, true);
+        verifyCheckboxDisabledClassNames(
+                options.remove(10).getAttribute("className"), false);
+
+        selectMenuPath("Component", "Item Enabled Provider",
+                "Item Enabled Provider", "Disable Item 3");
+
+        className = getSelect().findElements(By.tagName("span")).get(0)
+                .getAttribute("className");
+        verifyCheckboxDisabledClassNames(className, false);
+
+        className = getSelect().findElements(By.tagName("span")).get(3)
+                .getAttribute("className");
+        verifyCheckboxDisabledClassNames(className, true);
+
+        selectMenuPath("Component", "State", "Enabled");
+
+        options = getSelect().findElements(By.tagName("span"));
+
+        Assert.assertTrue(options.size() > 0);
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(cs -> verifyCheckboxDisabledClassNames(cs, true));
+
+        selectMenuPath("Component", "Item Enabled Provider",
+                "Item Enabled Provider", "Disable Item 5");
+
+        options = getSelect().findElements(By.tagName("span"));
+
+        Assert.assertTrue(options.size() > 0);
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(cs -> verifyCheckboxDisabledClassNames(cs, true));
+
+        selectMenuPath("Component", "State", "Enabled");
+
+        options = getSelect().findElements(By.tagName("span"));
+        className = options.remove(5).getAttribute("className");
+
+        Assert.assertTrue(options.size() > 0);
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(cs -> verifyCheckboxDisabledClassNames(cs, false));
+        verifyCheckboxDisabledClassNames(className, true);
+    }
+
     // needed to make tooltips work in IE tests
     @Override
     protected boolean requireWindowFocusForIE() {
         return true;
     }
+
+    private static void verifyCheckboxDisabledClassNames(String className,
+            boolean disabled) {
+        Assert.assertEquals(
+                disabled ? "No"
+                        : "Extra" + " v-checkbox-disabled class, was "
+                                + className,
+                disabled, className.contains("v-checkbox-disabled"));
+        Assert.assertEquals(
+                disabled ? "No"
+                        : "Extra" + " v-disabled class, was " + className,
+                disabled, className.contains("v-disabled"));
+    }
 }
diff --git a/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupTest.java b/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupTest.java
index bb1681d5f9..0fa7c3f265 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupTest.java
@@ -119,6 +119,82 @@ public class RadioButtonGroupTest extends MultiBrowserTest {
         assertEquals(lastLogRow, getLogRow(0));
     }
 
+    @Test // #9258
+    public void disabled_correctClassNamesApplied() {
+        openTestURL("theme=valo");
+        selectMenuPath("Component", "State", "Enabled");
+
+        List<WebElement> options = getSelect().findElements(By.tagName("span"));
+        Assert.assertTrue(options.size() > 0);
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(className -> verifyRadioButtonDisabledClassNames(
+                        className, true));
+
+        selectMenuPath("Component", "State", "Enabled");
+
+        options = getSelect().findElements(By.tagName("span"));
+        Assert.assertTrue(options.size() > 0);
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(className -> verifyRadioButtonDisabledClassNames(
+                        className, false));
+    }
+
+    @Test // #9258
+    public void itemDisabledWithEnabledProvider_correctClassNamesApplied() {
+        openTestURL("theme=valo");
+
+        List<WebElement> options = getSelect().findElements(By.tagName("span"));
+
+        Assert.assertTrue(options.size() > 0);
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(cs -> verifyRadioButtonDisabledClassNames(cs, false));
+
+        selectMenuPath("Component", "Item Enabled Provider",
+                "Item Enabled Provider", "Disable Item 0");
+
+        String className = getSelect().findElements(By.tagName("span")).get(0)
+                .getAttribute("className");
+        verifyRadioButtonDisabledClassNames(className, true);
+
+        selectMenuPath("Component", "Item Enabled Provider",
+                "Item Enabled Provider", "Disable Item 3");
+
+        className = getSelect().findElements(By.tagName("span")).get(0)
+                .getAttribute("className");
+        verifyRadioButtonDisabledClassNames(className, false);
+
+        className = getSelect().findElements(By.tagName("span")).get(3)
+                .getAttribute("className");
+        verifyRadioButtonDisabledClassNames(className, true);
+
+        selectMenuPath("Component", "State", "Enabled");
+
+        options = getSelect().findElements(By.tagName("span"));
+
+        Assert.assertTrue(options.size() > 0);
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(cs -> verifyRadioButtonDisabledClassNames(cs, true));
+
+        selectMenuPath("Component", "Item Enabled Provider",
+                "Item Enabled Provider", "Disable Item 5");
+
+        options = getSelect().findElements(By.tagName("span"));
+
+        Assert.assertTrue(options.size() > 0);
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(cs -> verifyRadioButtonDisabledClassNames(cs, true));
+
+        selectMenuPath("Component", "State", "Enabled");
+
+        options = getSelect().findElements(By.tagName("span"));
+        className = options.remove(5).getAttribute("className");
+
+        Assert.assertTrue(options.size() > 0);
+        options.stream().map(element -> element.getAttribute("className"))
+                .forEach(cs -> verifyRadioButtonDisabledClassNames(cs, false));
+        verifyRadioButtonDisabledClassNames(className, true);
+    }
+
     @Test
     public void itemIconGenerator() {
         selectMenuPath("Component", "Item Icon Generator",
@@ -251,4 +327,17 @@ public class RadioButtonGroupTest extends MultiBrowserTest {
     protected boolean requireWindowFocusForIE() {
         return true;
     }
+
+    private static void verifyRadioButtonDisabledClassNames(String className,
+            boolean disabled) {
+        Assert.assertEquals(
+                disabled ? "No"
+                        : "Extra" + " v-radiobutton-disabled class, was "
+                                + className,
+                disabled, className.contains("v-radiobutton-disabled"));
+        Assert.assertEquals(
+                disabled ? "No"
+                        : "Extra" + " v-disabled class, was " + className,
+                disabled, className.contains("v-disabled"));
+    }
 }