Skip to content

Commit b9a1641

Browse files
authored
fix!: withStatusLabel now sets field invalid state for consistent visual feedback (#22575)
Fixes #21707 Previously, withStatusLabel() only updated the status label but did not set the field's invalid state, resulting in inconsistent visual feedback where fields would not show the red background indicator for validation errors. This fix ensures that withStatusLabel() also delegates to the binder's handleValidationStatus() method, which properly sets/clears the field's invalid state through handleError()/clearError(). This provides consistent visual feedback (red background + status label) across all validation scenarios: - Required field validation - Custom validator failures - Conversion errors - Multiple validators
1 parent b96bcd8 commit b9a1641

File tree

2 files changed

+198
-7
lines changed

2 files changed

+198
-7
lines changed

flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -845,10 +845,11 @@ default BindingBuilder<BEAN, TARGET> withNullRepresentation(
845845
* fails.
846846
* <p>
847847
* The validation state of each field is updated whenever the user
848-
* modifies the value of that field.
849-
* <p>
850-
* This method allows to customize the way a binder displays error
851-
* messages.
848+
* modifies the value of that field. With this method, possible
849+
* validation error message will be shown in the {@code label} provided.
850+
* Additionally, the field's invalid state will be updated to provide
851+
* visual feedback (e.g., red background) for fields implementing
852+
* {@link com.vaadin.flow.component.HasValidation}.
852853
* <p>
853854
* This is just a shorthand for
854855
* {@link #withValidationStatusHandler(BindingValidationStatusHandler)}
@@ -870,6 +871,17 @@ default BindingBuilder<BEAN, TARGET> withStatusLabel(HasText label) {
870871
label.setText(status.getMessage().orElse(""));
871872
// Only show the label when validation has failed
872873
setVisible(label, status.isError());
874+
875+
// Update the field's invalid state for consistent visual
876+
// feedback. Delegates to the binder's error handler which sets
877+
// the field invalid and applies error styling.
878+
if (status.getBinding() instanceof BindingImpl) {
879+
@SuppressWarnings("unchecked")
880+
BindingImpl<BEAN, ?, TARGET> binding = (BindingImpl<BEAN, ?, TARGET>) status
881+
.getBinding();
882+
Binder<BEAN> binder = binding.getBinder();
883+
binder.handleValidationStatus(status);
884+
}
873885
});
874886
}
875887

flow-data/src/test/java/com/vaadin/flow/data/binder/BinderValidationStatusTest.java

Lines changed: 182 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import com.vaadin.flow.data.binder.Binder.BindingBuilder;
3030
import com.vaadin.flow.data.binder.BindingValidationStatus.Status;
3131
import com.vaadin.flow.data.binder.testcomponents.TestLabel;
32+
import com.vaadin.flow.data.binder.testcomponents.TestTextField;
33+
import com.vaadin.flow.data.converter.StringToIntegerConverter;
3234
import com.vaadin.flow.tests.data.bean.Person;
3335

3436
public class BinderValidationStatusTest
@@ -44,6 +46,7 @@ public void setUp() {
4446
@Override
4547
protected void handleError(HasValue<?, ?> field,
4648
ValidationResult result) {
49+
super.handleError(field, result);
4750
componentErrors.put(field, result.getErrorMessage());
4851
}
4952

@@ -127,7 +130,7 @@ public void bindingWithStatusLabel_labelIsUpdatedAccordingStatus() {
127130
}
128131

129132
@Test
130-
public void bindingWithStatusLabel_defaultStatusHandlerIsReplaced() {
133+
public void bindingWithStatusLabel_fieldInvalidStateIsSet() {
131134
TestLabel label = new TestLabel();
132135

133136
Binding<Person, String> binding = binder.forField(nameField)
@@ -142,8 +145,9 @@ public void bindingWithStatusLabel_defaultStatusHandlerIsReplaced() {
142145
// message
143146
binding.validate();
144147

145-
// default behavior should update component error for the nameField
146-
Assert.assertNull(componentErrors.get(nameField));
148+
// withStatusLabel updates both the label and the field's invalid state
149+
Assert.assertEquals(EMPTY_ERROR_MESSAGE,
150+
componentErrors.get(nameField));
147151
}
148152

149153
@Test(expected = IllegalStateException.class)
@@ -480,4 +484,179 @@ private void assertVisible(TestLabel label, boolean visible) {
480484
}
481485
}
482486

487+
// Tests for issue #21707: withStatusLabel should set field invalid state
488+
@Test
489+
public void withStatusLabel_validationError_setsFieldInvalidAndUpdatesLabel() {
490+
TestLabel label = new TestLabel();
491+
492+
binder.forField(nameField).withValidator(notEmpty)
493+
.withStatusLabel(label)
494+
.bind(Person::getFirstName, Person::setFirstName);
495+
496+
nameField.setValue("");
497+
binder.validate();
498+
499+
// Both label and field invalid state should be updated
500+
assertVisible(label, true);
501+
Assert.assertEquals(EMPTY_ERROR_MESSAGE, label.getText());
502+
Assert.assertTrue(nameField.isInvalid());
503+
Assert.assertEquals(EMPTY_ERROR_MESSAGE,
504+
componentErrors.get(nameField));
505+
}
506+
507+
@Test
508+
public void withStatusLabel_validValue_clearsFieldInvalidAndLabel() {
509+
TestLabel label = new TestLabel();
510+
511+
binder.forField(nameField).withValidator(notEmpty)
512+
.withStatusLabel(label)
513+
.bind(Person::getFirstName, Person::setFirstName);
514+
515+
// First set invalid
516+
nameField.setValue("");
517+
binder.validate();
518+
Assert.assertTrue(nameField.isInvalid());
519+
520+
// Then set valid
521+
nameField.setValue("Valid");
522+
binder.validate();
523+
524+
// Both label and field invalid state should be cleared
525+
assertVisible(label, false);
526+
Assert.assertEquals("", label.getText());
527+
Assert.assertFalse(nameField.isInvalid());
528+
Assert.assertNull(componentErrors.get(nameField));
529+
}
530+
531+
@Test
532+
public void withStatusLabel_customValidator_setsFieldInvalid() {
533+
TestLabel label = new TestLabel();
534+
String customError = "Must start with uppercase";
535+
536+
binder.forField(nameField)
537+
.withValidator(value -> Character.isUpperCase(value.charAt(0)),
538+
customError)
539+
.withStatusLabel(label)
540+
.bind(Person::getFirstName, Person::setFirstName);
541+
542+
nameField.setValue("lowercase");
543+
binder.validate();
544+
545+
// Field should be marked invalid with custom validator
546+
Assert.assertTrue(nameField.isInvalid());
547+
Assert.assertEquals(customError, componentErrors.get(nameField));
548+
assertVisible(label, true);
549+
Assert.assertEquals(customError, label.getText());
550+
}
551+
552+
@Test
553+
public void withStatusLabel_requiredField_setsFieldInvalid() {
554+
TestLabel label = new TestLabel();
555+
556+
binder.forField(nameField).asRequired(EMPTY_ERROR_MESSAGE)
557+
.withStatusLabel(label)
558+
.bind(Person::getFirstName, Person::setFirstName);
559+
560+
nameField.setValue("");
561+
binder.validate();
562+
563+
// Required field validation should set field invalid
564+
Assert.assertTrue(nameField.isInvalid());
565+
Assert.assertEquals(EMPTY_ERROR_MESSAGE,
566+
componentErrors.get(nameField));
567+
assertVisible(label, true);
568+
Assert.assertEquals(EMPTY_ERROR_MESSAGE, label.getText());
569+
}
570+
571+
@Test
572+
public void withStatusLabel_conversionError_setsFieldInvalid() {
573+
TestLabel label = new TestLabel();
574+
TestTextField ageField = new TestTextField();
575+
String conversionError = "Must be a number";
576+
577+
binder.forField(ageField)
578+
.withConverter(new StringToIntegerConverter(conversionError))
579+
.withStatusLabel(label).bind(Person::getAge, Person::setAge);
580+
581+
ageField.setValue("not a number");
582+
binder.validate();
583+
584+
// Conversion error should set field invalid
585+
Assert.assertTrue(ageField.isInvalid());
586+
assertVisible(label, true);
587+
Assert.assertEquals(conversionError, label.getText());
588+
}
589+
590+
@Test
591+
public void withStatusLabel_multipleValidators_setsFieldInvalid() {
592+
TestLabel label = new TestLabel();
593+
String lengthError = "Must be at least 3 characters";
594+
595+
binder.forField(nameField).withValidator(notEmpty)
596+
.withValidator(value -> value.length() >= 3, lengthError)
597+
.withStatusLabel(label)
598+
.bind(Person::getFirstName, Person::setFirstName);
599+
600+
nameField.setValue("ab");
601+
binder.validate();
602+
603+
// Multiple validators should still set field invalid
604+
Assert.assertTrue(nameField.isInvalid());
605+
Assert.assertEquals(lengthError, componentErrors.get(nameField));
606+
assertVisible(label, true);
607+
Assert.assertEquals(lengthError, label.getText());
608+
}
609+
610+
@Test
611+
public void withValidationStatusHandler_customHandler_fieldInvalidNotSetAutomatically() {
612+
AtomicReference<BindingValidationStatus<?>> statusCapture = new AtomicReference<>();
613+
614+
binder.forField(nameField).withValidator(notEmpty)
615+
.withValidationStatusHandler(statusCapture::set)
616+
.bind(Person::getFirstName, Person::setFirstName);
617+
618+
nameField.setValue("");
619+
binder.validate();
620+
621+
// Custom handler gives full control - field invalid state should NOT
622+
// be set automatically
623+
Assert.assertFalse(nameField.isInvalid());
624+
Assert.assertNull(componentErrors.get(nameField));
625+
626+
// But the handler should still receive the validation status
627+
Assert.assertNotNull(statusCapture.get());
628+
Assert.assertEquals(Status.ERROR, statusCapture.get().getStatus());
629+
Assert.assertEquals(EMPTY_ERROR_MESSAGE,
630+
statusCapture.get().getMessage().get());
631+
}
632+
633+
@Test
634+
public void withValidationStatusHandler_customHandler_canManuallySetFieldInvalid() {
635+
binder.forField(nameField).withValidator(notEmpty)
636+
.withValidationStatusHandler(status -> {
637+
// Custom handler that manually controls field invalid state
638+
if (status.isError()) {
639+
nameField.setInvalid(true);
640+
} else {
641+
nameField.setInvalid(false);
642+
}
643+
}).bind(Person::getFirstName, Person::setFirstName);
644+
645+
nameField.setValue("");
646+
binder.validate();
647+
648+
// Custom handler manually set the invalid state
649+
Assert.assertTrue(nameField.isInvalid());
650+
// But componentErrors should still be null because we're using custom
651+
// handler
652+
Assert.assertNull(componentErrors.get(nameField));
653+
654+
// Clear the error
655+
nameField.setValue("Valid");
656+
binder.validate();
657+
658+
// Custom handler should clear the invalid state
659+
Assert.assertFalse(nameField.isInvalid());
660+
}
661+
483662
}

0 commit comments

Comments
 (0)