parttio / textfieldformatter

TextField Formatter Vaadin add-on
Apache License 2.0
13 stars 9 forks source link

NumeralFormatter extended TextField skips/breaks .asRequired("") Binder validation #11

Closed m-bermel closed 6 years ago

m-bermel commented 6 years ago

Hi,

we have a case where we extend our TextField with the NumeralFormatter. This kind of breaks the Binder.asRequired. When the user removes his input the asRequired Validator isn't triggered and the TextField is marked as valid in the UI.

Example:

TextField birthYearField = new TextField();
new NumeralFieldFormatter("", "", 4, 0, true).extend(birthYearField );

binder.forField(birthYearField)
  .asRequired("Person must have birthyear")
  .bind(Person::getYear, Person::setYear);

As a workaround we're using the following Validator in addition to the asRequired

.withValidator(value ->  {
    return (null != value && value.longValue() >= 0);
},"")

I'm assuming the extended TextField returns an empty Long which then gets validated against HasValue.getEmptyValue (which returns null) and therefore is always valid.

johannesh2 commented 6 years ago

I cannot reproduce this. Can you post the complete binding code. I can't see the converter in your snippet. At least with the following code it works with or without the the extension with Vaadin 8.0.7

public class MyUI extends UI {
    @Override
    protected void init(VaadinRequest request) {
        TextField tf = new TextField();
        new NumeralFieldFormatter("", "", 4, 0, true).extend(tf);
        tf.addValueChangeListener(l -> Notification.show("Value: " + l.getValue()));
        Binder<Birthday> binder = new Binder<>();
        binder.forField(tf).asRequired("Year must be set").withConverter(new StringToLongConverter("Must be a number"))
                .bind(Birthday::getYear, Birthday::setYear);
        binder.setBean(new Birthday());
        setContent(tf);
    }

    public static class Birthday {
        private Long year = 0L;

        public synchronized Long getYear() {
            return year;
        }

        public synchronized void setYear(Long year) {
            this.year = year;
        }

    }
}
m-bermel commented 6 years ago

Converter is the standard Vaadin StringToLongConverter(). Maybe I should have mentioned that we're using Vaadin 8.3.3.. Sorry for that.

I can try to create code that reproduces the problem next week when I'm back at work. It's not urgent.

johannesh2 commented 6 years ago

The code I posted above seems to work also with 8.3.3.

m-bermel commented 6 years ago

Okay, this seems to be our fault so I'm closing this issue.

You can reproduce the behavior I described with the attached code below. But this isn't a problem with your TextField Formatter. It's more the order which we used in the binding. We first call .withConverter and then use the .asRequired... I tested it in our program and swapped the order to first use the .asRequired and then .withConverterand and now it works as intended.

Thank you for your time and sorry for wasting it. At least I learned something out of it.

image

    @Override
    protected void init(VaadinRequest request) {
        FormLayout fl = new FormLayout();

        TextField tf = new TextField();
        new NumeralFieldFormatter(".", ",", 2, 0, true).extend(tf);
        tf.addValueChangeListener(l -> Notification.show("Value: " + l.getValue()));

        Label validationLabel = new Label();

        Binder<Birthday> binder = new Binder<>();

        binder.forField(tf)
            .withConverter(new StringToLongConverter("Must be a number"))
            .asRequired("Year must be set")
            .bind(Birthday::getYear, Birthday::setYear);
        binder.setBean(new Birthday());

        Button validateButton = new Button("Validate", e -> {
            validationLabel.setValue(binder.validate().isOk() ? "true" : "false");
        });

        fl.addComponent(tf);
        fl.addComponent(validationLabel);
        fl.addComponent(validateButton);

        tf.setCaption("Input");
        validationLabel.setCaption("Valid?");

        setContent(fl);
    }

    public static class Birthday {
        private Long year = 0L;

        public synchronized Long getYear() {
            return year;
        }

        public synchronized void setYear(Long year) {
            this.year = year;
        }

    }
johannesh2 commented 6 years ago

You're welcome. I'm glad I could help.