Allow to adjust value change mode for components.

[SomeoneToIgnore]

This change is 

[vaadin-bot]

SonarQube analysis reported 4 issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

1. SimpleElementBindingStrategy.java#: This file has 795 lines, which is greater than 750 authorized. Split it into smaller files.
2. SimpleElementBindingStrategy.java#L707: Refactor this method to reduce its Cognitive Complexity from 17 to the 15 allowed.
3. HasValue.java#L49: Make "oldValue" transient or serializable.
4. HasValue.java#L50: Make "value" transient or serializable.

[denis-anisimov]

Reviewed 1 of 5 files at r1.
Review status: 1 of 5 files reviewed at latest revision, 1 unresolved discussion.

---

flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java, line 282 at r2 (raw file):

if (containsProperty(

Quoted 7 lines of code…

> model.getList(NodeFeatures.SYNCHRONIZED_PROPERTIES), > subProperty)) { > Console.debug("Ignoring property change for property '" > + fullPropertyName > + "' which is intended to be synchronized separately"); > return; > }

Is it ?

If the property is the model property then it doesn't have to be synchronized.
We should get notifications about property change even if the property is not synchornized explicitly (from the server side via registering the event listener for it).

Every model property is expected to "fire" server notification event (without any explicit synchronization).
It might be that the situation is not really correct since we will receive the RPC for the properties that we don't really need but that what we have at the moment.
This is done e.g. to be able to get the most recent value from the TemplateModel instance if it has a getter (which is quite common). And the only case when it's not needed is the only setter presence for the property in the model.

So it looks like this code is incorrect.
It should check whether the property is the model property and in this case let it goes as it has been done previously. And if it's not a model property then block its notifications.

---

Comments from Reviewable

[denis-anisimov]

Reviewed 2 of 5 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java, line 282 at r2 (raw file):

Previously, denis-anisimov (Denis) wrote…

if (containsProperty(
model.getList(NodeFeatures.SYNCHRONIZED_PROPERTIES),
subProperty)) {
Console.debug("Ignoring property change for property '"
+ fullPropertyName
+ "' which is intended to be synchronized separately");
return;
}

Is it ?

If the property is the model property then it doesn't have to be synchronized.
We should get notifications about property change even if the property is not synchornized explicitly (from the server side via registering the event listener for it).

Every model property is expected to "fire" server notification event (without any explicit synchronization).
It might be that the situation is not really correct since we will receive the RPC for the properties that we don't really need but that what we have at the moment.
This is done e.g. to be able to get the most recent value from the TemplateModel instance if it has a getter (which is quite common). And the only case when it's not needed is the only setter presence for the property in the model.

So it looks like this code is incorrect.
It should check whether the property is the model property and in this case let it goes as it has been done previously. And if it's not a model property then block its notifications.

This approach only improves the situation a bit disabling duplicating events in case of polymer element property.

It's OK for now but the separate ticket about disabling this code in case of non-model property is required.

---

Comments from Reviewable

[SomeoneToIgnore]

flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java, line 282 at r2 (raw file):

Previously, denis-anisimov (Denis) wrote…

This approach only improves the situation a bit disabling duplicating events in case of polymer element property.

It's OK for now but the separate ticket about disabling this code in case of non-model property is required.

#3425

---

Comments from Reviewable

[bmomani1]

will this feature be available for combo box too, because in my case, I have a combo box and bound to
LazyQueryContainer as its container DataSource, so we should take control over firing value change event to reduce server requests.