Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions CONVENTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,12 @@ with `isValid()` rather than refusing the value at set time.

Refuse a value only when the real control physically cannot produce it: a
slider clamps to its range and snaps to its step, so `RangeInputTester` and
`NumberSliderTester` do reject out-of-range and off-step values. Structural
refusals stay too, such as `null` on a field whose empty value is not `null`.
`NumberSliderTester` do reject out-of-range and off-step values, and a text
input truncates at `maxLength` and filters the keystrokes `allowedCharPattern`
does not match, so `TextFieldTester` and `TextAreaTester` reject a value that
breaks either one. `minLength`, `pattern` and required are validation-only and
keep committing an invalid value. Structural refusals stay too, such as `null`
on a field whose empty value is not `null`.

Read-only state counts towards usability. `isUsable()` is enabled + attached +
effectively visible + not inert + not read-only, and effective visibility walks
Expand Down
13 changes: 12 additions & 1 deletion guidelines/testers.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,20 @@ for it — not enforced at set time.

The exception is a control that physically cannot produce the value: a slider
clamps to its range and snaps to its step, so `RangeInputTester` and
`NumberSliderTester` do refuse out-of-range and off-step values. Structural
`NumberSliderTester` do refuse out-of-range and off-step values. A text input
truncates what is over `maxLength` and filters out the keystrokes
`allowedCharPattern` does not match, so `TextFieldTester` and `TextAreaTester`
refuse a value that breaks either one — while `minLength`, `pattern` and
required stay validation-only and keep committing an invalid value. Structural
refusals stay too, such as `null` on a field whose empty value is not `null`.

Where the line falls is a question about the control, not about the constraint:
ask whether a user sitting in front of the component could hand the field that
value at all. When they could not, refuse it with an `IllegalArgumentException`
whose message says what the browser does instead, rather than silently
correcting the value — a test that asks for the impossible has a bug in it, and
truncating or clamping behind its back would hide it.

