From 62e0e1ccb1f789bdb5c581fa5d88e2f34ef0843e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Fri, 5 Sep 2025 18:16:44 +0200 Subject: [PATCH 1/2] refactor: remove custom I18N merging logic --- .../vaadin/flow/component/avatar/Avatar.java | 4 +- .../flow/component/dashboard/Dashboard.java | 18 +------- .../datetimepicker/DateTimePicker.java | 3 +- .../flow/component/menubar/MenuBar.java | 29 +------------ .../flow/component/messages/MessageInput.java | 4 +- .../richtexteditor/RichTextEditor.java | 23 +--------- .../vaadin/flow/component/upload/Upload.java | 42 +------------------ 7 files changed, 8 insertions(+), 115 deletions(-) diff --git a/vaadin-avatar-flow-parent/vaadin-avatar-flow/src/main/java/com/vaadin/flow/component/avatar/Avatar.java b/vaadin-avatar-flow-parent/vaadin-avatar-flow/src/main/java/com/vaadin/flow/component/avatar/Avatar.java index af61d706af9..6b09c9e8d01 100644 --- a/vaadin-avatar-flow-parent/vaadin-avatar-flow/src/main/java/com/vaadin/flow/component/avatar/Avatar.java +++ b/vaadin-avatar-flow-parent/vaadin-avatar-flow/src/main/java/com/vaadin/flow/component/avatar/Avatar.java @@ -19,7 +19,6 @@ import java.util.Objects; import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.databind.node.ObjectNode; import com.vaadin.flow.component.Component; import com.vaadin.flow.component.HasSize; import com.vaadin.flow.component.HasStyle; @@ -153,8 +152,7 @@ public AvatarI18n getI18n() { public void setI18n(AvatarI18n i18n) { this.i18n = Objects.requireNonNull(i18n, "The i18n properties object should not be null"); - ObjectNode i18nObject = JacksonUtils.beanToJson(i18n); - getElement().setPropertyJson("i18n", i18nObject); + getElement().setPropertyJson("i18n", JacksonUtils.beanToJson(i18n)); } /** diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/Dashboard.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/Dashboard.java index f17e39d082d..5abec5bd8d5 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/Dashboard.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/Dashboard.java @@ -25,7 +25,6 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.ObjectNode; import com.vaadin.flow.component.AttachEvent; import com.vaadin.flow.component.Component; import com.vaadin.flow.component.ComponentEventListener; @@ -487,12 +486,7 @@ public DashboardI18n getI18n() { public void setI18n(DashboardI18n i18n) { this.i18n = Objects.requireNonNull(i18n, "The i18n properties object should not be null"); - getElement().getNode().runWhenAttached( - ui -> ui.beforeClientResponse(this, context -> { - if (i18n.equals(this.i18n)) { - setI18nWithJS(); - } - })); + getElement().setPropertyJson("i18n", JacksonUtils.beanToJson(i18n)); } @Override @@ -620,16 +614,6 @@ private void updateClientItems() { flatOrderedComponents.toArray(Component[]::new)); } - private void setI18nWithJS() { - ObjectNode i18nJson = JacksonUtils.beanToJson(i18n); - - // Assign new I18N object to WC, by merging the existing - // WC I18N, and the values from the new DashboardI18n instance, - // into an empty object - getElement().executeJs("this.i18n = Object.assign({}, this.i18n, $0);", - i18nJson); - } - private static String getWidgetRepresentation(DashboardWidget widget, int itemIndex) { return "{ component: $%d, colspan: %d, rowspan: %d, id: %d }" diff --git a/vaadin-date-time-picker-flow-parent/vaadin-date-time-picker-flow/src/main/java/com/vaadin/flow/component/datetimepicker/DateTimePicker.java b/vaadin-date-time-picker-flow-parent/vaadin-date-time-picker-flow/src/main/java/com/vaadin/flow/component/datetimepicker/DateTimePicker.java index d2015be282e..4a92ebd701c 100644 --- a/vaadin-date-time-picker-flow-parent/vaadin-date-time-picker-flow/src/main/java/com/vaadin/flow/component/datetimepicker/DateTimePicker.java +++ b/vaadin-date-time-picker-flow-parent/vaadin-date-time-picker-flow/src/main/java/com/vaadin/flow/component/datetimepicker/DateTimePicker.java @@ -947,9 +947,8 @@ public DateTimePickerI18n getI18n() { * the internationalized properties, not null */ public void setI18n(DateTimePickerI18n i18n) { - Objects.requireNonNull(i18n, + this.i18n = Objects.requireNonNull(i18n, "The i18n properties object should not be null"); - this.i18n = i18n; updateI18n(); } diff --git a/vaadin-menu-bar-flow-parent/vaadin-menu-bar-flow/src/main/java/com/vaadin/flow/component/menubar/MenuBar.java b/vaadin-menu-bar-flow-parent/vaadin-menu-bar-flow/src/main/java/com/vaadin/flow/component/menubar/MenuBar.java index 27b41e1baa5..f255936557d 100644 --- a/vaadin-menu-bar-flow-parent/vaadin-menu-bar-flow/src/main/java/com/vaadin/flow/component/menubar/MenuBar.java +++ b/vaadin-menu-bar-flow-parent/vaadin-menu-bar-flow/src/main/java/com/vaadin/flow/component/menubar/MenuBar.java @@ -21,8 +21,6 @@ import java.util.stream.Stream; import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.databind.node.ObjectNode; -import com.vaadin.flow.component.AttachEvent; import com.vaadin.flow.component.ClickEvent; import com.vaadin.flow.component.Component; import com.vaadin.flow.component.ComponentEventListener; @@ -426,32 +424,7 @@ public MenuBarI18n getI18n() { public void setI18n(MenuBarI18n i18n) { this.i18n = Objects.requireNonNull(i18n, "The i18n properties object should not be null"); - - runBeforeClientResponse(ui -> { - if (i18n == this.i18n) { - setI18nWithJS(); - } - }); - } - - private void setI18nWithJS() { - ObjectNode i18nJson = JacksonUtils.beanToJson(i18n); - - // Assign new I18N object to WC, by merging the existing - // WC I18N, and the values from the new MenuBarI18n instance, - // into an empty object - getElement().executeJs("this.i18n = Object.assign({}, this.i18n, $0);", - i18nJson); - } - - @Override - protected void onAttach(AttachEvent attachEvent) { - super.onAttach(attachEvent); - - // Element state is not persisted across attach/detach - if (this.i18n != null) { - setI18nWithJS(); - } + getElement().setPropertyJson("i18n", JacksonUtils.beanToJson(i18n)); } void resetContent() { diff --git a/vaadin-messages-flow-parent/vaadin-messages-flow/src/main/java/com/vaadin/flow/component/messages/MessageInput.java b/vaadin-messages-flow-parent/vaadin-messages-flow/src/main/java/com/vaadin/flow/component/messages/MessageInput.java index a2a8ce318e3..0d3be3765ee 100644 --- a/vaadin-messages-flow-parent/vaadin-messages-flow/src/main/java/com/vaadin/flow/component/messages/MessageInput.java +++ b/vaadin-messages-flow-parent/vaadin-messages-flow/src/main/java/com/vaadin/flow/component/messages/MessageInput.java @@ -147,8 +147,8 @@ public MessageInputI18n getI18n() { * the i18n object, not {@code null} */ public void setI18n(MessageInputI18n i18n) { - Objects.requireNonNull(i18n, "The i18n object should not be null"); - this.i18n = i18n; + this.i18n = Objects.requireNonNull(i18n, + "The i18n object should not be null"); getElement().setPropertyJson("i18n", JacksonUtils.beanToJson(i18n)); } } diff --git a/vaadin-rich-text-editor-flow-parent/vaadin-rich-text-editor-flow/src/main/java/com/vaadin/flow/component/richtexteditor/RichTextEditor.java b/vaadin-rich-text-editor-flow-parent/vaadin-rich-text-editor-flow/src/main/java/com/vaadin/flow/component/richtexteditor/RichTextEditor.java index f14cc4cfa5e..3aae9fc1444 100644 --- a/vaadin-rich-text-editor-flow-parent/vaadin-rich-text-editor-flow/src/main/java/com/vaadin/flow/component/richtexteditor/RichTextEditor.java +++ b/vaadin-rich-text-editor-flow-parent/vaadin-rich-text-editor-flow/src/main/java/com/vaadin/flow/component/richtexteditor/RichTextEditor.java @@ -17,7 +17,6 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.ObjectNode; import com.vaadin.flow.component.AbstractSinglePropertyField; import com.vaadin.flow.component.AttachEvent; import com.vaadin.flow.component.ComponentEvent; @@ -97,22 +96,7 @@ public RichTextEditorI18n getI18n() { public void setI18n(RichTextEditorI18n i18n) { this.i18n = Objects.requireNonNull(i18n, "The i18n properties object should not be null"); - - runBeforeClientResponse(ui -> { - if (i18n == this.i18n) { - setI18nWithJS(); - } - }); - } - - private void setI18nWithJS() { - ObjectNode i18nJson = JacksonUtils.beanToJson(i18n); - - // Assign new I18N object to WC, by merging the existing - // WC I18N, and the values from the new RichTextEditorI18n instance, - // into an empty object - getElement().executeJs("this.i18n = Object.assign({}, this.i18n, $0);", - i18nJson); + getElement().setPropertyJson("i18n", JacksonUtils.beanToJson(i18n)); } void runBeforeClientResponse(SerializableConsumer command) { @@ -154,11 +138,6 @@ protected void onAttach(AttachEvent attachEvent) { // presentation value to run the necessary JS for initializing the // client-side element setPresentationValue(getValue()); - - // Element state is not persisted across attach/detach - if (this.i18n != null) { - setI18nWithJS(); - } } /** diff --git a/vaadin-upload-flow-parent/vaadin-upload-flow/src/main/java/com/vaadin/flow/component/upload/Upload.java b/vaadin-upload-flow-parent/vaadin-upload-flow/src/main/java/com/vaadin/flow/component/upload/Upload.java index 337927713b8..7302a642414 100644 --- a/vaadin-upload-flow-parent/vaadin-upload-flow/src/main/java/com/vaadin/flow/component/upload/Upload.java +++ b/vaadin-upload-flow-parent/vaadin-upload-flow/src/main/java/com/vaadin/flow/component/upload/Upload.java @@ -26,8 +26,6 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.ObjectNode; -import com.vaadin.flow.component.AttachEvent; import com.vaadin.flow.component.Component; import com.vaadin.flow.component.ComponentEventListener; import com.vaadin.flow.component.HasEnabled; @@ -756,45 +754,7 @@ private boolean isMultiFileReceiver(Receiver receiver) { public void setI18n(UploadI18N i18n) { this.i18n = Objects.requireNonNull(i18n, "The i18n properties object should not be null"); - - runBeforeClientResponse(ui -> { - if (i18n == this.i18n) { - setI18nWithJS(); - } - }); - } - - private void setI18nWithJS() { - ObjectNode i18nJson = JacksonUtils.beanToJson(this.i18n); - - // Assign new I18N object to WC, by deeply merging the existing - // WC I18N, and the values from the new UploadI18N instance, - // into an empty object - getElement().executeJs( - "const dropFiles = Object.assign({}, this.i18n.dropFiles, $0.dropFiles);" - + "const addFiles = Object.assign({}, this.i18n.addFiles, $0.addFiles);" - + "const error = Object.assign({}, this.i18n.error, $0.error);" - + "const uploadingStatus = Object.assign({}, this.i18n.uploading.status, $0.uploading && $0.uploading.status);" - + "const uploadingRemainingTime = Object.assign({}, this.i18n.uploading.remainingTime, $0.uploading && $0.uploading.remainingTime);" - + "const uploadingError = Object.assign({}, this.i18n.uploading.error, $0.uploading && $0.uploading.error);" - + "const uploading = {status: uploadingStatus," - + " remainingTime: uploadingRemainingTime," - + " error: uploadingError};" - + "const units = $0.units || this.i18n.units;" - + "this.i18n = Object.assign({}, this.i18n, $0, {" - + " addFiles: addFiles, dropFiles: dropFiles," - + " uploading: uploading, units: units});", - i18nJson); - } - - @Override - protected void onAttach(AttachEvent attachEvent) { - super.onAttach(attachEvent); - - // Element state is not persisted across attach/detach - if (this.i18n != null) { - setI18nWithJS(); - } + getElement().setPropertyJson("i18n", JacksonUtils.beanToJson(i18n)); } void runBeforeClientResponse(SerializableConsumer command) { From 0dcdc152fd218ea176fc01ce36b4f708afcdb12e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Mon, 8 Sep 2025 16:25:48 +0200 Subject: [PATCH 2/2] simplify upload I18n IT --- .../component/upload/tests/UploadI18nIT.java | 150 +----------------- 1 file changed, 3 insertions(+), 147 deletions(-) diff --git a/vaadin-upload-flow-parent/vaadin-upload-flow-integration-tests/src/test/java/com/vaadin/flow/component/upload/tests/UploadI18nIT.java b/vaadin-upload-flow-parent/vaadin-upload-flow-integration-tests/src/test/java/com/vaadin/flow/component/upload/tests/UploadI18nIT.java index 60eb24ebd91..5c9d418e2a1 100644 --- a/vaadin-upload-flow-parent/vaadin-upload-flow-integration-tests/src/test/java/com/vaadin/flow/component/upload/tests/UploadI18nIT.java +++ b/vaadin-upload-flow-parent/vaadin-upload-flow-integration-tests/src/test/java/com/vaadin/flow/component/upload/tests/UploadI18nIT.java @@ -15,28 +15,18 @@ */ package com.vaadin.flow.component.upload.tests; -import java.util.HashMap; -import java.util.Map; - import org.junit.Assert; import org.junit.Test; import org.openqa.selenium.By; import org.openqa.selenium.WebElement; -import com.vaadin.flow.component.upload.UploadI18N; import com.vaadin.flow.component.upload.testbench.UploadElement; -import com.vaadin.flow.internal.JsonSerializer; import com.vaadin.flow.testutil.TestPath; -import elemental.json.Json; -import elemental.json.JsonObject; -import elemental.json.JsonType; -import elemental.json.JsonValue; - @TestPath("vaadin-upload/i18n") public class UploadI18nIT extends AbstractUploadIT { @Test - public void testFullI18nShouldAffectLabels() { + public void setFullI18n_updatesTranslations() { open(); UploadElement upload = $(UploadElement.class).id("upload-full-i18n"); @@ -52,31 +42,8 @@ public void testFullI18nShouldAffectLabels() { dropLabel.getText()); } - /** - * Verifies that every single translation provided by the UploadI18N - * instance is set in the web component. - * - * Testing internals here, in favour of setting up the web component with - * files/event handlers for every possible state. - */ @Test - public void testFullI18nShouldOverrideCompleteConfigurationInWebComponentProperty() { - open(); - - UploadElement upload = $(UploadElement.class).id("upload-full-i18n"); - JsonObject i18nJson = getUploadI18nPropertyAsJson(upload); - Map translationMap = jsonToMap(i18nJson); - - UploadI18N expected = UploadTestsI18N.RUSSIAN_FULL; - JsonObject expectedJson = (JsonObject) JsonSerializer.toJson(expected); - deeplyRemoveNullValuesFromJsonObject(expectedJson); - Map expectedMap = jsonToMap(expectedJson); - - assertTranslationMapsAreEqual(expectedMap, translationMap); - } - - @Test - public void testPartialI18nShouldAffectLabels() { + public void setPartialI18n_mergesTranslationsWithDefaults() { open(); UploadElement upload = $(UploadElement.class).id("upload-partial-i18n"); @@ -93,35 +60,8 @@ public void testPartialI18nShouldAffectLabels() { dropLabel.getText()); } - /** - * Verifies that setting a partial UploadI18N configuration still results in - * a complete configuration in the web component, and that null values in - * the UploadI18N configuration are ignored. - * - * Testing internals here, in favour of setting up the web component with - * files/event handlers for every possible state. - */ @Test - public void testPartialI18nShouldSetFullConfigurationWithoutNullValuesInWebComponentProperty() { - open(); - - UploadElement upload = $(UploadElement.class).id("upload-partial-i18n"); - JsonObject i18nJson = getUploadI18nPropertyAsJson(upload); - Map translationMap = jsonToMap(i18nJson); - - UploadI18N fullTranslation = UploadTestsI18N.RUSSIAN_FULL; - JsonObject fullTranslationJson = (JsonObject) JsonSerializer - .toJson(fullTranslation); - deeplyRemoveNullValuesFromJsonObject(fullTranslationJson); - Map fullTranslationMap = jsonToMap(fullTranslationJson); - UploadTestsI18N.OPTIONAL_KEYS.forEach(fullTranslationMap::remove); - - assertTranslationMapsHaveSameKeys(fullTranslationMap, translationMap); - assertTranslationMapHasNoMissingTranslations(translationMap); - } - - @Test - public void testDetachReattachI18nIsPreserved() { + public void setI18n_detach_reattach_i18nPreserved() { open(); WebElement btnSetI18n = findElement(By.id("btn-set-i18n")); @@ -143,88 +83,4 @@ public void testDetachReattachI18nIsPreserved() { UploadTestsI18N.RUSSIAN_FULL.getDropFiles().getOne(), dropLabel.getText()); } - - private void assertTranslationMapsAreEqual(Map expected, - Map actual) { - expected.keySet().forEach(expectedKey -> { - Assert.assertTrue("Missing translation key: " + expectedKey, - actual.containsKey(expectedKey)); - String expectedValue = expected.get(expectedKey); - String actualValue = actual.get(expectedKey); - Assert.assertEquals( - String.format("Mismatching translation: %s!=%s", - expectedValue, actualValue), - expectedValue, actualValue); - }); - } - - private void assertTranslationMapsHaveSameKeys(Map expected, - Map actual) { - expected.keySet().forEach(expectedKey -> { - // Cancel was removed in - // https://github.com/vaadin/web-components/pull/2723 - if (!"cancel".equals(expectedKey)) { - Assert.assertTrue("Missing translation key: " + expectedKey, - actual.containsKey(expectedKey)); - } - }); - } - - private void assertTranslationMapHasNoMissingTranslations( - Map map) { - map.keySet().forEach(key -> { - // Cancel was removed in - // https://github.com/vaadin/web-components/pull/2723 - if (!"cancel".equals(key)) { - String value = map.get(key); - Assert.assertNotNull("Missing translation value: " + key, - value); - } - }); - } - - /** - * Converts a deeply nested JsonObject into a Map of key / value pairs, - * where the key is the path through the object to the property, and the - * value is the string value of the property, or null if the property was - * null - */ - private Map jsonToMap(JsonObject jsonObject) { - return jsonToMap(new HashMap<>(), "", jsonObject); - } - - private Map jsonToMap(Map output, - String path, JsonObject node) { - for (String key : node.keys()) { - JsonValue jsonValue = node.get(key); - String subPath = path.isEmpty() ? key : path + "." + key; - - if (jsonValue.getType() == JsonType.OBJECT) { - jsonToMap(output, subPath, (JsonObject) jsonValue); - } else if (jsonValue.getType() == JsonType.NULL) { - output.put(subPath, null); - } else { - String stringValue = jsonValue.asString(); - output.put(subPath, stringValue); - } - } - return output; - } - - private JsonObject getUploadI18nPropertyAsJson(UploadElement upload) { - String i18nJsonString = (String) upload.getCommandExecutor() - .executeScript("return JSON.stringify(arguments[0].i18n)", - upload); - return Json.parse(i18nJsonString); - } - - private void deeplyRemoveNullValuesFromJsonObject(JsonObject jsonObject) { - for (String key : jsonObject.keys()) { - if (jsonObject.get(key).getType() == JsonType.OBJECT) { - deeplyRemoveNullValuesFromJsonObject(jsonObject.get(key)); - } else if (jsonObject.get(key).getType() == JsonType.NULL) { - jsonObject.remove(key); - } - } - } }