Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#12132 leads to NPEs #12207

Closed
lennartfricke opened this issue Feb 25, 2021 · 1 comment · Fixed by #12218
Closed

#12132 leads to NPEs #12207

lennartfricke opened this issue Feb 25, 2021 · 1 comment · Fixed by #12218

Comments

@lennartfricke
Copy link
Contributor

With Vaadin 8.12.1, we get NPE Exceptions in code introduced in #12132.

As also pointed out in #12132 (comment).

Though it is rather unlikely that a field_value == null leads to a model_value != null, there might be cases this happens.

We encountered null -> Optional.empty -> null, but chains like null -> default value -> null might also be possible.
This also questions the null check of value in line

Maybe the solution is:

FIELDVALUE converted = convertToFieldType(value);
if (!Objects.equals(field.getValue(), converted)) {
    getField().setValue(converted);
}

The code leads to "immediate" normalization of values in the gui by running the conversion back and forth and makes it more difficult to allow users to enter values that have multiple representations e.g. enter decimal values too slow with StringToDoubleConverter and default TextFields is quite challenging when typing slowly. 😉

@TatuLund
Copy link
Contributor

TatuLund commented Mar 1, 2021

The code leads to "immediate" normalization of values in the gui by running the conversion back and forth and makes it more difficult to allow users to enter values that have multiple representations

There are different value change modes in TextField to overcome the possible issues regarding this. If you use eager mode, the input can become clunky, but you can use delayed mode instead and control it by parameters, so that TextField waits for next key press longer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants