Skip to content

Commit

Permalink
refactor: do not rely on _hasInputValue for detecting bad input (#6991)
Browse files Browse the repository at this point in the history
  • Loading branch information
vursen authored Jan 3, 2025
1 parent 3f566fd commit 6da1b8f
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.function.Function;

import com.vaadin.flow.component.AttachEvent;
import com.vaadin.flow.component.Synchronize;
import com.vaadin.flow.component.shared.ClientValidationUtil;
import com.vaadin.flow.component.shared.ValidationUtil;
import com.vaadin.flow.component.shared.internal.ValidationController;
Expand Down Expand Up @@ -66,7 +65,7 @@ public abstract class AbstractNumberField<C extends AbstractNumberField<C, T>, T
boolean fromComponent = context == null;

boolean hasBadInput = valueEquals(value, getEmptyValue())
&& isInputValuePresent();
&& !getInputElementValue().isEmpty();
if (hasBadInput) {
return ValidationResult.error(getI18nErrorMessage(
AbstractNumberFieldI18n::getBadInputErrorMessage));
Expand Down Expand Up @@ -152,7 +151,7 @@ public AbstractNumberField(SerializableFunction<String, T> parser,
getElement().addEventListener("unparsable-change", e -> {
validate();
fireValidationStatusChangeEvent();
});
}).synchronizeProperty("_inputElementValue");
}

@Override
Expand All @@ -175,6 +174,9 @@ public void setValueChangeMode(ValueChangeMode valueChangeMode) {
}

super.setValueChangeMode(valueChangeMode);

getSynchronizationRegistration()
.synchronizeProperty("_inputElementValue");
}

