From 0ce030d61f3c59ab551c454275d27964282caa5e Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Fri, 23 Aug 2024 13:13:40 +0300 Subject: [PATCH 01/21] feat: introduce add and remove widget functionality to dashboard --- .../dashboard/tests/DashboardPage.java | 41 +++- .../dashboard/tests/DashboardIT.java | 50 +++- .../vaadin-dashboard-flow/pom.xml | 5 + .../flow/component/dashboard/Dashboard.java | 218 +++++++++++++++++- .../dashboard/tests/DashboardTest.java | 87 ++++++- .../dashboard/testbench/DashboardElement.java | 13 +- 6 files changed, 409 insertions(+), 5 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java index c18f2554312..7edf22ecd96 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java @@ -1,4 +1,4 @@ -/** +/* * Copyright 2000-2024 Vaadin Ltd. * * This program is available under Vaadin Commercial License and Service Terms. @@ -8,7 +8,10 @@ */ package com.vaadin.flow.component.dashboard.tests; +import com.vaadin.flow.component.dashboard.Dashboard; +import com.vaadin.flow.component.dashboard.DashboardWidget; import com.vaadin.flow.component.html.Div; +import com.vaadin.flow.component.html.NativeButton; import com.vaadin.flow.router.Route; /** @@ -16,4 +19,40 @@ */ @Route("vaadin-dashboard") public class DashboardPage extends Div { + + public DashboardPage() { + DashboardWidget widget1 = new DashboardWidget(); + widget1.setTitle("Widget 1"); + widget1.setId("widget-1"); + + DashboardWidget widget2 = new DashboardWidget(); + widget2.setTitle("Widget 2"); + widget2.setId("widget-2"); + + DashboardWidget widget3 = new DashboardWidget(); + widget3.setTitle("Widget 3"); + widget3.setId("widget-3"); + + Dashboard dashboard = new Dashboard(); + dashboard.add(widget1, widget2, widget3); + + NativeButton addWidgetAtIndex1 = new NativeButton("Add widget at index 1"); + addWidgetAtIndex1.addClickListener(click -> { + DashboardWidget widgetAtIndex1 = new DashboardWidget(); + widgetAtIndex1.setTitle("Widget at index 1"); + widgetAtIndex1.setId("widget-at-index-1"); + dashboard.addWidgetAtIndex(1, widgetAtIndex1); + }); + addWidgetAtIndex1.setId("add-widget-at-index-1"); + + NativeButton removeWidgets1And3 = new NativeButton("Remove widgets 1 and 3"); + removeWidgets1And3.addClickListener(click -> dashboard.remove(widget1, widget3)); + removeWidgets1And3.setId("remove-widgets-1-and-3"); + + NativeButton removeAllWidgets = new NativeButton("Remove all widgets"); + removeAllWidgets.addClickListener(click -> dashboard.removeAll()); + removeAllWidgets.setId("remove-all-widgets"); + + add(addWidgetAtIndex1, removeWidgets1And3, removeAllWidgets, dashboard); + } } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java index d97c9463a62..bb0c294113a 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java @@ -1,4 +1,4 @@ -/** +/* * Copyright 2000-2024 Vaadin Ltd. * * This program is available under Vaadin Commercial License and Service Terms. @@ -8,6 +8,15 @@ */ package com.vaadin.flow.component.dashboard.tests; +import java.util.Arrays; +import java.util.List; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.vaadin.flow.component.dashboard.testbench.DashboardElement; +import com.vaadin.flow.component.dashboard.testbench.DashboardWidgetElement; import com.vaadin.flow.testutil.TestPath; import com.vaadin.tests.AbstractComponentIT; @@ -16,4 +25,43 @@ */ @TestPath("vaadin-dashboard") public class DashboardIT extends AbstractComponentIT { + + private DashboardElement dashboardElement; + + @Before + public void init() { + open(); + dashboardElement = $(DashboardElement.class).waitForFirst(); + } + + @Test + public void addWidgets_widgetsAreCorrectlyAdded() { + assertWidgetsByTitle("Widget 1", "Widget 2", "Widget 3"); + } + + @Test + public void addWidgetsAtIndex1_widgetIsAddedIntoTheCorrectPlace() { + $("button").id("add-widget-at-index-1").click(); + assertWidgetsByTitle("Widget 1", "Widget at index 1", "Widget 2", "Widget 3"); + } + + @Test + public void removeWidgets1And3_widgetsAreCorrectlyRemoved() { + $("button").id("remove-widgets-1-and-3").click(); + assertWidgetsByTitle("Widget 2"); + } + + @Test + public void removeAllWidgets_widgetsAreCorrectlyRemoved() { + $("button").id("remove-all-widgets").click(); + assertWidgetsByTitle(); + } + + private void assertWidgetsByTitle(String... expectedWidgetTitles) { + List widgets = dashboardElement.getWidgets(); + List widgetTitles = widgets.stream() + .map(DashboardWidgetElement::getTitle).toList(); + Assert.assertEquals(Arrays.asList(expectedWidgetTitles), + widgetTitles); + } } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/pom.xml b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/pom.xml index aac38f7d4d4..54f333ae89e 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/pom.xml +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/pom.xml @@ -54,6 +54,11 @@ com.vaadin flow-html-components + + com.vaadin + vaadin-renderer-flow + ${project.version} + 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 9ac14d04e14..c4b9a0b2f5f 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 @@ -1,4 +1,4 @@ -/** +/* * Copyright 2000-2024 Vaadin Ltd. * * This program is available under Vaadin Commercial License and Service Terms. @@ -8,10 +8,27 @@ */ package com.vaadin.flow.component.dashboard; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +import org.slf4j.LoggerFactory; + +import com.vaadin.flow.component.AttachEvent; import com.vaadin.flow.component.Component; import com.vaadin.flow.component.Tag; import com.vaadin.flow.component.dependency.JsModule; import com.vaadin.flow.component.dependency.NpmPackage; +import com.vaadin.flow.dom.ElementDetachEvent; +import com.vaadin.flow.dom.ElementDetachListener; +import com.vaadin.flow.shared.Registration; + +import elemental.json.Json; +import elemental.json.JsonArray; +import elemental.json.JsonObject; /** * @author Vaadin Ltd @@ -22,4 +39,203 @@ @JsModule("@vaadin/dashboard/src/vaadin-dashboard.js") // @NpmPackage(value = "@vaadin/dashboard", version = "24.6.0-alpha0") public class Dashboard extends Component { + + private final List widgets = new ArrayList<>(); + + private boolean pendingUpdate = false; + + /** + * Creates an empty dashboard. + */ + public Dashboard() { + getElement().getNode().addAttachListener(this::attachRenderer); + } + + /** + * Returns the widgets in the dashboard. + * + * @return The widgets in the dashboard + */ + public List getWidgets() { + return Collections.unmodifiableList(widgets); + } + + /** + * Adds the given widgets to the dashboard. + * + * @param widgets + * the widgets to add, not {@code null} + */ + public void add(DashboardWidget... widgets) { + Objects.requireNonNull(widgets, "Widgets to add cannot be null."); + List toAdd = new ArrayList<>(widgets.length); + for (DashboardWidget widget : widgets) { + Objects.requireNonNull(widget, "Widget to add cannot be null."); + toAdd.add(widget); + } + toAdd.forEach(this::doAddWidget); + updateClient(); + } + + /** + * Adds the given widget as child of this dashboard at the specific index. + *

+ * In case the specified widget has already been added to another parent, it + * will be removed from there and added to this one. + * + * @param index + * the index, where the widget will be added. The index must be + * non-negative and may not exceed the children count + * @param widget + * the widget to add, not {@code null} + */ + public void addWidgetAtIndex(int index, DashboardWidget widget) { + Objects.requireNonNull(widget, "Widget to add cannot be null."); + if (index < 0) { + throw new IllegalArgumentException( + "Cannot add a widget with a negative index."); + } + // The case when the index is bigger than the children count is handled + // inside the method below + doAddWidget(index, widget); + updateClient(); + } + + /** + * Removes the given widgets from this dashboard. + * + * @param widgets + * the widgets to remove, not {@code null} + * @throws IllegalArgumentException + * if there is a widget whose non {@code null} parent is not + * this dashboard + */ + public void remove(DashboardWidget... widgets) { + Objects.requireNonNull(widgets, "Widgets to remove cannot be null."); + List toRemove = new ArrayList<>(widgets.length); + for (DashboardWidget widget : widgets) { + Objects.requireNonNull(widget, "Widget to remove cannot be null."); + Element parent = widget.getElement().getParent(); + if (parent == null) { + LoggerFactory.getLogger(getClass()).debug( + "Removal of a widget with no parent does nothing."); + continue; + } + if (getElement().equals(parent)) { + toRemove.add(widget); + } else { + throw new IllegalArgumentException("The given widget (" + widget + + ") is not a child of this dashboard"); + } + } + toRemove.forEach(this::doRemoveWidget); + updateClient(); + } + + /** + * Removes all widgets from this dashboard. + */ + public void removeAll() { + doRemoveAllWidgets(); + updateClient(); + */ + + private final Map childDetachListenerMap = new HashMap<>(); + + // Must not use lambda here as that would break serialization. See + // https://github.com/vaadin/flow-components/issues/5597 + private final ElementDetachListener childDetachListener = new ElementDetachListener() { + @Override + public void onDetach(ElementDetachEvent e) { + var detachedElement = e.getSource(); + getWidgets().stream() + .filter(widget -> Objects.equals(detachedElement, + widget.getElement())) + .findAny().ifPresent(detachedWidget -> { + // The child was removed from the dashboard + + // Remove the registration for the child detach listener + childDetachListenerMap.get(detachedWidget.getElement()) + .remove(); + childDetachListenerMap + .remove(detachedWidget.getElement()); + + doRemoveWidget(detachedWidget); + updateClient(); + }); + } + }; + + @Override + protected void onAttach(AttachEvent attachEvent) { + super.onAttach(attachEvent); + updateClient(); + } + + private void updateClient() { + if (pendingUpdate) { + return; + } + pendingUpdate = true; + getElement().getNode() + .runWhenAttached(ui -> ui.beforeClientResponse(this, ctx -> { + doUpdateClient(); + pendingUpdate = false; + })); + } + + private void doUpdateClient() { + getElement().getChildren().forEach(child -> { + if (!childDetachListenerMap.containsKey(child)) { + childDetachListenerMap.put(child, + child.addDetachListener(childDetachListener)); + } + }); + List widgetNodeIds = getWidgets().stream() + .map(this::getWidgetNodeId).toList(); + getElement().setPropertyList("virtualChildNodeIds", widgetNodeIds); + getElement().setPropertyJson("items", createItemsJsonArray()); + } + + private void attachRenderer() { + getElement().executeJs( + "Vaadin.FlowComponentHost.patchVirtualContainer(this);"); + String appId = UI.getCurrent().getInternals().getAppId(); + getElement().executeJs( + "this.renderer = (root, _, model) => Vaadin.FlowComponentHost.setChildNodes($0, [model.item.nodeid], root);", + appId); + } + + private JsonArray createItemsJsonArray() { + JsonArray jsonItems = Json.createArray(); + for (DashboardWidget widget : widgets) { + JsonObject jsonItem = Json.createObject(); + */ + jsonItems.set(jsonItems.length(), jsonItem); + } + return jsonItems; + } + + private int getWidgetNodeId(DashboardWidget widget) { + return widget.getElement().getNode().getId(); + } + + private void doRemoveAllWidgets() { + widgets.clear(); + getElement().removeAllChildren(); + } + + private void doRemoveWidget(DashboardWidget widget) { + widgets.remove(widget); + getElement().removeChild(widget.getElement()); + } + + private void doAddWidget(int index, DashboardWidget widget) { + widgets.add(index, widget); + getElement().insertChild(index, widget.getElement()); + } + + private void doAddWidget(DashboardWidget widget) { + widgets.add(widget); + getElement().appendChild(widget.getElement()); } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java index 883a15383fb..02ffbbd964b 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java @@ -1,4 +1,4 @@ -/** +/* * Copyright 2000-2024 Vaadin Ltd. * * This program is available under Vaadin Commercial License and Service Terms. @@ -8,5 +8,90 @@ */ package com.vaadin.flow.component.dashboard.tests; +import java.util.Collections; +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; + +import com.vaadin.flow.component.dashboard.Dashboard; +import com.vaadin.flow.component.dashboard.DashboardWidget; + public class DashboardTest { + + @Test + public void addWidget_widgetIsAdded() { + Dashboard dashboard = new Dashboard(); + DashboardWidget widget1 = new DashboardWidget(); + DashboardWidget widget2 = new DashboardWidget(); + dashboard.add(widget1, widget2); + + Assert.assertEquals(List.of(widget1, widget2), dashboard.getWidgets()); + } + + @Test + public void addNullWidget_exceptionIsThrown() { + Dashboard dashboard = new Dashboard(); + Assert.assertThrows(NullPointerException.class, () -> dashboard.add((DashboardWidget) null)); + } + + @Test + public void addNullWidgetInArray_noWidgetIsAdded() { + Dashboard dashboard = new Dashboard(); + DashboardWidget widget = new DashboardWidget(); + try { + dashboard.add(widget, null); + } catch (NullPointerException e) { + // Do nothing + } + Assert.assertEquals(Collections.emptyList(), dashboard.getWidgets()); + } + + @Test + public void addWidgetAtIndex_widgetIsCorrectlyAdded() { + Dashboard dashboard = new Dashboard(); + DashboardWidget widget1 = new DashboardWidget(); + DashboardWidget widget2 = new DashboardWidget(); + DashboardWidget widget3 = new DashboardWidget(); + dashboard.add(widget1, widget2); + dashboard.addWidgetAtIndex(1, widget3); + + Assert.assertEquals(List.of(widget1, widget3, widget2), + dashboard.getWidgets()); + } + + @Test + public void addNullWidgetAtIndex_exceptionIsThrown() { + Dashboard dashboard = new Dashboard(); + Assert.assertThrows(NullPointerException.class, () -> dashboard.addWidgetAtIndex(0, null)); + } + + @Test + public void removeWidget_widgetIsRemoved() { + Dashboard dashboard = new Dashboard(); + DashboardWidget widget1 = new DashboardWidget(); + DashboardWidget widget2 = new DashboardWidget(); + dashboard.add(widget1, widget2); + dashboard.remove(widget1); + + Assert.assertEquals(List.of(widget2), dashboard.getWidgets()); + } + + @Test + public void removeNullWidget_exceptionIsThrown() { + Dashboard dashboard = new Dashboard(); + Assert.assertThrows(NullPointerException.class, () -> dashboard.remove((DashboardWidget) null)); + + } + + @Test + public void removeAllWidgets_widgetsAreRemoved() { + Dashboard dashboard = new Dashboard(); + DashboardWidget widget1 = new DashboardWidget(); + DashboardWidget widget2 = new DashboardWidget(); + dashboard.add(widget1, widget2); + dashboard.removeAll(); + + Assert.assertEquals(Collections.emptyList(), dashboard.getWidgets()); + } } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java index 3f86d9d7ba4..1cdff480828 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java @@ -1,4 +1,4 @@ -/** +/* * Copyright 2000-2024 Vaadin Ltd. * * This program is available under Vaadin Commercial License and Service Terms. @@ -8,6 +8,8 @@ */ package com.vaadin.flow.component.dashboard.testbench; +import java.util.List; + import com.vaadin.testbench.TestBenchElement; import com.vaadin.testbench.elementsbase.Element; @@ -16,4 +18,13 @@ */ @Element("vaadin-dashboard") public class DashboardElement extends TestBenchElement { + + /** + * Returns the widgets in the dashboard. + * + * @return The widgets in the dashboard + */ + public List getWidgets() { + return $(DashboardWidgetElement.class).all(); + } } From 42ac8c8329a5044fb75b5d281094298bfa6a9db0 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Fri, 23 Aug 2024 13:25:24 +0300 Subject: [PATCH 02/21] fix: add missing lines and run formatter --- .../component/dashboard/tests/DashboardPage.java | 9 ++++++--- .../flow/component/dashboard/tests/DashboardIT.java | 6 +++--- .../vaadin/flow/component/dashboard/Dashboard.java | 6 +++++- .../component/dashboard/tests/DashboardTest.java | 9 ++++++--- .../dashboard/testbench/DashboardElement.java | 13 ++++++++++--- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java index 7edf22ecd96..441ea53d80f 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java @@ -36,7 +36,8 @@ public DashboardPage() { Dashboard dashboard = new Dashboard(); dashboard.add(widget1, widget2, widget3); - NativeButton addWidgetAtIndex1 = new NativeButton("Add widget at index 1"); + NativeButton addWidgetAtIndex1 = new NativeButton( + "Add widget at index 1"); addWidgetAtIndex1.addClickListener(click -> { DashboardWidget widgetAtIndex1 = new DashboardWidget(); widgetAtIndex1.setTitle("Widget at index 1"); @@ -45,8 +46,10 @@ public DashboardPage() { }); addWidgetAtIndex1.setId("add-widget-at-index-1"); - NativeButton removeWidgets1And3 = new NativeButton("Remove widgets 1 and 3"); - removeWidgets1And3.addClickListener(click -> dashboard.remove(widget1, widget3)); + NativeButton removeWidgets1And3 = new NativeButton( + "Remove widgets 1 and 3"); + removeWidgets1And3 + .addClickListener(click -> dashboard.remove(widget1, widget3)); removeWidgets1And3.setId("remove-widgets-1-and-3"); NativeButton removeAllWidgets = new NativeButton("Remove all widgets"); diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java index bb0c294113a..ee58c1deb56 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java @@ -42,7 +42,8 @@ public void addWidgets_widgetsAreCorrectlyAdded() { @Test public void addWidgetsAtIndex1_widgetIsAddedIntoTheCorrectPlace() { $("button").id("add-widget-at-index-1").click(); - assertWidgetsByTitle("Widget 1", "Widget at index 1", "Widget 2", "Widget 3"); + assertWidgetsByTitle("Widget 1", "Widget at index 1", "Widget 2", + "Widget 3"); } @Test @@ -61,7 +62,6 @@ private void assertWidgetsByTitle(String... expectedWidgetTitles) { List widgets = dashboardElement.getWidgets(); List widgetTitles = widgets.stream() .map(DashboardWidgetElement::getTitle).toList(); - Assert.assertEquals(Arrays.asList(expectedWidgetTitles), - widgetTitles); + Assert.assertEquals(Arrays.asList(expectedWidgetTitles), widgetTitles); } } 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 c4b9a0b2f5f..bef0c117f6b 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 @@ -20,8 +20,10 @@ import com.vaadin.flow.component.AttachEvent; import com.vaadin.flow.component.Component; import com.vaadin.flow.component.Tag; +import com.vaadin.flow.component.UI; import com.vaadin.flow.component.dependency.JsModule; import com.vaadin.flow.component.dependency.NpmPackage; +import com.vaadin.flow.dom.Element; import com.vaadin.flow.dom.ElementDetachEvent; import com.vaadin.flow.dom.ElementDetachListener; import com.vaadin.flow.shared.Registration; @@ -37,6 +39,7 @@ @NpmPackage(value = "@vaadin/polymer-legacy-adapter", version = "24.5.0-alpha8") @JsModule("@vaadin/polymer-legacy-adapter/style-modules.js") @JsModule("@vaadin/dashboard/src/vaadin-dashboard.js") +@JsModule("./flow-component-renderer.js") // @NpmPackage(value = "@vaadin/dashboard", version = "24.6.0-alpha0") public class Dashboard extends Component { @@ -210,7 +213,7 @@ private JsonArray createItemsJsonArray() { JsonArray jsonItems = Json.createArray(); for (DashboardWidget widget : widgets) { JsonObject jsonItem = Json.createObject(); - */ + jsonItem.put("nodeid", getWidgetNodeId(widget)); jsonItems.set(jsonItems.length(), jsonItem); } return jsonItems; @@ -238,4 +241,5 @@ private void doAddWidget(int index, DashboardWidget widget) { private void doAddWidget(DashboardWidget widget) { widgets.add(widget); getElement().appendChild(widget.getElement()); + } } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java index 02ffbbd964b..32302fb1a60 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java @@ -32,7 +32,8 @@ public void addWidget_widgetIsAdded() { @Test public void addNullWidget_exceptionIsThrown() { Dashboard dashboard = new Dashboard(); - Assert.assertThrows(NullPointerException.class, () -> dashboard.add((DashboardWidget) null)); + Assert.assertThrows(NullPointerException.class, + () -> dashboard.add((DashboardWidget) null)); } @Test @@ -63,7 +64,8 @@ public void addWidgetAtIndex_widgetIsCorrectlyAdded() { @Test public void addNullWidgetAtIndex_exceptionIsThrown() { Dashboard dashboard = new Dashboard(); - Assert.assertThrows(NullPointerException.class, () -> dashboard.addWidgetAtIndex(0, null)); + Assert.assertThrows(NullPointerException.class, + () -> dashboard.addWidgetAtIndex(0, null)); } @Test @@ -80,7 +82,8 @@ public void removeWidget_widgetIsRemoved() { @Test public void removeNullWidget_exceptionIsThrown() { Dashboard dashboard = new Dashboard(); - Assert.assertThrows(NullPointerException.class, () -> dashboard.remove((DashboardWidget) null)); + Assert.assertThrows(NullPointerException.class, + () -> dashboard.remove((DashboardWidget) null)); } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java index 1cdff480828..6b6870af58c 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java @@ -1,10 +1,17 @@ /* * Copyright 2000-2024 Vaadin Ltd. * - * This program is available under Vaadin Commercial License and Service Terms. + * 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 * - * See {@literal } for the full - * license. + * 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.component.dashboard.testbench; From 8c7be9bb52fb33d7fb295eb753d94fab4a526844 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Fri, 23 Aug 2024 13:51:15 +0300 Subject: [PATCH 03/21] fix: fix compilation error --- .../java/com/vaadin/flow/component/dashboard/Dashboard.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 bef0c117f6b..3696eac71e0 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 @@ -141,7 +141,7 @@ public void remove(DashboardWidget... widgets) { public void removeAll() { doRemoveAllWidgets(); updateClient(); - */ + } private final Map childDetachListenerMap = new HashMap<>(); From b050a2fcb91fa79337209153b5945a01723098a3 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Fri, 23 Aug 2024 14:48:00 +0300 Subject: [PATCH 04/21] chore: run formatter --- .../component/dashboard/tests/DashboardPage.java | 2 +- .../component/dashboard/tests/DashboardIT.java | 2 +- .../flow/component/dashboard/Dashboard.java | 2 +- .../component/dashboard/tests/DashboardTest.java | 2 +- .../dashboard/testbench/DashboardElement.java | 15 ++++----------- 5 files changed, 8 insertions(+), 15 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java index 441ea53d80f..759c0b77504 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java @@ -1,4 +1,4 @@ -/* +/** * Copyright 2000-2024 Vaadin Ltd. * * This program is available under Vaadin Commercial License and Service Terms. diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java index ee58c1deb56..5882f619a7a 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java @@ -1,4 +1,4 @@ -/* +/** * Copyright 2000-2024 Vaadin Ltd. * * This program is available under Vaadin Commercial License and Service Terms. 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 3696eac71e0..a181ab07ffb 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 @@ -1,4 +1,4 @@ -/* +/** * Copyright 2000-2024 Vaadin Ltd. * * This program is available under Vaadin Commercial License and Service Terms. diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java index 32302fb1a60..6515b20e04c 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java @@ -1,4 +1,4 @@ -/* +/** * Copyright 2000-2024 Vaadin Ltd. * * This program is available under Vaadin Commercial License and Service Terms. diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java index 6b6870af58c..725f5b8008b 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-testbench/src/main/java/com/vaadin/flow/component/dashboard/testbench/DashboardElement.java @@ -1,17 +1,10 @@ -/* +/** * 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 + * This program is available under Vaadin Commercial License and Service Terms. * - * 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. + * See {@literal } for the full + * license. */ package com.vaadin.flow.component.dashboard.testbench; From fbeb49e8f728962078b8e5851790a6d8814a1b14 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 26 Aug 2024 18:50:27 +0300 Subject: [PATCH 05/21] test: remove first and last widgets and use clickelementswithjs --- .../dashboard/tests/DashboardPage.java | 26 ++++++++++++++----- .../dashboard/tests/DashboardIT.java | 9 ++++--- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java index 759c0b77504..12a98e075e3 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardPage.java @@ -8,6 +8,8 @@ */ package com.vaadin.flow.component.dashboard.tests; +import java.util.List; + import com.vaadin.flow.component.dashboard.Dashboard; import com.vaadin.flow.component.dashboard.DashboardWidget; import com.vaadin.flow.component.html.Div; @@ -46,16 +48,28 @@ public DashboardPage() { }); addWidgetAtIndex1.setId("add-widget-at-index-1"); - NativeButton removeWidgets1And3 = new NativeButton( - "Remove widgets 1 and 3"); - removeWidgets1And3 - .addClickListener(click -> dashboard.remove(widget1, widget3)); - removeWidgets1And3.setId("remove-widgets-1-and-3"); + NativeButton removeFirstAndLastWidgets = new NativeButton( + "Remove first and last widgets"); + removeFirstAndLastWidgets.addClickListener(click -> { + List currentWidgets = dashboard.getWidgets(); + if (currentWidgets.isEmpty()) { + return; + } + int currentWidgetCount = currentWidgets.size(); + if (currentWidgetCount == 1) { + dashboard.remove(dashboard.getWidgets().get(0)); + } else { + dashboard.remove(dashboard.getWidgets().get(0), + dashboard.getWidgets().get(currentWidgetCount - 1)); + } + }); + removeFirstAndLastWidgets.setId("remove-first-and-last-widgets"); NativeButton removeAllWidgets = new NativeButton("Remove all widgets"); removeAllWidgets.addClickListener(click -> dashboard.removeAll()); removeAllWidgets.setId("remove-all-widgets"); - add(addWidgetAtIndex1, removeWidgets1And3, removeAllWidgets, dashboard); + add(addWidgetAtIndex1, removeFirstAndLastWidgets, removeAllWidgets, + dashboard); } } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java index 5882f619a7a..036f95940cb 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java @@ -14,6 +14,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.openqa.selenium.By; import com.vaadin.flow.component.dashboard.testbench.DashboardElement; import com.vaadin.flow.component.dashboard.testbench.DashboardWidgetElement; @@ -41,20 +42,20 @@ public void addWidgets_widgetsAreCorrectlyAdded() { @Test public void addWidgetsAtIndex1_widgetIsAddedIntoTheCorrectPlace() { - $("button").id("add-widget-at-index-1").click(); + clickElementWithJs(findElement(By.id("add-widget-at-index-1"))); assertWidgetsByTitle("Widget 1", "Widget at index 1", "Widget 2", "Widget 3"); } @Test - public void removeWidgets1And3_widgetsAreCorrectlyRemoved() { - $("button").id("remove-widgets-1-and-3").click(); + public void removeFirstAndLastWidgets_widgetsAreCorrectlyRemoved() { + clickElementWithJs(findElement(By.id("remove-first-and-last-widgets"))); assertWidgetsByTitle("Widget 2"); } @Test public void removeAllWidgets_widgetsAreCorrectlyRemoved() { - $("button").id("remove-all-widgets").click(); + clickElementWithJs(findElement(By.id("remove-all-widgets"))); assertWidgetsByTitle(); } From 73ede6b854ab53b85474b481901ba4f54e2061ad Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 26 Aug 2024 18:53:30 +0300 Subject: [PATCH 06/21] refactor: make remove from parent use removevirtualchild for widgets --- .../component/dashboard/DashboardWidget.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java index 270963d93ad..284bc63c0e5 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java @@ -8,10 +8,13 @@ */ package com.vaadin.flow.component.dashboard; +import java.util.Optional; + import com.vaadin.flow.component.Component; import com.vaadin.flow.component.Tag; import com.vaadin.flow.component.dependency.JsModule; import com.vaadin.flow.component.dependency.NpmPackage; +import com.vaadin.flow.dom.Node; /** * @author Vaadin Ltd @@ -41,4 +44,18 @@ public String getTitle() { public void setTitle(String title) { getElement().setProperty("widgetTitle", title == null ? "" : title); } + + @Override + public void removeFromParent() { + Optional optionalParent = getParent(); + if (optionalParent.isPresent() + && optionalParent.get() instanceof Dashboard) { + Node parent = getElement().getParentNode(); + if (parent != null) { + parent.removeVirtualChild(getElement()); + } + } else { + super.removeFromParent(); + } + } } From beea359664261445c3795f5ec69e9428423c594b Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 26 Aug 2024 18:54:57 +0300 Subject: [PATCH 07/21] refactor: use append and remove with virtual children and remove from parent before add --- .../flow/component/dashboard/Dashboard.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) 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 a181ab07ffb..e7e16f98a82 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 @@ -224,22 +224,30 @@ private int getWidgetNodeId(DashboardWidget widget) { } private void doRemoveAllWidgets() { + List elementsToRemove = widgets.stream() + .map(Component::getElement).toList(); + elementsToRemove.forEach(getElement()::removeVirtualChild); widgets.clear(); - getElement().removeAllChildren(); } private void doRemoveWidget(DashboardWidget widget) { + getElement().removeVirtualChild(widget.getElement()); widgets.remove(widget); - getElement().removeChild(widget.getElement()); } private void doAddWidget(int index, DashboardWidget widget) { + if (widget.getParent().isPresent()) { + widget.removeFromParent(); + } + getElement().appendVirtualChild(widget.getElement()); widgets.add(index, widget); - getElement().insertChild(index, widget.getElement()); } private void doAddWidget(DashboardWidget widget) { + if (widget.getParent().isPresent()) { + widget.removeFromParent(); + } + getElement().appendVirtualChild(widget.getElement()); widgets.add(widget); - getElement().appendChild(widget.getElement()); } } From ef25b524ea413466e77223e66ff36dad622c2ac8 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 26 Aug 2024 18:55:22 +0300 Subject: [PATCH 08/21] chore: remove unused code --- .../java/com/vaadin/flow/component/dashboard/Dashboard.java | 3 --- 1 file changed, 3 deletions(-) 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 e7e16f98a82..858b44f3816 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 @@ -194,9 +194,6 @@ private void doUpdateClient() { child.addDetachListener(childDetachListener)); } }); - List widgetNodeIds = getWidgets().stream() - .map(this::getWidgetNodeId).toList(); - getElement().setPropertyList("virtualChildNodeIds", widgetNodeIds); getElement().setPropertyJson("items", createItemsJsonArray()); } From 193f851ca59991e14907b90535285b27d86ef760 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 26 Aug 2024 18:56:14 +0300 Subject: [PATCH 09/21] refactor: merge attach handlers --- .../java/com/vaadin/flow/component/dashboard/Dashboard.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 858b44f3816..f430f4f3424 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 @@ -51,7 +51,6 @@ public class Dashboard extends Component { * Creates an empty dashboard. */ public Dashboard() { - getElement().getNode().addAttachListener(this::attachRenderer); } /** @@ -172,7 +171,8 @@ public void onDetach(ElementDetachEvent e) { @Override protected void onAttach(AttachEvent attachEvent) { super.onAttach(attachEvent); - updateClient(); + attachRenderer(); + doUpdateClient(); } private void updateClient() { From 846cffa720bda7d960b57fbcb6cfde6e4f16b9e7 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 26 Aug 2024 18:56:53 +0300 Subject: [PATCH 10/21] refactor: do not try to remove virtual child on detach --- .../java/com/vaadin/flow/component/dashboard/Dashboard.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f430f4f3424..de4b702d504 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 @@ -162,7 +162,7 @@ public void onDetach(ElementDetachEvent e) { childDetachListenerMap .remove(detachedWidget.getElement()); - doRemoveWidget(detachedWidget); + widgets.remove(detachedWidget); updateClient(); }); } From ca6690aa93c282a1cc7393800a675efd896442f3 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 26 Aug 2024 18:58:26 +0300 Subject: [PATCH 11/21] refactor: use widgets list to update detach listeners --- .../com/vaadin/flow/component/dashboard/Dashboard.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) 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 de4b702d504..ecbc442b858 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 @@ -188,10 +188,12 @@ private void updateClient() { } private void doUpdateClient() { - getElement().getChildren().forEach(child -> { - if (!childDetachListenerMap.containsKey(child)) { - childDetachListenerMap.put(child, - child.addDetachListener(childDetachListener)); + widgets.forEach(widget -> { + Element childWidgetElement = widget.getElement(); + if (!childDetachListenerMap.containsKey(childWidgetElement)) { + childDetachListenerMap.put(childWidgetElement, + childWidgetElement + .addDetachListener(childDetachListener)); } }); getElement().setPropertyJson("items", createItemsJsonArray()); From 00641de7920020c14454ddf1574f02b26269d505 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 26 Aug 2024 18:59:10 +0300 Subject: [PATCH 12/21] test: add more unit tests for testing node ids on detach --- .../dashboard/tests/DashboardTest.java | 127 ++++++++++++++++-- 1 file changed, 119 insertions(+), 8 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java index 6515b20e04c..eb2d2f4112b 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java @@ -8,20 +8,48 @@ */ package com.vaadin.flow.component.dashboard.tests; +import java.util.Arrays; import java.util.Collections; import java.util.List; +import org.junit.After; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; +import com.vaadin.flow.component.Component; +import com.vaadin.flow.component.UI; import com.vaadin.flow.component.dashboard.Dashboard; import com.vaadin.flow.component.dashboard.DashboardWidget; +import com.vaadin.flow.internal.JsonUtils; +import com.vaadin.flow.server.VaadinSession; + +import elemental.json.JsonArray; public class DashboardTest { + private final UI ui = new UI(); + private Dashboard dashboard; + + @Before + public void setup() { + UI.setCurrent(ui); + VaadinSession session = Mockito.mock(VaadinSession.class); + Mockito.when(session.hasLock()).thenReturn(true); + ui.getInternals().setSession(session); + dashboard = new Dashboard(); + ui.add(dashboard); + fakeClientCommunication(); + } + + @After + public void tearDown() { + UI.setCurrent(null); + } + @Test public void addWidget_widgetIsAdded() { - Dashboard dashboard = new Dashboard(); DashboardWidget widget1 = new DashboardWidget(); DashboardWidget widget2 = new DashboardWidget(); dashboard.add(widget1, widget2); @@ -31,14 +59,12 @@ public void addWidget_widgetIsAdded() { @Test public void addNullWidget_exceptionIsThrown() { - Dashboard dashboard = new Dashboard(); Assert.assertThrows(NullPointerException.class, () -> dashboard.add((DashboardWidget) null)); } @Test public void addNullWidgetInArray_noWidgetIsAdded() { - Dashboard dashboard = new Dashboard(); DashboardWidget widget = new DashboardWidget(); try { dashboard.add(widget, null); @@ -50,7 +76,6 @@ public void addNullWidgetInArray_noWidgetIsAdded() { @Test public void addWidgetAtIndex_widgetIsCorrectlyAdded() { - Dashboard dashboard = new Dashboard(); DashboardWidget widget1 = new DashboardWidget(); DashboardWidget widget2 = new DashboardWidget(); DashboardWidget widget3 = new DashboardWidget(); @@ -63,14 +88,12 @@ public void addWidgetAtIndex_widgetIsCorrectlyAdded() { @Test public void addNullWidgetAtIndex_exceptionIsThrown() { - Dashboard dashboard = new Dashboard(); Assert.assertThrows(NullPointerException.class, () -> dashboard.addWidgetAtIndex(0, null)); } @Test public void removeWidget_widgetIsRemoved() { - Dashboard dashboard = new Dashboard(); DashboardWidget widget1 = new DashboardWidget(); DashboardWidget widget2 = new DashboardWidget(); dashboard.add(widget1, widget2); @@ -81,7 +104,6 @@ public void removeWidget_widgetIsRemoved() { @Test public void removeNullWidget_exceptionIsThrown() { - Dashboard dashboard = new Dashboard(); Assert.assertThrows(NullPointerException.class, () -> dashboard.remove((DashboardWidget) null)); @@ -89,7 +111,6 @@ public void removeNullWidget_exceptionIsThrown() { @Test public void removeAllWidgets_widgetsAreRemoved() { - Dashboard dashboard = new Dashboard(); DashboardWidget widget1 = new DashboardWidget(); DashboardWidget widget2 = new DashboardWidget(); dashboard.add(widget1, widget2); @@ -97,4 +118,94 @@ public void removeAllWidgets_widgetsAreRemoved() { Assert.assertEquals(Collections.emptyList(), dashboard.getWidgets()); } + + @Test + public void removeWidgetFromParent_widgetIsRemoved() { + DashboardWidget widget1 = new DashboardWidget(); + dashboard.add(widget1); + fakeClientCommunication(); + widget1.removeFromParent(); + Assert.assertEquals(Collections.emptyList(), dashboard.getWidgets()); + } + + @Test + public void addWidget_virtualNodeIdsInSync() { + DashboardWidget widget1 = new DashboardWidget(); + dashboard.add(widget1); + fakeClientCommunication(); + assertVirtualChildren(dashboard, widget1); + } + + @Test + public void removeWidget_virtualNodeIdsInSync() { + DashboardWidget widget1 = new DashboardWidget(); + dashboard.add(widget1); + fakeClientCommunication(); + widget1.removeFromParent(); + fakeClientCommunication(); + assertVirtualChildren(dashboard); + } + + @Test + public void selfRemoveChild_virtualNodeIdsInSync() { + DashboardWidget widget1 = new DashboardWidget(); + DashboardWidget widget2 = new DashboardWidget(); + dashboard.add(widget1, widget2); + fakeClientCommunication(); + widget1.removeFromParent(); + fakeClientCommunication(); + assertVirtualChildren(dashboard, widget2); + } + + @Test + public void addSeparately_selfRemoveChild_doesNotThrow() { + DashboardWidget widget1 = new DashboardWidget(); + DashboardWidget widget2 = new DashboardWidget(); + dashboard.add(widget1); + dashboard.add(widget2); + fakeClientCommunication(); + widget1.removeFromParent(); + fakeClientCommunication(); + assertVirtualChildren(dashboard, widget2); + } + + @Test + public void addWidgetToAnotherDashboard_widgetIsAdded() { + DashboardWidget widget1 = new DashboardWidget(); + dashboard.add(widget1); + fakeClientCommunication(); + + Dashboard newDashboard = new Dashboard(); + ui.add(newDashboard); + newDashboard.add(widget1); + fakeClientCommunication(); + + assertVirtualChildren(dashboard); + assertVirtualChildren(newDashboard, widget1); + } + + private void fakeClientCommunication() { + ui.getInternals().getStateTree().runExecutionsBeforeClientResponse(); + ui.getInternals().getStateTree().collectChanges(ignore -> { + }); + } + + private static void assertVirtualChildren(Dashboard dashboard, + Component... components) { + // Get a List of the node ids + List expectedChildNodeIds = Arrays.stream(components) + .map(component -> component.getElement().getNode().getId()) + .toList(); + // Get the node ids from the items property of the dashboard + List actualChildNodeIds = getChildNodeIds(dashboard); + Assert.assertEquals(expectedChildNodeIds, actualChildNodeIds); + } + + private static List getChildNodeIds(Dashboard dashboard) { + JsonArray jsonArrayOfIds = (JsonArray) dashboard.getElement() + .getPropertyRaw("items"); + return JsonUtils.objectStream(jsonArrayOfIds) + .mapToInt(obj -> (int) obj.getNumber("nodeid")).boxed() + .toList(); + } } From b6b8e35b6b1924eae248713e2c0ea69f4f90373a Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 27 Aug 2024 10:53:32 +0300 Subject: [PATCH 13/21] refactor: use dashboard remove when removing widget from parent --- .../vaadin/flow/component/dashboard/DashboardWidget.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java index 284bc63c0e5..7f9e2b65143 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java @@ -14,7 +14,6 @@ import com.vaadin.flow.component.Tag; import com.vaadin.flow.component.dependency.JsModule; import com.vaadin.flow.component.dependency.NpmPackage; -import com.vaadin.flow.dom.Node; /** * @author Vaadin Ltd @@ -49,11 +48,8 @@ public void setTitle(String title) { public void removeFromParent() { Optional optionalParent = getParent(); if (optionalParent.isPresent() - && optionalParent.get() instanceof Dashboard) { - Node parent = getElement().getParentNode(); - if (parent != null) { - parent.removeVirtualChild(getElement()); - } + && optionalParent.get() instanceof Dashboard dashboard) { + dashboard.remove(this); } else { super.removeFromParent(); } From 7eebb0908fc0535db4ef0587e6f91f34a369d23c Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 27 Aug 2024 10:53:58 +0300 Subject: [PATCH 14/21] test: use clickelementwithjs without locating element --- .../vaadin/flow/component/dashboard/tests/DashboardIT.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java index 036f95940cb..68c20102e7b 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardIT.java @@ -14,7 +14,6 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import org.openqa.selenium.By; import com.vaadin.flow.component.dashboard.testbench.DashboardElement; import com.vaadin.flow.component.dashboard.testbench.DashboardWidgetElement; @@ -42,20 +41,20 @@ public void addWidgets_widgetsAreCorrectlyAdded() { @Test public void addWidgetsAtIndex1_widgetIsAddedIntoTheCorrectPlace() { - clickElementWithJs(findElement(By.id("add-widget-at-index-1"))); + clickElementWithJs("add-widget-at-index-1"); assertWidgetsByTitle("Widget 1", "Widget at index 1", "Widget 2", "Widget 3"); } @Test public void removeFirstAndLastWidgets_widgetsAreCorrectlyRemoved() { - clickElementWithJs(findElement(By.id("remove-first-and-last-widgets"))); + clickElementWithJs("remove-first-and-last-widgets"); assertWidgetsByTitle("Widget 2"); } @Test public void removeAllWidgets_widgetsAreCorrectlyRemoved() { - clickElementWithJs(findElement(By.id("remove-all-widgets"))); + clickElementWithJs("remove-all-widgets"); assertWidgetsByTitle(); } From 752873df4464c7135508af0e1028b93857d02a6a Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 27 Aug 2024 11:56:33 +0300 Subject: [PATCH 15/21] refactor: remove detach listeners --- .../flow/component/dashboard/Dashboard.java | 38 ------------------- 1 file changed, 38 deletions(-) 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 ecbc442b858..da457c34f02 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 @@ -10,9 +10,7 @@ import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Objects; import org.slf4j.LoggerFactory; @@ -24,9 +22,6 @@ import com.vaadin.flow.component.dependency.JsModule; import com.vaadin.flow.component.dependency.NpmPackage; import com.vaadin.flow.dom.Element; -import com.vaadin.flow.dom.ElementDetachEvent; -import com.vaadin.flow.dom.ElementDetachListener; -import com.vaadin.flow.shared.Registration; import elemental.json.Json; import elemental.json.JsonArray; @@ -142,31 +137,6 @@ public void removeAll() { updateClient(); } - private final Map childDetachListenerMap = new HashMap<>(); - - // Must not use lambda here as that would break serialization. See - // https://github.com/vaadin/flow-components/issues/5597 - private final ElementDetachListener childDetachListener = new ElementDetachListener() { - @Override - public void onDetach(ElementDetachEvent e) { - var detachedElement = e.getSource(); - getWidgets().stream() - .filter(widget -> Objects.equals(detachedElement, - widget.getElement())) - .findAny().ifPresent(detachedWidget -> { - // The child was removed from the dashboard - - // Remove the registration for the child detach listener - childDetachListenerMap.get(detachedWidget.getElement()) - .remove(); - childDetachListenerMap - .remove(detachedWidget.getElement()); - - widgets.remove(detachedWidget); - updateClient(); - }); - } - }; @Override protected void onAttach(AttachEvent attachEvent) { @@ -188,14 +158,6 @@ private void updateClient() { } private void doUpdateClient() { - widgets.forEach(widget -> { - Element childWidgetElement = widget.getElement(); - if (!childDetachListenerMap.containsKey(childWidgetElement)) { - childDetachListenerMap.put(childWidgetElement, - childWidgetElement - .addDetachListener(childDetachListener)); - } - }); getElement().setPropertyJson("items", createItemsJsonArray()); } From 426c2889620d989105de6cae362dd91b307899c3 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 27 Aug 2024 11:57:26 +0300 Subject: [PATCH 16/21] test: add tests for moving widgets between parents --- .../dashboard/tests/DashboardTest.java | 80 +++++++++++++++++-- 1 file changed, 74 insertions(+), 6 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java index eb2d2f4112b..036dbd9b83c 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java @@ -22,6 +22,7 @@ import com.vaadin.flow.component.UI; import com.vaadin.flow.component.dashboard.Dashboard; import com.vaadin.flow.component.dashboard.DashboardWidget; +import com.vaadin.flow.component.html.Div; import com.vaadin.flow.internal.JsonUtils; import com.vaadin.flow.server.VaadinSession; @@ -170,18 +171,78 @@ public void addSeparately_selfRemoveChild_doesNotThrow() { } @Test - public void addWidgetToAnotherDashboard_widgetIsAdded() { - DashboardWidget widget1 = new DashboardWidget(); - dashboard.add(widget1); + public void addWidgetToLayout_widgetIsAdded() { + Div layout = new Div(); + ui.add(layout); + DashboardWidget widget = new DashboardWidget(); + layout.add(widget); + fakeClientCommunication(); + Assert.assertTrue(widget.getParent().isPresent()); + Assert.assertEquals(layout, widget.getParent().get()); + } + + @Test + public void removeWidgetFromLayout_widgetIsRemoved() { + Div layout = new Div(); + ui.add(layout); + DashboardWidget widget = new DashboardWidget(); + layout.add(widget); + fakeClientCommunication(); + layout.remove(widget); + fakeClientCommunication(); + Assert.assertTrue(widget.getParent().isEmpty()); + } + + @Test + public void addWidgetToLayout_removeFromParent_widgetIsRemoved() { + Div layout = new Div(); + ui.add(layout); + DashboardWidget widget = new DashboardWidget(); + layout.add(widget); + fakeClientCommunication(); + widget.removeFromParent(); + fakeClientCommunication(); + Assert.assertTrue(widget.getParent().isEmpty()); + } + + @Test + public void addWidgetFromLayoutToDashboard_widgetIsAdded() { + Div parent = new Div(); + ui.add(parent); + DashboardWidget widget = new DashboardWidget(); + parent.add(widget); fakeClientCommunication(); + dashboard.add(widget); + fakeClientCommunication(); + assertWidgets(dashboard, widget); + } + @Test + public void addWidgetToAnotherDashboard_widgetIsMoved() { + DashboardWidget widget = new DashboardWidget(); + dashboard.add(widget); + fakeClientCommunication(); Dashboard newDashboard = new Dashboard(); ui.add(newDashboard); - newDashboard.add(widget1); + newDashboard.add(widget); fakeClientCommunication(); + assertWidgets(dashboard); + assertWidgets(newDashboard, widget); + } - assertVirtualChildren(dashboard); - assertVirtualChildren(newDashboard, widget1); + @Test + public void addWidgetFromLayoutToOtherLayout_widgetIsAdded() { + Div parent = new Div(); + ui.add(parent); + DashboardWidget widget = new DashboardWidget(); + parent.add(widget); + fakeClientCommunication(); + Div newParent = new Div(); + ui.add(newParent); + newParent.add(widget); + fakeClientCommunication(); + Assert.assertTrue(widget.getParent().isPresent()); + Assert.assertEquals(newParent, widget.getParent().get()); } private void fakeClientCommunication() { @@ -190,6 +251,13 @@ private void fakeClientCommunication() { }); } + private static void assertWidgets(Dashboard dashboard, + DashboardWidget... expectedWidgets) { + assertVirtualChildren(dashboard, expectedWidgets); + Assert.assertEquals(Arrays.asList(expectedWidgets), + dashboard.getWidgets()); + } + private static void assertVirtualChildren(Dashboard dashboard, Component... components) { // Get a List of the node ids From 7a6908ed958495ee0be908e390ecaf3f847b82f9 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 27 Aug 2024 14:54:52 +0300 Subject: [PATCH 17/21] test: add unit tests for moving widgets between components --- .../dashboard/tests/DashboardWidgetTest.java | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetTest.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetTest.java index bea69c56ff2..8ae7bcd2423 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetTest.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetTest.java @@ -8,5 +8,86 @@ */ package com.vaadin.flow.component.dashboard.tests; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +import com.vaadin.flow.component.UI; +import com.vaadin.flow.component.dashboard.DashboardWidget; +import com.vaadin.flow.component.html.Div; +import com.vaadin.flow.server.VaadinSession; + public class DashboardWidgetTest { + + private final UI ui = new UI(); + + @Before + public void setup() { + UI.setCurrent(ui); + VaadinSession session = Mockito.mock(VaadinSession.class); + Mockito.when(session.hasLock()).thenReturn(true); + ui.getInternals().setSession(session); + } + + @After + public void tearDown() { + UI.setCurrent(null); + } + + @Test + public void addWidgetToLayout_widgetIsAdded() { + Div layout = new Div(); + ui.add(layout); + DashboardWidget widget = new DashboardWidget(); + layout.add(widget); + fakeClientCommunication(); + Assert.assertTrue(layout.getChildren().anyMatch(widget::equals)); + } + + @Test + public void removeWidgetFromLayout_widgetIsRemoved() { + Div layout = new Div(); + ui.add(layout); + DashboardWidget widget = new DashboardWidget(); + layout.add(widget); + fakeClientCommunication(); + layout.remove(widget); + fakeClientCommunication(); + Assert.assertTrue(layout.getChildren().noneMatch(widget::equals)); + } + + @Test + public void addWidgetToLayout_removeFromParent_widgetIsRemoved() { + Div layout = new Div(); + ui.add(layout); + DashboardWidget widget = new DashboardWidget(); + layout.add(widget); + fakeClientCommunication(); + widget.removeFromParent(); + fakeClientCommunication(); + Assert.assertTrue(layout.getChildren().noneMatch(widget::equals)); + } + + @Test + public void addWidgetFromLayoutToAnotherLayout_widgetIsMoved() { + Div parent = new Div(); + ui.add(parent); + DashboardWidget widget = new DashboardWidget(); + parent.add(widget); + fakeClientCommunication(); + Div newParent = new Div(); + ui.add(newParent); + newParent.add(widget); + fakeClientCommunication(); + Assert.assertTrue(parent.getChildren().noneMatch(widget::equals)); + Assert.assertTrue(newParent.getChildren().anyMatch(widget::equals)); + } + + private void fakeClientCommunication() { + ui.getInternals().getStateTree().runExecutionsBeforeClientResponse(); + ui.getInternals().getStateTree().collectChanges(ignore -> { + }); + } } From 1b69222767692937051ac765913ee2017de0debd Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 27 Aug 2024 15:06:58 +0300 Subject: [PATCH 18/21] test: cleanup dashboard widget related unit tests --- .../dashboard/tests/DashboardTest.java | 94 ++++++------------- 1 file changed, 30 insertions(+), 64 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java index 036dbd9b83c..24c2af1d691 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTest.java @@ -9,7 +9,6 @@ package com.vaadin.flow.component.dashboard.tests; import java.util.Arrays; -import java.util.Collections; import java.util.List; import org.junit.After; @@ -54,8 +53,8 @@ public void addWidget_widgetIsAdded() { DashboardWidget widget1 = new DashboardWidget(); DashboardWidget widget2 = new DashboardWidget(); dashboard.add(widget1, widget2); - - Assert.assertEquals(List.of(widget1, widget2), dashboard.getWidgets()); + fakeClientCommunication(); + assertWidgets(dashboard, widget1, widget2); } @Test @@ -72,7 +71,8 @@ public void addNullWidgetInArray_noWidgetIsAdded() { } catch (NullPointerException e) { // Do nothing } - Assert.assertEquals(Collections.emptyList(), dashboard.getWidgets()); + fakeClientCommunication(); + assertWidgets(dashboard); } @Test @@ -81,10 +81,10 @@ public void addWidgetAtIndex_widgetIsCorrectlyAdded() { DashboardWidget widget2 = new DashboardWidget(); DashboardWidget widget3 = new DashboardWidget(); dashboard.add(widget1, widget2); + fakeClientCommunication(); dashboard.addWidgetAtIndex(1, widget3); - - Assert.assertEquals(List.of(widget1, widget3, widget2), - dashboard.getWidgets()); + fakeClientCommunication(); + assertWidgets(dashboard, widget1, widget3, widget2); } @Test @@ -98,16 +98,16 @@ public void removeWidget_widgetIsRemoved() { DashboardWidget widget1 = new DashboardWidget(); DashboardWidget widget2 = new DashboardWidget(); dashboard.add(widget1, widget2); + fakeClientCommunication(); dashboard.remove(widget1); - - Assert.assertEquals(List.of(widget2), dashboard.getWidgets()); + fakeClientCommunication(); + assertWidgets(dashboard, widget2); } @Test public void removeNullWidget_exceptionIsThrown() { Assert.assertThrows(NullPointerException.class, () -> dashboard.remove((DashboardWidget) null)); - } @Test @@ -115,9 +115,10 @@ public void removeAllWidgets_widgetsAreRemoved() { DashboardWidget widget1 = new DashboardWidget(); DashboardWidget widget2 = new DashboardWidget(); dashboard.add(widget1, widget2); + fakeClientCommunication(); dashboard.removeAll(); - - Assert.assertEquals(Collections.emptyList(), dashboard.getWidgets()); + fakeClientCommunication(); + assertWidgets(dashboard); } @Test @@ -126,7 +127,8 @@ public void removeWidgetFromParent_widgetIsRemoved() { dashboard.add(widget1); fakeClientCommunication(); widget1.removeFromParent(); - Assert.assertEquals(Collections.emptyList(), dashboard.getWidgets()); + fakeClientCommunication(); + assertWidgets(dashboard); } @Test @@ -134,7 +136,7 @@ public void addWidget_virtualNodeIdsInSync() { DashboardWidget widget1 = new DashboardWidget(); dashboard.add(widget1); fakeClientCommunication(); - assertVirtualChildren(dashboard, widget1); + assertWidgets(dashboard, widget1); } @Test @@ -144,7 +146,7 @@ public void removeWidget_virtualNodeIdsInSync() { fakeClientCommunication(); widget1.removeFromParent(); fakeClientCommunication(); - assertVirtualChildren(dashboard); + assertWidgets(dashboard); } @Test @@ -155,7 +157,7 @@ public void selfRemoveChild_virtualNodeIdsInSync() { fakeClientCommunication(); widget1.removeFromParent(); fakeClientCommunication(); - assertVirtualChildren(dashboard, widget2); + assertWidgets(dashboard, widget2); } @Test @@ -167,54 +169,33 @@ public void addSeparately_selfRemoveChild_doesNotThrow() { fakeClientCommunication(); widget1.removeFromParent(); fakeClientCommunication(); - assertVirtualChildren(dashboard, widget2); - } - - @Test - public void addWidgetToLayout_widgetIsAdded() { - Div layout = new Div(); - ui.add(layout); - DashboardWidget widget = new DashboardWidget(); - layout.add(widget); - fakeClientCommunication(); - Assert.assertTrue(widget.getParent().isPresent()); - Assert.assertEquals(layout, widget.getParent().get()); + assertWidgets(dashboard, widget2); } @Test - public void removeWidgetFromLayout_widgetIsRemoved() { - Div layout = new Div(); - ui.add(layout); + public void addWidgetFromLayoutToDashboard_widgetIsMoved() { + Div parent = new Div(); + ui.add(parent); DashboardWidget widget = new DashboardWidget(); - layout.add(widget); + parent.add(widget); fakeClientCommunication(); - layout.remove(widget); + dashboard.add(widget); fakeClientCommunication(); - Assert.assertTrue(widget.getParent().isEmpty()); + Assert.assertTrue(parent.getChildren().noneMatch(widget::equals)); + assertWidgets(dashboard, widget); } @Test - public void addWidgetToLayout_removeFromParent_widgetIsRemoved() { - Div layout = new Div(); - ui.add(layout); + public void addWidgetFromDashboardToLayout_widgetIsMoved() { DashboardWidget widget = new DashboardWidget(); - layout.add(widget); - fakeClientCommunication(); - widget.removeFromParent(); + dashboard.add(widget); fakeClientCommunication(); - Assert.assertTrue(widget.getParent().isEmpty()); - } - - @Test - public void addWidgetFromLayoutToDashboard_widgetIsAdded() { Div parent = new Div(); ui.add(parent); - DashboardWidget widget = new DashboardWidget(); parent.add(widget); fakeClientCommunication(); - dashboard.add(widget); - fakeClientCommunication(); - assertWidgets(dashboard, widget); + assertWidgets(dashboard); + Assert.assertTrue(parent.getChildren().anyMatch(widget::equals)); } @Test @@ -230,21 +211,6 @@ public void addWidgetToAnotherDashboard_widgetIsMoved() { assertWidgets(newDashboard, widget); } - @Test - public void addWidgetFromLayoutToOtherLayout_widgetIsAdded() { - Div parent = new Div(); - ui.add(parent); - DashboardWidget widget = new DashboardWidget(); - parent.add(widget); - fakeClientCommunication(); - Div newParent = new Div(); - ui.add(newParent); - newParent.add(widget); - fakeClientCommunication(); - Assert.assertTrue(widget.getParent().isPresent()); - Assert.assertEquals(newParent, widget.getParent().get()); - } - private void fakeClientCommunication() { ui.getInternals().getStateTree().runExecutionsBeforeClientResponse(); ui.getInternals().getStateTree().collectChanges(ignore -> { From 07ea60dec15da42dd70d4027d460a6e581a64e0b Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 27 Aug 2024 15:08:06 +0300 Subject: [PATCH 19/21] refactor: use add remove directly instead of using virtual children --- .../flow/component/dashboard/Dashboard.java | 54 +++++++++++++++---- .../component/dashboard/DashboardWidget.java | 13 ----- 2 files changed, 44 insertions(+), 23 deletions(-) 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 da457c34f02..cf51607f042 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 @@ -10,8 +10,11 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.stream.Stream; import org.slf4j.LoggerFactory; @@ -22,6 +25,9 @@ import com.vaadin.flow.component.dependency.JsModule; import com.vaadin.flow.component.dependency.NpmPackage; import com.vaadin.flow.dom.Element; +import com.vaadin.flow.dom.ElementDetachEvent; +import com.vaadin.flow.dom.ElementDetachListener; +import com.vaadin.flow.shared.Registration; import elemental.json.Json; import elemental.json.JsonArray; @@ -145,6 +151,32 @@ protected void onAttach(AttachEvent attachEvent) { doUpdateClient(); } + private final Map childDetachListenerMap = new HashMap<>(); + + // Must not use lambda here as that would break serialization. See + // https://github.com/vaadin/flow-components/issues/5597 + private final ElementDetachListener childDetachListener = new ElementDetachListener() { + @Override + public void onDetach(ElementDetachEvent e) { + var detachedElement = e.getSource(); + getWidgets().stream() + .filter(widget -> Objects.equals(detachedElement, + widget.getElement())) + .findAny().ifPresent(detachedWidget -> { + // The child was removed from the dashboard + + // Remove the registration for the child detach listener + childDetachListenerMap.get(detachedWidget.getElement()) + .remove(); + childDetachListenerMap + .remove(detachedWidget.getElement()); + + widgets.remove(detachedWidget); + updateClient(); + }); + } + }; + private void updateClient() { if (pendingUpdate) { return; @@ -158,6 +190,14 @@ private void updateClient() { } private void doUpdateClient() { + widgets.forEach(widget -> { + Element childWidgetElement = widget.getElement(); + if (!childDetachListenerMap.containsKey(childWidgetElement)) { + childDetachListenerMap.put(childWidgetElement, + childWidgetElement + .addDetachListener(childDetachListener)); + } + }); getElement().setPropertyJson("items", createItemsJsonArray()); } @@ -187,28 +227,22 @@ private int getWidgetNodeId(DashboardWidget widget) { private void doRemoveAllWidgets() { List elementsToRemove = widgets.stream() .map(Component::getElement).toList(); - elementsToRemove.forEach(getElement()::removeVirtualChild); + elementsToRemove.forEach(getElement()::removeChild); widgets.clear(); } private void doRemoveWidget(DashboardWidget widget) { - getElement().removeVirtualChild(widget.getElement()); + getElement().removeChild(widget.getElement()); widgets.remove(widget); } private void doAddWidget(int index, DashboardWidget widget) { - if (widget.getParent().isPresent()) { - widget.removeFromParent(); - } - getElement().appendVirtualChild(widget.getElement()); + getElement().appendChild(widget.getElement()); widgets.add(index, widget); } private void doAddWidget(DashboardWidget widget) { - if (widget.getParent().isPresent()) { - widget.removeFromParent(); - } - getElement().appendVirtualChild(widget.getElement()); + getElement().appendChild(widget.getElement()); widgets.add(widget); } } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java index 7f9e2b65143..270963d93ad 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardWidget.java @@ -8,8 +8,6 @@ */ package com.vaadin.flow.component.dashboard; -import java.util.Optional; - import com.vaadin.flow.component.Component; import com.vaadin.flow.component.Tag; import com.vaadin.flow.component.dependency.JsModule; @@ -43,15 +41,4 @@ public String getTitle() { public void setTitle(String title) { getElement().setProperty("widgetTitle", title == null ? "" : title); } - - @Override - public void removeFromParent() { - Optional optionalParent = getParent(); - if (optionalParent.isPresent() - && optionalParent.get() instanceof Dashboard dashboard) { - dashboard.remove(this); - } else { - super.removeFromParent(); - } - } } From ea1a1f8865b0ff444472b62f368e4bd3b44dd8b0 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 27 Aug 2024 15:18:26 +0300 Subject: [PATCH 20/21] refactor: simplify removing all widgets by reusing single remove logic --- .../java/com/vaadin/flow/component/dashboard/Dashboard.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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 cf51607f042..c447d34f2c5 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 @@ -225,10 +225,7 @@ private int getWidgetNodeId(DashboardWidget widget) { } private void doRemoveAllWidgets() { - List elementsToRemove = widgets.stream() - .map(Component::getElement).toList(); - elementsToRemove.forEach(getElement()::removeChild); - widgets.clear(); + new ArrayList<>(widgets).forEach(this::doRemoveWidget); } private void doRemoveWidget(DashboardWidget widget) { From 738953e1a3df48050907f7652c09cf49ee6cea3c Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 27 Aug 2024 16:54:43 +0300 Subject: [PATCH 21/21] refactor: override getchildren --- .../java/com/vaadin/flow/component/dashboard/Dashboard.java | 4 ++++ 1 file changed, 4 insertions(+) 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 c447d34f2c5..b7526b27ba3 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 @@ -143,6 +143,10 @@ public void removeAll() { updateClient(); } + @Override + public Stream getChildren() { + return getWidgets().stream().map(Component.class::cast); + } @Override protected void onAttach(AttachEvent attachEvent) {