Skip to content

refactor: remove custom I18N merging logic #7971

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.

Sign up for GitHub

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

refactor: remove custom I18N merging logic #7971

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62e0e1c
refactor: remove custom I18N merging logic
sissbruecker Sep 5, 2025
2aed50b
Merge branch 'main' into refactor/use-partial-i18n
sissbruecker Sep 8, 2025
0dcdc15
simplify upload I18n IT
sissbruecker Sep 8, 2025
b58274d
Merge branch 'main' into refactor/use-partial-i18n
sissbruecker Sep 8, 2025
52a0f3b
Merge branch 'main' into refactor/use-partial-i18n
sissbruecker Sep 9, 2025
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
4 changes: 1 addition & 3 deletions ...flow-parent/vaadin-avatar-flow/src/main/java/com/vaadin/flow/component/avatar/Avatar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}

/**
Expand Down
18 changes: 1 addition & 17 deletions ...nt/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/Dashboard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 }"
Expand Down
3 changes: 1 addition & 2 deletions ...me-picker-flow/src/main/java/com/vaadin/flow/component/datetimepicker/DateTimePicker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,9 +947,8 @@ public DateTimePickerI18n getI18n() {
* the internationalized properties, not <code>null</code>
*/
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();
}

Expand Down
29 changes: 1 addition & 28 deletions ...-parent/vaadin-menu-bar-flow/src/main/java/com/vaadin/flow/component/menubar/MenuBar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions ...t/vaadin-messages-flow/src/main/java/com/vaadin/flow/component/messages/MessageInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
23 changes: 1 addition & 22 deletions ...xt-editor-flow/src/main/java/com/vaadin/flow/component/richtexteditor/RichTextEditor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<UI> command) {
Expand Down Expand Up @@ -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();
}
}

/**
Expand Down
150 changes: 3 additions & 147 deletions ...-integration-tests/src/test/java/com/vaadin/flow/component/upload/tests/UploadI18nIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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() {
Copy link
Contributor Author

@sissbruecker sissbruecker Sep 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests were checking that the nested I18N structure was properly merged by the executeJs call that has now been removed. The equivalent would now be to check the internal __effectiveI18n property.

However, since the merging logic is part of the web component, I think it's better to remove this. The web components could use some better test coverage for partial I18n, but that's a separate issue.

open();

UploadElement upload = $(UploadElement.class).id("upload-full-i18n");
JsonObject i18nJson = getUploadI18nPropertyAsJson(upload);
Map<String, String> translationMap = jsonToMap(i18nJson);

UploadI18N expected = UploadTestsI18N.RUSSIAN_FULL;
JsonObject expectedJson = (JsonObject) JsonSerializer.toJson(expected);
deeplyRemoveNullValuesFromJsonObject(expectedJson);
Map<String, String> expectedMap = jsonToMap(expectedJson);

assertTranslationMapsAreEqual(expectedMap, translationMap);
}

@Test
public void testPartialI18nShouldAffectLabels() {
public void setPartialI18n_mergesTranslationsWithDefaults() {
open();

UploadElement upload = $(UploadElement.class).id("upload-partial-i18n");
Expand All @@ -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<String, String> translationMap = jsonToMap(i18nJson);

UploadI18N fullTranslation = UploadTestsI18N.RUSSIAN_FULL;
JsonObject fullTranslationJson = (JsonObject) JsonSerializer
.toJson(fullTranslation);
deeplyRemoveNullValuesFromJsonObject(fullTranslationJson);
Map<String, String> 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"));
Expand All @@ -143,88 +83,4 @@ public void testDetachReattachI18nIsPreserved() {
UploadTestsI18N.RUSSIAN_FULL.getDropFiles().getOne(),
dropLabel.getText());
}

private void assertTranslationMapsAreEqual(Map<String, String> expected,
Map<String, String> 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<String, String> expected,
Map<String, String> 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<String, String> 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<String, String> jsonToMap(JsonObject jsonObject) {
return jsonToMap(new HashMap<>(), "", jsonObject);
}

private Map<String, String> jsonToMap(Map<String, String> 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);
}
}
}
}
Loading