fix: allow changing checkboxgroup value if disabled checkbox value do…

…esn't change (#1903) (#1968) Co-authored-by: Sascha Ißbrücker <sascha.issbruecker@googlemail.com> Co-authored-by: Guille <alvarezguille@users.noreply.github.com> Co-authored-by: David Sosa <76832183+sosa-vaadin@users.noreply.github.com> Co-authored-by: Sascha Ißbrücker <sascha.issbruecker@googlemail.com> Co-authored-by: Guille <alvarezguille@users.noreply.github.com>
vaadin · Aug 3, 2021 · f136532 · f136532
1 parent 8c6e97f
commit f136532
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 7 deletions.
diff --git a/...src/main/java/com/vaadin/flow/component/checkbox/tests/CheckboxGroupDisabledItemPage.java b/...src/main/java/com/vaadin/flow/component/checkbox/tests/CheckboxGroupDisabledItemPage.java
@@ -1,6 +1,8 @@
 package com.vaadin.flow.component.checkbox.tests;
 
 import com.vaadin.flow.component.checkbox.CheckboxGroup;
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.component.html.NativeButton;
 import com.vaadin.flow.component.orderedlayout.VerticalLayout;
 import com.vaadin.flow.router.Route;
 
@@ -18,6 +20,18 @@ public CheckboxGroupDisabledItemPage() {
         group.select("bar");
         group.setItemEnabledProvider(item -> !"bar".equals(item));
         group.setId("checkbox-group-disabled-item");
-        add(group);
+
+        NativeButton toggleBarButton = new NativeButton("Toggle \"bar\"",
+                event -> {
+                    boolean isBarSelected = group.isSelected("bar");
+                    if (isBarSelected) {
+                        group.deselect("bar");
+                    } else {
+                        group.select("bar");
+                    }
+                });
+        toggleBarButton.setId("toggle-bar-button");
+
+        add(group, new Div(toggleBarButton));
     }
 }
diff --git a/...s/src/test/java/com/vaadin/flow/component/checkbox/tests/CheckboxGroupDisabledItemIT.java b/...s/src/test/java/com/vaadin/flow/component/checkbox/tests/CheckboxGroupDisabledItemIT.java
@@ -37,4 +37,45 @@ public void disabledGroupItemChecked() {
         Assert.assertEquals(Boolean.TRUE.toString(),
                 checkboxes.get(1).getAttribute("checked"));
     }
+
+    @Test
+    public void disabledItemCanBeCheckedProgrammatically() {
+        open();
+        TestBenchElement group = $(TestBenchElement.class)
+                .id("checkbox-group-disabled-item");
+        List<TestBenchElement> checkboxes = group.$("vaadin-checkbox").all();
+        TestBenchElement secondCheckbox = checkboxes.get(1);
+        TestBenchElement toggleBarButton = $("button").id("toggle-bar-button");
+
+        // Deselect
+        toggleBarButton.click();
+        Assert.assertNull(secondCheckbox.getAttribute("checked"));
+
+        // Reselect
+        toggleBarButton.click();
+        Assert.assertEquals(Boolean.TRUE.toString(),
+                secondCheckbox.getAttribute("checked"));
+    }
+
+    /**
+     * Regression test for:
+     * https://github.com/vaadin/flow-components/issues/1185
+     */
+    @Test
+    public void enabledItemCanBeCheckedManuallyWhenSettingItemEnabledProviderAfterSelectingValue() {
+        open();
+        TestBenchElement group = $(TestBenchElement.class)
+                .id("checkbox-group-disabled-item");
+        List<TestBenchElement> checkboxes = group.$("vaadin-checkbox").all();
+        TestBenchElement firstCheckbox = checkboxes.get(0);
+
+        // Select
+        firstCheckbox.click();
+        Assert.assertEquals(Boolean.TRUE.toString(),
+                firstCheckbox.getAttribute("checked"));
+
+        // Deselect
+        firstCheckbox.click();
+        Assert.assertNull(firstCheckbox.getAttribute("checked"));
+    }
 }
diff --git a/.../vaadin-checkbox-flow/src/main/java/com/vaadin/flow/component/checkbox/CheckboxGroup.java b/.../vaadin-checkbox-flow/src/main/java/com/vaadin/flow/component/checkbox/CheckboxGroup.java
@@ -442,12 +442,16 @@ protected boolean valueEquals(Set<T> value1, Set<T> value2) {
 
     @Override
     protected boolean hasValidValue() {
-        Set<T> selectedItems = presentationToModel(this,
+        // we need to compare old value with new value to see if any disabled
+        // items changed their value
+        Set<T> value = presentationToModel(this,
                 (JsonArray) getElement().getPropertyRaw(VALUE));
-        if (selectedItems == null || selectedItems.isEmpty()) {
-            return true;
-        }
-        return selectedItems.stream().allMatch(itemEnabledProvider);
+        Set<T> oldValue = getValue();
+
+        // disabled items cannot change their value
+        return getCheckboxItems().filter(CheckBoxItem::isDisabledBoolean)
+                .noneMatch(item -> oldValue.contains(item.getItem()) != value
+                        .contains(item.getItem()));
     }
 
     @SuppressWarnings("unchecked")
@@ -540,7 +544,7 @@ private void validateSelectionEnabledState(PropertyChangeEvent event) {
             } finally {
                 registerValidation();
             }
-            // Now make sure that the button is still in the correct state
+            // Now make sure that the checkbox is still in the correct state
             Set<T> value = presentationToModel(this,
                     (JsonArray) event.getValue());
             getCheckboxItems()