`isValid()` means "not marked invalid, and the current value passes the
component's own default validator" — it delegates to `getDefaultValidator()`
rather than re-checking required / `min` / `max` / `step` by hand. Re-checking
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,26 +93,33 @@ public void nonInteractableField_throwsOnSetValue() {
@Test
void textAreaWithValidation_doNotPreventInvalid_doNotThrow() {
// Only accept numbers
view.textArea.setAllowedCharPattern("\\d*");
view.textArea.setPattern("\\d*");

final TextAreaTester<TextArea> ta_ = test(view.textArea);
final String faultyValue = "Invalid value, but doesn't throw";
ta_.setValue(faultyValue);
Assertions.assertEquals(faultyValue, view.textArea.getValue(),
"Value should have been set.");
Assertions.assertTrue(ta_.getComponent().isInvalid(),
"A validation-only constraint leaves the field invalid");
}

@Test
public void textAreaWithPattern_patternIsValidated() {
public void textAreaWithAllowedCharPattern_disallowedCharIsRefused() {
TextArea tf = view.textArea;
// Only accept numbers
tf.setPattern("\\d*");
tf.setAllowedCharPattern("\\d");

final TextAreaTester<TextArea> ta_ = test(tf);
ta_.setValue("1234");

Assertions.assertEquals("1234", tf.getValue());
Assertions.assertFalse(ta_.getComponent().isInvalid());

Assertions.assertThrows(IllegalArgumentException.class,
() -> ta_.setValue("hello"),
"The browser filters out a keystroke the allowed char pattern "
+ "does not match");
Assertions.assertEquals("1234", tf.getValue(),
"A refused value should not have been committed");
}

@Test
Expand All @@ -126,13 +133,29 @@ public void textAreaWithMinLength_lengthIsChecked() {
}

@Test
public void textAreaWithMaxLength_lengthIsChecked() {
public void textAreaWithMaxLength_longerValueIsRefused() {
TextArea tf = view.textArea;
tf.setMaxLength(3);

final TextAreaTester<TextArea> ta_ = test(tf);
ta_.setValue("1234");
Assertions.assertTrue(ta_.getComponent().isInvalid());
ta_.setValue("123");
Assertions.assertEquals("123", tf.getValue(),
"A value at the limit should have been set");

Assertions.assertThrows(IllegalArgumentException.class,
() -> ta_.setValue("1234"),
"The browser truncates the characters over maxLength");
Assertions.assertEquals("123", tf.getValue(),
"A refused value should not have been committed");
}

@Test
public void textArea_nullValue_isRefused() {
final TextAreaTester<TextArea> ta_ = test(view.textArea);

Assertions.assertThrows(IllegalArgumentException.class,
() -> ta_.setValue(null),
"A text area has no null state, clear() empties it");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,20 +102,32 @@ void textFieldWithValidation_doNotPreventInvalid_doNotThrow() {
tf_.setValue(faultyValue);
Assertions.assertEquals(faultyValue, view.textField.getValue(),
"Value should have been set.");
Assertions.assertTrue(tf_.getComponent().isInvalid(),
"A validation-only constraint leaves the field invalid");
}

@Test
public void textFieldWithPattern_patternIsValidated() {
public void textFieldWithAllowedCharPattern_disallowedCharIsRefused() {
TextField tf = view.textField;
// Only accept numbers
tf.setAllowedCharPattern("\\d*");
tf.setAllowedCharPattern("\\d");

final TextFieldTester<TextField, String> tf_ = test(tf);
tf_.setValue("1234");

Assertions.assertEquals("1234", tf.getValue());

Assertions.assertThrows(IllegalArgumentException.class,
() -> tf_.setValue("hello"),
"The browser filters out a keystroke the allowed char pattern "
+ "does not match");
Assertions.assertEquals("1234", tf.getValue(),
"A refused value should not have been committed");

// The browser only warns about a pattern it cannot compile.
tf.setAllowedCharPattern("[0-9");
tf_.setValue("hello");
Assertions.assertFalse(tf_.getComponent().isInvalid());
Assertions.assertEquals("hello", tf.getValue(),
"A pattern that is not a regular expression restricts nothing");
}

@Test
Expand All @@ -129,13 +141,28 @@ public void textFieldWithMinLength_lengthIsChecked() {
}

@Test
public void textFieldWithMaxLength_lengthIsChecked() {
public void textFieldWithMaxLength_longerValueIsRefused() {
TextField tf = view.textField;
tf.setMaxLength(3);

final TextFieldTester<TextField, String> tf_ = test(tf);
tf_.setValue("1234");
Assertions.assertTrue(tf_.getComponent().isInvalid());
tf_.setValue("123");
Assertions.assertEquals("123", tf.getValue(),
"A value at the limit should have been set");

Assertions.assertThrows(IllegalArgumentException.class,
() -> tf_.setValue("1234"),
"The browser truncates the characters over maxLength");
Assertions.assertEquals("123", tf.getValue(),
"A refused value should not have been committed");

tf.setMaxLength(0);
Assertions.assertThrows(IllegalArgumentException.class,
() -> tf_.setValue("1"),
"A maxLength of zero is a limit of zero, not an unset limit");
tf_.setValue("");
Assertions.assertEquals("", tf.getValue(),
"Emptying the field stays possible under any limit");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,35 @@ public TextAreaTester(T component) {
}

/**
* Set the value to the component if it is usable.
*
* For a non interactable component an IllegalStateException will be thrown
* as the end user would not be able to set a value.
* Set the given value for the component, as the user would type it.
* <p/>
* A value that only breaks a validation constraint — shorter than
* {@literal minLength}, not matching {@literal pattern}, or the empty value
* on a required field — is committed all the same, because the browser
* commits it too and simply leaves the field invalid. Assert that outcome
* with {@link com.vaadin.flow.component.HasValidation#isInvalid()} instead
* of expecting this method to throw.
* <p/>
* A value the user physically cannot type is refused: the browser truncates
* what is over {@literal maxLength} and filters out the keystrokes
* {@literal allowedCharPattern} does not match, so a longer value or a
* disallowed character fails with an {@link IllegalArgumentException}. So
* does {@code null}, as a text area has no null state — emptying the field
* is {@link #clear()}.
*
* @param value
* value to set
* @throws IllegalStateException
* if the component is not usable
* @throws IllegalArgumentException
* if the value is one the user could not have typed
*/
public void setValue(String value) {
ensureComponentIsUsable();

TextInputConstraints.ensureValueIsNotNull(getComponent(), value);
TextInputConstraints.ensureValueCanBeTyped(getComponent(), value);

setValueAsUser(value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,35 @@ public TextFieldTester(T component) {
}

/**
* Set the value to the component if it is usable.
*
* For a non interactable component an IllegalStateException will be thrown
* as the end user would not be able to set a value.
* Set the given value for the component, as the user would type it.
* <p/>
* A value that only breaks a validation constraint — shorter than
* {@literal minLength}, not matching {@literal pattern}, or the empty value
* on a required field — is committed all the same, because the browser
* commits it too and simply leaves the field invalid. Assert that outcome
* with {@link com.vaadin.flow.component.HasValidation#isInvalid()} instead
* of expecting this method to throw.
* <p/>
* A value the user physically cannot type is refused: the browser truncates
* what is over {@literal maxLength} and filters out the keystrokes
* {@literal allowedCharPattern} does not match, so a longer value or a
* disallowed character fails with an {@link IllegalArgumentException}. So
* does {@code null} on a field whose empty value is not {@code null}, as a
* text input has no null state — emptying the field is {@link #clear()}.
*
* @param value
* value to set
* @throws IllegalStateException
* if the component is not usable
* @throws IllegalArgumentException
* if the value is one the user could not have typed
*/
public void setValue(V value) {
ensureComponentIsUsable();

if (value == null && getComponent().getEmptyValue() != null) {
throw new IllegalArgumentException(
"Field doesn't allow null values");
TextInputConstraints.ensureValueIsNotNull(getComponent(), value);
if (value instanceof String text) {
TextInputConstraints.ensureValueCanBeTyped(getComponent(), text);
}

setValueAsUser(value);
Expand Down
Loading
Loading