Skip to content

Commit

Permalink
refactor: update TimePicker and BigDecimalField to not use _hasInputV…
Browse files Browse the repository at this point in the history 
…alue
  • Loading branch information
@vursen
vursen committed Nov 21, 2024
1 parent 46679a3 commit 5130a6d
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 53 deletions.
56 changes: 35 additions & 21 deletions ...in-text-field-flow/src/main/java/com/vaadin/flow/component/textfield/BigDecimalField.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@
import java.io.Serializable;
import java.math.BigDecimal;
import java.text.DecimalFormatSymbols;
import java.time.LocalDate;
import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Function;

import com.vaadin.flow.component.AttachEvent;
import com.vaadin.flow.component.Synchronize;
import com.vaadin.flow.component.Tag;
import com.vaadin.flow.component.UI;
import com.vaadin.flow.component.dependency.JsModule;
Expand Down Expand Up @@ -123,7 +123,7 @@ public class BigDecimalField extends TextFieldBase<BigDecimalField, BigDecimal>
boolean fromComponent = context == null;

boolean hasBadInput = valueEquals(value, getEmptyValue())
&& isInputValuePresent();
&& !getInputElementValue().isEmpty();
if (hasBadInput) {
return ValidationResult.error(getI18nErrorMessage(
BigDecimalFieldI18n::getBadInputErrorMessage));
Expand Down Expand Up @@ -166,6 +166,13 @@ public BigDecimalField() {
addValueChangeListener(e -> validate());
}

@Override
public void setValueChangeMode(ValueChangeMode valueChangeMode) {
super.setValueChangeMode(valueChangeMode);
getSynchronizationRegistration()
.synchronizeProperty("_inputElementValue");
}

/**
* Constructs an empty {@code BigDecimalField} with the given label.
*
Expand Down Expand Up @@ -311,27 +318,22 @@ public void setValue(BigDecimal value) {
boolean isOldValueEmpty = valueEquals(oldValue, getEmptyValue());
boolean isNewValueEmpty = valueEquals(value, getEmptyValue());
boolean isValueRemainedEmpty = isOldValueEmpty && isNewValueEmpty;
boolean isInputValuePresent = isInputValuePresent();
boolean isInputElementValueEmpty = getInputElementValue().isEmpty();

// 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);

if (isValueRemainedEmpty && isInputValuePresent) {
// Clear the input element from possible bad input.
getElement().executeJs("this._inputElementValue = ''");
// Revalidate if setValue(null) didn't result in a value change but
// cleared bad input
if (isValueRemainedEmpty && !isInputElementValueEmpty) {
validate();
fireValidationStatusChangeEvent();
} else {
// Restore the input element's value in case it was cleared
// in the above branch. That can happen when setValue(null)
// and setValue(...) are subsequently called within one round-trip
// and there was bad input.
getElement().executeJs("this._inputElementValue = this.value");
}
}

Expand Down Expand Up @@ -403,14 +405,26 @@ private void fireValidationStatusChangeEvent() {
}

/**
* Returns whether the input element has a value or not.
* Gets the value of the input element. This value is updated on the server
* when the web component dispatches a `change` event. Except when clearing
* the value, {@link #setValue(LocalDate)} does not update the input element
* value on the server because it requires date formatting, which is
* implemented on the web component's side.
*
* @return <code>true</code> if the input element's value is populated,
* <code>false</code> otherwise
* @return the value of the input element
*/
private String getInputElementValue() {
return getElement().getProperty("_inputElementValue", "");
}

/**
* Sets the value of the input element.
*
* @param value
* the value to set
*/
@Synchronize(property = "_hasInputValue", value = "has-input-value-changed")
private boolean isInputValuePresent() {
return getElement().getProperty("_hasInputValue", false);
private void setInputElementValue(String value) {
getElement().setProperty("_inputElementValue", value);
}

/**
Expand Down
30 changes: 13 additions & 17 deletions ...va/com/vaadin/flow/component/textfield/validation/BigDecimalFieldBasicValidationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@
*/
package com.vaadin.flow.component.textfield.validation;

import java.lang.reflect.Method;
import java.math.BigDecimal;

import org.junit.Assert;
import org.junit.Test;

import com.vaadin.flow.component.Component;
import com.vaadin.flow.component.textfield.BigDecimalField;
import com.vaadin.flow.dom.Element;
import com.vaadin.tests.validation.AbstractBasicValidationTest;

public class BigDecimalFieldBasicValidationTest
Expand All @@ -34,23 +35,24 @@ public void addValidationStatusChangeListener_addAnotherListenerOnInvocation_noE
});

