From 69923d1b9d2087a898975ba1d982524ed11c413a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Tue, 18 May 2021 17:28:38 +0200 Subject: [PATCH] CP: disable client side validation (#973) (#986) Fixes: vaadin/vaadin-select#265 Warranty: Fixes client-side select overriding validation state from server Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com> Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com> (cherry picked from commit 14676def81188ec9a674bfe202ebac8d71b65067) --- .../pom.xml | 11 ++ .../OverrideClientValidationPage.java | 92 ++++++++++++ .../select/test/BasicFeaturesIT.java | 5 + .../test/OverrideClientValidationIT.java | 138 ++++++++++++++++++ .../component/select/FieldValidationUtil.java | 69 +++++++++ .../vaadin/flow/component/select/Select.java | 21 ++- .../generated/GeneratedVaadinSelect.java | 5 +- 7 files changed, 330 insertions(+), 11 deletions(-) create mode 100644 vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/main/java/com/vaadin/flow/component/select/examples/OverrideClientValidationPage.java create mode 100644 vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/test/java/com/vaadin/flow/component/select/test/OverrideClientValidationIT.java create mode 100644 vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/FieldValidationUtil.java diff --git a/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/pom.xml b/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/pom.xml index c8afdde65c9..f316a7e0789 100644 --- a/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/pom.xml +++ b/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/pom.xml @@ -83,6 +83,17 @@ com.vaadin flow-polymer-template + + com.vaadin + vaadin-grid-flow + ${project.version} + + + com.vaadin + vaadin-grid-testbench + ${project.version} + test + diff --git a/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/main/java/com/vaadin/flow/component/select/examples/OverrideClientValidationPage.java b/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/main/java/com/vaadin/flow/component/select/examples/OverrideClientValidationPage.java new file mode 100644 index 00000000000..6fb535197bc --- /dev/null +++ b/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/main/java/com/vaadin/flow/component/select/examples/OverrideClientValidationPage.java @@ -0,0 +1,92 @@ +/* + * Copyright 2000-2021 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + * + */ + +package com.vaadin.flow.component.select.examples; + +import com.vaadin.flow.component.grid.Grid; +import com.vaadin.flow.component.html.Div; +import com.vaadin.flow.component.html.H1; +import com.vaadin.flow.component.html.NativeButton; +import com.vaadin.flow.component.html.Span; +import com.vaadin.flow.component.select.Select; +import com.vaadin.flow.data.renderer.ComponentRenderer; +import com.vaadin.flow.router.Route; + +@Route("vaadin-select/override-client-validation") +public class OverrideClientValidationPage extends Div { + + public static final String ID_BASIC_SELECT = "basic-select"; + public static final String ID_BASIC_SELECT_RESULT_SPAN = "basic-select-result-span"; + public static final String ID_SET_INVALID_BUTTON = "set-invalid-button"; + public static final String ID_LOG_BUTTON = "log-button"; + public static final String ID_DETACH_BUTTON = "detach-button"; + public static final String ID_REATTACH_BUTTON = "reattach-button"; + + private Select basicSelect; + private Span basicSelectResultSpan; + private Select selectInGrid; + + public OverrideClientValidationPage() { + createBasicSetup(); + createGridSetup(); + createActions(); + } + + private void createBasicSetup() { + basicSelect = new Select<>("a", "b", "c"); + basicSelect.setId(ID_BASIC_SELECT); + + basicSelectResultSpan = new Span(); + basicSelectResultSpan.setId(ID_BASIC_SELECT_RESULT_SPAN); + + add(new H1("Basic select usage"), basicSelect, basicSelectResultSpan); + } + + private void createGridSetup() { + selectInGrid = new Select<>(); + Grid grid = new Grid<>(); + grid.setItems("test"); + grid.addColumn(new ComponentRenderer<>(item -> selectInGrid, + (component, item) -> component)); + add(new H1("Grid select usage"), grid); + } + + private void createActions() { + NativeButton setInvalidButton = new NativeButton("Set all invalid", + e -> { + basicSelect.setInvalid(true); + selectInGrid.setInvalid(true); + }); + setInvalidButton.setId(ID_SET_INVALID_BUTTON); + + NativeButton logButton = new NativeButton("Log validation state", + e -> basicSelectResultSpan.setText( + basicSelect.isInvalid() ? "invalid" : "valid")); + logButton.setId(ID_LOG_BUTTON); + + NativeButton detachButton = new NativeButton("Detach select", + e -> this.remove(basicSelect)); + detachButton.setId(ID_DETACH_BUTTON); + + NativeButton reattachButton = new NativeButton("Reattach select", + e -> this.addComponentAsFirst(basicSelect)); + reattachButton.setId(ID_REATTACH_BUTTON); + + addComponentAsFirst(new Div(setInvalidButton, logButton, detachButton, + reattachButton)); + } +} diff --git a/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/test/java/com/vaadin/flow/component/select/test/BasicFeaturesIT.java b/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/test/java/com/vaadin/flow/component/select/test/BasicFeaturesIT.java index 62dffcdd2ab..70a5c846674 100644 --- a/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/test/java/com/vaadin/flow/component/select/test/BasicFeaturesIT.java +++ b/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/test/java/com/vaadin/flow/component/select/test/BasicFeaturesIT.java @@ -8,6 +8,11 @@ @TestPath("vaadin-select/") public class BasicFeaturesIT extends AbstractSelectIT { + @Test + public void test_initialClientValue() { + Assert.assertEquals("", selectElement.getProperty("value")); + } + @Test public void testEnabled_disabling_userCannotSelect() { page.toggleEnabled(false); diff --git a/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/test/java/com/vaadin/flow/component/select/test/OverrideClientValidationIT.java b/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/test/java/com/vaadin/flow/component/select/test/OverrideClientValidationIT.java new file mode 100644 index 00000000000..e7c4a09606d --- /dev/null +++ b/vaadin-select-flow-parent/vaadin-select-flow-integration-tests/src/test/java/com/vaadin/flow/component/select/test/OverrideClientValidationIT.java @@ -0,0 +1,138 @@ +/* + * Copyright 2000-2021 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + * + */ + +package com.vaadin.flow.component.select.test; + +import com.vaadin.flow.component.grid.testbench.GridElement; +import com.vaadin.flow.component.select.examples.OverrideClientValidationPage; +import com.vaadin.flow.component.select.testbench.SelectElement; +import com.vaadin.flow.testutil.TestPath; +import com.vaadin.testbench.TestBenchElement; +import com.vaadin.tests.AbstractComponentIT; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +@TestPath("vaadin-select/override-client-validation") +public class OverrideClientValidationIT extends AbstractComponentIT { + + private SelectElement basicSelectElement; + private TestBenchElement setInvalidButton; + private TestBenchElement logButton; + private TestBenchElement detachButton; + private TestBenchElement reattachButton; + private TestBenchElement basicSelectResultSpan; + private SelectElement selectInGridElement; + + @Before + public void setUp() { + open(); + basicSelectElement = $(SelectElement.class) + .id(OverrideClientValidationPage.ID_BASIC_SELECT); + setInvalidButton = $("button") + .id(OverrideClientValidationPage.ID_SET_INVALID_BUTTON); + logButton = $("button").id(OverrideClientValidationPage.ID_LOG_BUTTON); + detachButton = $("button") + .id(OverrideClientValidationPage.ID_DETACH_BUTTON); + reattachButton = $("button") + .id(OverrideClientValidationPage.ID_REATTACH_BUTTON); + basicSelectResultSpan = $("span") + .id(OverrideClientValidationPage.ID_BASIC_SELECT_RESULT_SPAN); + GridElement gridElement = $(GridElement.class).first(); + selectInGridElement = gridElement.$(SelectElement.class).first(); + } + + @Test + public void testTriggeringClientValidationShouldNotOverrideClientValidationState() { + // Set server state to invalid + setInvalidButton.click(); + assertClientSideSelectValidationState(basicSelectElement, false); + + // Trigger client side validation + triggerClientSideValidation(basicSelectElement); + // Client side state should still be invalid + assertClientSideSelectValidationState(basicSelectElement, false); + } + + @Test + public void testModifyingClientSideValidationStateShouldNotAffectServerSideValidationState() { + // Set server state to invalid + setInvalidButton.click(); + logButton.click(); + Assert.assertEquals("invalid", basicSelectResultSpan.getText()); + + // Overwrite client side validation state to be valid + overwriteClientSideValidationState(basicSelectElement, true); + // Server state should still be invalid + logButton.click(); + Assert.assertEquals("invalid", basicSelectResultSpan.getText()); + } + + @Test + public void testDetachingAndReattachingShouldStillOverrideClientValidation() { + // Set server state to invalid + setInvalidButton.click(); + assertClientSideSelectValidationState(basicSelectElement, false); + + // Detach and reattach + detachButton.click(); + reattachButton.click(); + // Need to refresh Selenium reference + basicSelectElement = $(SelectElement.class) + .id(OverrideClientValidationPage.ID_BASIC_SELECT); + + // Client side state should still be invalid after reattaching + assertClientSideSelectValidationState(basicSelectElement, false); + + // Trigger client side validation after reattaching + triggerClientSideValidation(basicSelectElement); + // Client side state should still be invalid after reattaching and + // triggering validation + assertClientSideSelectValidationState(basicSelectElement, false); + } + + @Test + public void testTriggeringClientValidationShouldNotOverrideClientValidationStateWhenUsedInComponentRenderer() { + // Set server state to invalid + setInvalidButton.click(); + assertClientSideSelectValidationState(selectInGridElement, false); + + triggerClientSideValidation(selectInGridElement); + + assertClientSideSelectValidationState(selectInGridElement, false); + } + + private void assertClientSideSelectValidationState( + SelectElement selectElement, boolean valid) { + Boolean validationState = selectElement.getPropertyBoolean("invalid"); + + Assert.assertEquals("Validation state did not match", !valid, + validationState); + } + + private void triggerClientSideValidation(SelectElement selectElement) { + selectElement.getCommandExecutor() + .executeScript("arguments[0].validate()", selectElement); + getCommandExecutor().waitForVaadin(); + } + + private void overwriteClientSideValidationState(SelectElement selectElement, + boolean valid) { + selectElement.setProperty("invalid", !valid); + getCommandExecutor().waitForVaadin(); + } +} diff --git a/vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/FieldValidationUtil.java b/vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/FieldValidationUtil.java new file mode 100644 index 00000000000..a8429ead6e1 --- /dev/null +++ b/vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/FieldValidationUtil.java @@ -0,0 +1,69 @@ +/* + * Copyright 2000-2019 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.flow.component.select; + +import com.vaadin.flow.internal.StateNode; + +/** + * Utility class for select Flow component to disable client side validation. + * + * @author Vaadin Ltd + */ +final class FieldValidationUtil { + + private FieldValidationUtil() { + // utility class should not be instantiated + } + + static void disableClientValidation(Select component) { + // Since this method should be called for every time when the component + // is attached to the UI, lets check that it is actually so + if (!component.isAttached()) { + throw new IllegalStateException(String.format( + "Component %s is not attached. Client side " + + "validation can only be disabled for a component " + + "when it has been attached to the UI and because " + + "it should be called again once the component is " + + "removed/added, you should call this method from " + + "the onAttach() method of the component.", + component.toString())); + } + // Wait until the response is being written as the validation state + // should not change after that + final StateNode componentNode = component.getElement().getNode(); + componentNode.runWhenAttached(ui -> ui.getInternals().getStateTree() + .beforeClientResponse(componentNode, + executionContext -> overrideClientValidation( + component))); + } + + private static void overrideClientValidation(Select component) { + // Overwrite client validation method to simply return validation state + // set from server + StringBuilder expression = new StringBuilder( + "this.validate = function () {return !this.invalid;};"); + + if (component.isInvalid()) { + /* + * By default the invalid flag is set to false. Workaround the case + * where the client side validation overrides the invalid state + * before the validation function itself is overridden above. + */ + expression.append("this.invalid = true;"); + } + component.getElement().executeJs(expression.toString()); + } +} diff --git a/vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/Select.java b/vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/Select.java index e552f6f94a8..81979a024ec 100644 --- a/vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/Select.java +++ b/vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/Select.java @@ -15,14 +15,6 @@ */ package com.vaadin.flow.component.select; -import java.io.Serializable; -import java.util.Collection; -import java.util.Objects; -import java.util.Optional; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; -import java.util.stream.Stream; - import com.vaadin.flow.component.AttachEvent; import com.vaadin.flow.component.Component; import com.vaadin.flow.component.ComponentUtil; @@ -59,6 +51,14 @@ import com.vaadin.flow.function.SerializablePredicate; import com.vaadin.flow.shared.Registration; +import java.io.Serializable; +import java.util.Collection; +import java.util.Objects; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Stream; + /** * A customizable drop-down select component similar to a native browser select. *

@@ -123,6 +123,10 @@ public Select() { getElement().setProperty("invalid", false); getElement().setProperty("opened", false); + // Trigger model-to-presentation conversion in constructor, so that + // the client side component has a correct initial value of an empty + // string + setPresentationValue(null); getElement().appendChild(listBox.getElement()); @@ -746,6 +750,7 @@ protected boolean hasValidValue() { protected void onAttach(AttachEvent attachEvent) { super.onAttach(attachEvent); initConnector(); + FieldValidationUtil.disableClientValidation(this); } /** diff --git a/vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/generated/GeneratedVaadinSelect.java b/vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/generated/GeneratedVaadinSelect.java index 109fd651dde..2a5c1a62e01 100644 --- a/vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/generated/GeneratedVaadinSelect.java +++ b/vaadin-select-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/generated/GeneratedVaadinSelect.java @@ -392,13 +392,12 @@ protected void setRequired(boolean required) { *

* Set to true if the value is invalid. *

- * This property is synchronized automatically from client side when a - * 'invalid-changed' event happens. + * This property is not synchronized automatically from the client side, so + * the returned value may not be the same as in client side. *

* * @return the {@code invalid} property from the webcomponent */ - @Synchronize(property = "invalid", value = "invalid-changed") protected boolean isInvalidBoolean() { return getElement().getProperty("invalid", false); }