From ea7c4a93cfdff5b324dfe796ff6c7cf983052b35 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Pekka=20Hyv=C3=B6nen?= Date: Wed, 22 Nov 2017 12:36:07 +0200 Subject: [PATCH] Fix missing v-disabled for RadioButtonGroup and CheckBoxGroup options (#10332) * Fix missing v-disabled for RadioButtonGroup * Fix missing v-disabled for CheckBoxGroup Resolves #9258 --- .../com/vaadin/client/ui/VCheckBoxGroup.java | 4 + .../vaadin/client/ui/VRadioButtonGroup.java | 12 ++- .../checkbox/CheckBoxGroupTestUI.java | 16 +++ .../radiobutton/RadioButtonGroupTestUI.java | 15 +++ .../checkboxgroup/CheckBoxGroupTest.java | 97 +++++++++++++++++++ .../RadioButtonGroupTest.java | 89 +++++++++++++++++ 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> 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 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> 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 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 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 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 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")); + } } -- 2.39.5