// Trigger ValidationStatusChangeEvent
simulateBadInput();
fakeClientPropertyChange(testField, "_inputElementValue", "foo");
fakeClientPropertyChange(testField, "value", "foo");
testField.clear();
}

@Test
public void badInput_validate_emptyErrorMessageDisplayed() {
testField.getElement().setProperty("_hasInputValue", true);
simulateBadInput();
fakeClientPropertyChange(testField, "_inputElementValue", "foo");
fakeClientPropertyChange(testField, "value", "foo");
Assert.assertEquals("", testField.getErrorMessage());
}

@Test
public void badInput_setI18nErrorMessage_validate_i18nErrorMessageDisplayed() {
testField.setI18n(new BigDecimalField.BigDecimalFieldI18n()
.setBadInputErrorMessage("Value has invalid format"));
testField.getElement().setProperty("_hasInputValue", true);
simulateBadInput();
fakeClientPropertyChange(testField, "_inputElementValue", "foo");
fakeClientPropertyChange(testField, "value", "foo");
Assert.assertEquals("Value has invalid format",
testField.getErrorMessage());
}
Expand Down Expand Up @@ -104,16 +106,10 @@ protected BigDecimalField createTestField() {
return new BigDecimalField();
}

private void simulateBadInput() {
testField.getElement().setProperty("_hasInputValue", true);

try {
Method validateMethod = BigDecimalField.class
.getDeclaredMethod("validate");
validateMethod.setAccessible(true);
validateMethod.invoke(testField);
} catch (Exception e) {
throw new RuntimeException(e);
}
private void fakeClientPropertyChange(Component component, String property,
String value) {
Element element = component.getElement();
element.getStateProvider().setProperty(element.getNode(), property,
value, false);
}
}
48 changes: 33 additions & 15 deletions ...aadin-time-picker-flow/src/main/java/com/vaadin/flow/component/timepicker/TimePicker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.io.Serializable;
import java.time.Duration;
import java.time.LocalDate;
import java.time.LocalTime;
import java.time.temporal.ChronoUnit;
import java.util.Locale;
Expand Down Expand Up @@ -368,25 +369,18 @@ public void setValue(LocalTime value) {
boolean isValueRemainedEmpty = isOldValueEmpty && isNewValueEmpty;
boolean isInputValuePresent = isInputValuePresent();

// 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.
// Revalidate if setValue(null) didn't result in a value change but
// cleared 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 = ''");
validate();
fireValidationStatusChangeEvent();
}
Expand Down Expand Up @@ -453,9 +447,33 @@ private void fireValidationStatusChangeEvent() {
* @return <code>true</code> if the input element's value is populated,
* <code>false</code> otherwise
*/
@Synchronize(property = "_hasInputValue", value = "has-input-value-changed")
protected boolean isInputValuePresent() {
return getElement().getProperty("_hasInputValue", false);
return !getInputElementValue().isEmpty();
}

/**
* Gets the value of the input element. This value is updated on the server
* when the web component dispatches a `change` or `unparsable-change`
* event. Except when clearing the value, {@link #setValue(LocalDate)} does
* not update the input element value on the server because it requires date
* formatting, which is implemented on the web component's side.
*
* @return the value of the input element
*/
@Synchronize(property = "_inputElementValue", value = { "change",
"unparsable-change" })
private String getInputElementValue() {
return getElement().getProperty("_inputElementValue", "");
}

/**
* Sets the value of the input element.
*
* @param value
* the value to set
*/
private void setInputElementValue(String value) {
getElement().setProperty("_inputElementValue", value);
}

/**
Expand Down

0 comments on commit 5130a6d

Please sign in to comment.