Don't remove error message when the field becomes valid

Fix for #3439
vaadin · Feb 13, 2018 · 5f5a873 · 5f5a873
1 parent a65bc84
commit 5f5a873
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 19 deletions.
diff --git a/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java b/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
@@ -38,7 +38,6 @@
 import java.util.stream.Stream;
 
 import com.googlecode.gentyref.GenericTypeReflector;
-
 import com.vaadin.flow.component.Component;
 import com.vaadin.flow.component.HasText;
 import com.vaadin.flow.component.HasValidation;
@@ -621,7 +620,7 @@ default BindingBuilder<BEAN, TARGET> asRequired(String errorMessage) {
          * @see HasValue#setRequiredIndicatorVisible(boolean)
          * @see HasValue#isEmpty()
          * @return this binding, for chaining
-         * 
+         *
          */
         default BindingBuilder<BEAN, TARGET> asRequired() {
             return asRequired(context -> "");
@@ -2026,7 +2025,7 @@ public Registration addStatusChangeListener(StatusChangeListener listener) {
 
     /**
      * Adds a listener to the binder.
-     * 
+     *
      * @param eventType
      *            the type of the event
      * @param method
@@ -2111,11 +2110,11 @@ protected <FIELDVALUE, TARGET> BindingBuilder<BEAN, TARGET> doCreateBinding(
      *            the validation error result
      */
     protected void handleError(HasValue<?, ?> field, ValidationResult result) {
-            if (field instanceof HasValidation) {
-                HasValidation fieldWithValidation = (HasValidation) field;
-                fieldWithValidation.setInvalid(true);
-                fieldWithValidation.setErrorMessage(result.getErrorMessage());
-            }
+        if (field instanceof HasValidation) {
+            HasValidation fieldWithValidation = (HasValidation) field;
+            fieldWithValidation.setInvalid(true);
+            fieldWithValidation.setErrorMessage(result.getErrorMessage());
+        }
     }
 
     /**
@@ -2128,7 +2127,6 @@ protected void clearError(HasValue<?, ?> field) {
         if (field instanceof HasValidation) {
             HasValidation fieldWithValidation = (HasValidation) field;
             fieldWithValidation.setInvalid(false);
-            fieldWithValidation.setErrorMessage(null);
         }
     }
 

diff --git a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java
@@ -16,7 +16,6 @@
 
 package com.vaadin.flow.data.binder;
 
-import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.isEmptyString;
@@ -476,7 +475,7 @@ public void setRequired_withErrorMessage_fieldGetsRequiredIndicatorAndValidator(
         Assert.assertEquals("foobar", componentErrors.get(textField));
 
         textField.setValue("value");
-        assertThat(textField.getErrorMessage(), isEmptyString());
+        assertFalse(textField.isInvalid());
         assertTrue(textField.isRequiredIndicatorVisible());
     }
 
@@ -526,7 +525,7 @@ public void setRequired_withErrorMessageProvider_fieldGetsRequiredIndicatorAndVa
         Assert.assertEquals(1, invokes.get());
 
         textField.setValue("value");
-        assertThat(textField.getErrorMessage(), isEmptyString());
+        assertFalse(textField.isInvalid());
         assertTrue(textField.isRequiredIndicatorVisible());
     }
 
@@ -880,8 +879,7 @@ public void execute_binding_status_handler_from_binder_status_handler() {
 
         // Restore values and test no errors.
         ageField.setValue(String.valueOf(initialAge));
-        assertThat("There should be no error", ageField.getErrorMessage(),
-                is(""));
+        assertFalse("The field should be valid", ageField.isInvalid());
 
         bindingHandler.expectingError = false;
         nameField.setValue(initialName);

diff --git a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTestBase.java b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTestBase.java
@@ -15,9 +15,6 @@
  */
 package com.vaadin.flow.data.binder;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.isEmptyOrNullString;
-
 import java.io.ByteArrayOutputStream;
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
@@ -78,8 +75,6 @@ void assertInvalidField(String expectedErrorMessage, HasValidation field) {
     }
 
     void assertValidField(HasValidation field) {
-        assertThat("The field should contain no error message",
-                field.getErrorMessage(), isEmptyOrNullString());
         Assert.assertFalse("The field should be valid", field.isInvalid());
     }
 

diff --git a/flow-server/src/main/java/com/vaadin/flow/component/HasValidation.java b/flow-server/src/main/java/com/vaadin/flow/component/HasValidation.java
@@ -25,6 +25,12 @@ public interface HasValidation {
 
     /**
      * Sets an error message to the component.
+     * <p>
+     * The Web Component is responsible for deciding when to show the error
+     * message to the user, and this is usually triggered by triggering the
+     * invalid state for the Web Component. Which means that there is no need to
+     * clean up the message when component becomes valid (otherwise it may lead
+     * to undesired visual effects).
      *
      * @param errorMessage
      *            a new error message
@@ -40,6 +46,10 @@ public interface HasValidation {
 
     /**
      * Sets the validity of the component input.
+     * <p>
+     * When component becomes valid it hides the error message by itself, so
+     * there is no need to clean up the error message via the
+     * {@link #setErrorMessage(String)} call.
      *
      * @param invalid
      *            new value for component input validity