@Override
Expand Down Expand Up @@ -230,27 +232,20 @@ public void setValue(T value) {
boolean isOldValueEmpty = valueEquals(oldValue, getEmptyValue());
boolean isNewValueEmpty = valueEquals(value, getEmptyValue());
boolean isValueRemainedEmpty = isOldValueEmpty && isNewValueEmpty;
boolean isInputValuePresent = isInputValuePresent();
String oldInputElementValue = getInputElementValue();

// When the value is cleared programmatically, reset hasInputValue
// so that the following validation doesn't treat this as bad input.
// When the value is cleared programmatically, there is no change event
// that would synchronize _inputElementValue, so we reset it ourselves
// to prevent the following validation from treating this as bad input.
if (isNewValueEmpty) {
getElement().setProperty("_hasInputValue", false);
setInputElementValue("");
}

super.setValue(value);

// Clear the input element from possible bad input.
if (isValueRemainedEmpty && isInputValuePresent) {
// The check for value presence guarantees that a non-empty value
// won't get cleared when setValue(null) and setValue(...) are
// subsequently called within one round-trip.
// Flow only sends the final component value to the client
// when you update the value multiple times during a round-trip
// and the final value is sent in place of the first one, so
// `executeJs` can end up invoked after a non-empty value is set.
getElement()
.executeJs("if (!this.value) this._inputElementValue = ''");
// Revalidate if setValue(null) didn't result in a value change but
// cleared bad input
if (isValueRemainedEmpty && !oldInputElementValue.isEmpty()) {
validate();
fireValidationStatusChangeEvent();
}
Expand Down Expand Up @@ -341,15 +336,12 @@ protected double getStepDouble() {
return step;
}

/**
* Returns whether the input element has a value or not.
*
* @return <code>true</code> if the input element's value is populated,
* <code>false</code> otherwise
*/
@Synchronize(property = "_hasInputValue", value = "has-input-value-changed")
private boolean isInputValuePresent() {
return getElement().getProperty("_hasInputValue", false);
private String getInputElementValue() {
return getElement().getProperty("_inputElementValue", "");
}

private void setInputElementValue(String value) {
getElement().setProperty("_inputElementValue", value);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
import org.junit.Assert;
import org.junit.Test;

import com.vaadin.flow.component.Component;
import com.vaadin.flow.component.textfield.IntegerField;
import com.vaadin.flow.dom.DomEvent;
import com.vaadin.flow.dom.Element;
import com.vaadin.flow.internal.nodefeature.ElementListenerMap;
import com.vaadin.tests.validation.AbstractBasicValidationTest;

Expand All @@ -35,23 +37,23 @@ public void addValidationStatusChangeListener_addAnotherListenerOnInvocation_noE
});

// Trigger ValidationStatusChangeEvent
testField.getElement().setProperty("_hasInputValue", true);
fakeClientPropertyChange(testField, "_inputElementValue", "NaN");
testField.clear();
}

@Test
public void badInput_validate_emptyErrorMessageDisplayed() {
testField.getElement().setProperty("_hasInputValue", true);
fireUnparsableChangeDomEvent();
fakeClientPropertyChange(testField, "_inputElementValue", "NaN");
fakeClientDomEvent(testField, "unparsable-change");
Assert.assertEquals("", testField.getErrorMessage());
}

@Test
public void badInput_setI18nErrorMessage_validate_i18nErrorMessageDisplayed() {
testField.setI18n(new IntegerField.IntegerFieldI18n()
.setBadInputErrorMessage("Value has invalid format"));
testField.getElement().setProperty("_hasInputValue", true);
fireUnparsableChangeDomEvent();
fakeClientPropertyChange(testField, "_inputElementValue", "NaN");
fakeClientDomEvent(testField, "unparsable-change");
Assert.assertEquals("Value has invalid format",
testField.getErrorMessage());
}
Expand Down Expand Up @@ -137,10 +139,16 @@ protected IntegerField createTestField() {
return new IntegerField();
}

private void fireUnparsableChangeDomEvent() {
DomEvent unparsableChangeDomEvent = new DomEvent(testField.getElement(),
"unparsable-change", Json.createObject());
testField.getElement().getNode().getFeature(ElementListenerMap.class)
.fireEvent(unparsableChangeDomEvent);
private void fakeClientDomEvent(Component component, String eventName) {
Element element = component.getElement();
DomEvent event = new DomEvent(element, eventName, Json.createObject());
element.getNode().getFeature(ElementListenerMap.class).fireEvent(event);
}

private void fakeClientPropertyChange(Component component, String property,
String value) {
Element element = component.getElement();
element.getStateProvider().setProperty(element.getNode(), property,
value, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
import org.junit.Assert;
import org.junit.Test;

import com.vaadin.flow.component.Component;
import com.vaadin.flow.component.textfield.NumberField;
import com.vaadin.flow.dom.DomEvent;
import com.vaadin.flow.dom.Element;
import com.vaadin.flow.internal.nodefeature.ElementListenerMap;
import com.vaadin.tests.validation.AbstractBasicValidationTest;

Expand All @@ -35,23 +37,23 @@ public void addValidationStatusChangeListener_addAnotherListenerOnInvocation_noE
});

// Trigger ValidationStatusChangeEvent
testField.getElement().setProperty("_hasInputValue", true);
fakeClientPropertyChange(testField, "_inputElementValue", "NaN");
testField.clear();
}

@Test
public void badInput_validate_emptyErrorMessageDisplayed() {
testField.getElement().setProperty("_hasInputValue", true);
fireUnparsableChangeDomEvent();
fakeClientPropertyChange(testField, "_inputElementValue", "NaN");
fakeClientDomEvent(testField, "unparsable-change");
Assert.assertEquals("", testField.getErrorMessage());
}

@Test
public void badInput_setI18nErrorMessage_validate_i18nErrorMessageDisplayed() {
testField.setI18n(new NumberField.NumberFieldI18n()
.setBadInputErrorMessage("Value has invalid format"));
testField.getElement().setProperty("_hasInputValue", true);
fireUnparsableChangeDomEvent();
fakeClientPropertyChange(testField, "_inputElementValue", "NaN");
fakeClientDomEvent(testField, "unparsable-change");
Assert.assertEquals("Value has invalid format",
testField.getErrorMessage());
}
Expand Down Expand Up @@ -137,10 +139,16 @@ protected NumberField createTestField() {
return new NumberField();
}

private void fireUnparsableChangeDomEvent() {
DomEvent unparsableChangeDomEvent = new DomEvent(testField.getElement(),
"unparsable-change", Json.createObject());
testField.getElement().getNode().getFeature(ElementListenerMap.class)
.fireEvent(unparsableChangeDomEvent);
private void fakeClientDomEvent(Component component, String eventName) {
Element element = component.getElement();
DomEvent event = new DomEvent(element, eventName, Json.createObject());
element.getNode().getFeature(ElementListenerMap.class).fireEvent(event);
}

private void fakeClientPropertyChange(Component component, String property,
String value) {
Element element = component.getElement();
element.getStateProvider().setProperty(element.getNode(), property,
value, false);
}
}

0 comments on commit 6da1b8f

Please sign in to comment.