fix: client side value binding logic (#20431) (#20443)

Changes client side value binding logic so that if the user modifies the value during server round-trip, the value earlier sent to the server no longer overwrites user's changes once the round-trip finishes. Instead, user's changes are preserved. However, if the server-side value change handling logic actually changes the value and returns the new value to the client, that value will override any user input during round-trip.

Fixes #20365

Co-authored-by: Teppo Kurki <[email protected]>

flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java

@@ -15,7 +15,7 @@
  */
 package com.vaadin.client.flow.binding;
 
-import java.util.function.BiConsumer;
+import java.util.Optional;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
@@ -737,11 +737,20 @@ private void updateProperty(MapProperty mapProperty, Element element) {
         if (mapProperty.hasValue()) {
             Object treeValue = mapProperty.getValue();
             Object domValue = WidgetUtil.getJsProperty(element, name);
+            Optional<Object> previousDomValue = mapProperty
+                    .getPreviousDomValue();
+
+            // User might have modified DOM value during server round-trip.
+            // That is why we only want to update to the tree value if the tree
+            // value is different from the pre-server-round-trip DOM value.
+            boolean updateToTreeValue = previousDomValue
+                    .map(o -> !WidgetUtil.equals(treeValue, o)).orElse(true);
+
             // We compare with the current property to avoid setting properties
             // which are updated on the client side, e.g. when synchronizing
             // properties to the server (won't work for readonly properties).
-            if (WidgetUtil.isUndefined(domValue)
-                    || !WidgetUtil.equals(domValue, treeValue)) {
+            if (updateToTreeValue && (WidgetUtil.isUndefined(domValue)
+                    || !WidgetUtil.equals(domValue, treeValue))) {
                 Reactive.runWithComputation(null,
                         () -> WidgetUtil.setJsProperty(element, name,
                                 PolymerUtils.createModelTree(treeValue)));
@@ -753,6 +762,7 @@ private void updateProperty(MapProperty mapProperty, Element element) {
             // the value
             WidgetUtil.setJsProperty(element, name, null);
         }
+        mapProperty.clearPreviousDomValue();
     }
 
     private void updateStyleProperty(MapProperty mapProperty, Element element) {
@@ -1317,6 +1327,12 @@ private void handleDomEvent(Event event, BindingContext context) {
                 eventData.put(expressionString, expressionValue);
             }
         }
+        synchronizeProperties.forEach(name -> {
+            NodeMap map = node.getMap(NodeFeatures.ELEMENT_PROPERTIES);
+            MapProperty mapProperty = map.getProperty(name);
+            Object domValue = WidgetUtil.getJsProperty(element, name);
+            mapProperty.setPreviousDomValue(domValue);
+        });
 
         JsMap<String, Runnable> commands = JsCollections.map();
         synchronizeProperties.forEach(name -> commands.set(name,

flow-client/src/main/java/com/vaadin/client/flow/nodefeature/MapProperty.java

@@ -17,6 +17,7 @@
 
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 
 import com.vaadin.client.flow.StateNode;
 import com.vaadin.client.flow.StateTree;
@@ -65,6 +66,7 @@ protected void dispatchEvent(MapPropertyChangeListener listener,
     private boolean hasValue = false;
 
     private final boolean forceValueUpdate;
+    private Optional<Object> previousDomValue = Optional.empty();
 
     /**
      * Creates a new property.
@@ -333,4 +335,34 @@ public Runnable getSyncToServerCommand(Object newValue) {
         }
         return NO_OP;
     }
+
+    /**
+     * Stores previous DOM value of this property for detection of value
+     * modification by the user during the server round-trip.
+     *
+     * @param previousDomValue
+     *            DOM value of property prior to server round-trip start. Can be
+     *            <code>null</code>;
+     */
+    public void setPreviousDomValue(Object previousDomValue) {
+        this.previousDomValue = Optional.ofNullable(previousDomValue);
+    }
+
+    /**
+     * Returns previous DOM value of this property for detection of value
+     * modification by the user during the server round-trip.
+     *
+     * @return Optional of previous DOM value. Empty optional is returned if
+     *         previous value has not been stored.
+     */
+    public Optional<Object> getPreviousDomValue() {
+        return previousDomValue;
+    }
+
+    /**
+     * Clears the previous DOM value of this property.
+     */
+    public void clearPreviousDomValue() {
+        this.previousDomValue = Optional.empty();
+    }
 }

flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/ClientSideValueChangeView.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2000-2024 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.flow.uitest.ui;
+
+import com.vaadin.flow.component.html.Input;
+import com.vaadin.flow.component.html.Span;
+import com.vaadin.flow.router.Route;
+import com.vaadin.flow.uitest.servlet.ViewTestLayout;
+
+@Route(value = "com.vaadin.flow.uitest.ui.ClientSideValueChangeView", layout = ViewTestLayout.class)
+public class ClientSideValueChangeView extends AbstractDivView {
+
+    @Override
+    protected void onShow() {
+        Input input = new Input();
+        input.setId("inputfield");
+
+        input.addValueChangeListener(e -> {
+            try {
+                Thread.sleep(1000);
+            } catch (InterruptedException ex) {
+                throw new RuntimeException(ex);
+            }
+            Span span = new Span("done");
+            span.setId("status");
+            add(span);
+        });
+
+        add(input);
+
+        Input input2 = new Input();
+        input2.setId("inputfieldserversetsvalue");
+
+        input2.addValueChangeListener(e -> {
+            try {
+                Thread.sleep(1000);
+            } catch (InterruptedException ex) {
+                throw new RuntimeException(ex);
+            }
+            input2.setValue("fromserver");
+            Span span = new Span("done");
+            span.setId("statusserversetsvalue");
+            add(span);
+        });
+
+        add(input2);
+    }
+
+}

flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/ClientSideValueChangeIT.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2000-2024 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.flow.uitest.ui;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.support.ui.ExpectedConditions;
+
+import com.vaadin.flow.component.html.testbench.InputTextElement;
+import com.vaadin.flow.testutil.ChromeBrowserTest;
+
+public class ClientSideValueChangeIT extends ChromeBrowserTest {
+
+    @Test
+    public void clientSideValueEntryDuringRoundTrip_enteredValueShouldNotBeOverridden() {
+        open();
+
+        getCommandExecutor().disableWaitForVaadin();
+
+        InputTextElement input = $(InputTextElement.class).id("inputfield");
+        input.setValue("abc");
+        input.sendKeys("123");
+
+        waitUntil(ExpectedConditions.presenceOfElementLocated(By.id("status")));
+
+        Assert.assertEquals(
+                "User input during round-trip was unexpectedly overridden",
+                "abc123",
+                $(InputTextElement.class).id("inputfield").getValue());
+    }
+
+    @Test
+    public void clientSideValueEntryDuringRoundTrip_serverChangesValue_serverValueShouldBeUsed() {
+        open();
+
+        getCommandExecutor().disableWaitForVaadin();
+
+        InputTextElement input = $(InputTextElement.class)
+                .id("inputfieldserversetsvalue");
+        input.setValue("abc");
+        input.sendKeys("123");
+
+        waitUntil(ExpectedConditions
+                .presenceOfElementLocated(By.id("statusserversetsvalue")));
+
+        Assert.assertEquals(
+                "Value set by server during round-trip was unexpectedly overridden",
+                "fromserver", $(InputTextElement.class)
+                        .id("inputfieldserversetsvalue").getValue());
+    }
+}