From d1d2d89fa03479f91f4e8f3adce75f69a5d8b52a Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 10 Sep 2024 19:39:17 +0300 Subject: [PATCH 01/11] feat: support drag drop for dashboard --- .../flow/component/dashboard/Dashboard.java | 212 ++++++++++++++++- .../DashboardItemReorderEndEvent.java | 52 +++++ .../DashboardItemReorderStartEvent.java | 37 +++ .../tests/DashboardDragDropTest.java | 216 ++++++++++++++++++ .../dashboard/tests/DashboardTest.java | 82 ++----- .../dashboard/tests/DashboardTestBase.java | 111 +++++++++ .../dashboard/tests/DashboardWidgetTest.java | 50 ++-- 7 files changed, 670 insertions(+), 90 deletions(-) create mode 100644 vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardItemReorderEndEvent.java create mode 100644 vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardItemReorderStartEvent.java create mode 100644 vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropTest.java create mode 100644 vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTestBase.java 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 9759822f802..361a1074c13 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 @@ -11,9 +11,14 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -21,10 +26,15 @@ import com.vaadin.flow.component.AttachEvent; import com.vaadin.flow.component.Component; +import com.vaadin.flow.component.ComponentEventListener; 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.Element; +import com.vaadin.flow.shared.Registration; + +import elemental.json.JsonArray; +import elemental.json.JsonObject; /** * @author Vaadin Ltd @@ -48,6 +58,9 @@ public class Dashboard extends Component implements HasWidgets { */ public Dashboard() { childDetachHandler = getChildDetachHandler(); + customizeItemReorderEndEvent(); + setEditable(true); + addItemReorderEndListener(this::onItemReorderEnd); } /** @@ -275,6 +288,49 @@ public void setGap(String gap) { getStyle().set("--vaadin-dashboard-gap", gap); } + /** + * Sets the option to make the dashboard editable. + * + * @param editable + * whether to set the dashboard editable + */ + public void setEditable(boolean editable) { + getElement().setProperty("editable", editable); + } + + /** + * Returns whether the dashboard is editable. + * + * @return whether to set the dashboard editable + */ + public boolean isEditable() { + return getElement().getProperty("editable", false); + } + + /** + * Adds an item reorder start listener to this dashboard. + * + * @param listener + * the listener to add, not null + * @return a handle that can be used for removing the listener + */ + public Registration addItemReorderStartListener( + ComponentEventListener listener) { + return addListener(DashboardItemReorderStartEvent.class, listener); + } + + /** + * Adds an item reorder end listener to this dashboard. + * + * @param listener + * the listener to add, not null + * @return a handle that can be used for removing the listener + */ + public Registration addItemReorderEndListener( + ComponentEventListener listener) { + return addListener(DashboardItemReorderEndEvent.class, listener); + } + @Override public Stream getChildren() { return childrenComponents.stream(); @@ -320,8 +376,9 @@ private void updateClientItems() { .map(widget -> getWidgetRepresentation(widget, itemIndex.getAndIncrement())) .collect(Collectors.joining(",")); - itemRepresentation = "{ component: $%d, items: [ %s ] }" - .formatted(sectionIndex, sectionWidgetsRepresentation); + itemRepresentation = "{ component: $%d, items: [ %s ], nodeid: %d }" + .formatted(sectionIndex, sectionWidgetsRepresentation, + section.getElement().getNode().getId()); } else { itemRepresentation = getWidgetRepresentation( (DashboardWidget) component, @@ -337,8 +394,8 @@ private void updateClientItems() { private static String getWidgetRepresentation(DashboardWidget widget, int itemIndex) { - return "{ component: $%d, colspan: %d, rowspan: %d }" - .formatted(itemIndex, widget.getColspan(), widget.getRowspan()); + return "{ component: $%d, colspan: %d, rowspan: %d, nodeid: %d }" + .formatted(itemIndex, widget.getColspan(), widget.getRowspan(), widget.getElement().getNode().getId()); } private void doRemoveAll() { @@ -390,4 +447,151 @@ Collection getDirectChildren() { } }; } + + private void onItemReorderEnd( + DashboardItemReorderEndEvent dashboardItemReorderEndEvent) { + JsonArray orderedItemsFromClient = dashboardItemReorderEndEvent + .getItems(); + if (areItemsInSync(orderedItemsFromClient)) { + reorderItems(orderedItemsFromClient); + } else { + LoggerFactory.getLogger(getClass()) + .debug("Dashboard items do not match those on the server."); + } + updateClient(); + } + + private void reorderItems(JsonArray orderedItemsFromClient) { + // Keep references to the root level children before clearing them + Map nodeIdToComponent = childrenComponents.stream() + .collect(Collectors.toMap( + component -> component.getElement().getNode().getId(), + Function.identity())); + // Remove all children and add them back using the node IDs from client + // items + childrenComponents.clear(); + for (int rootLevelItemIdx = 0; rootLevelItemIdx < orderedItemsFromClient + .length(); rootLevelItemIdx++) { + JsonObject rootLevelItemFromClient = orderedItemsFromClient + .getObject(rootLevelItemIdx); + int rootLevelItemNodeId = (int) rootLevelItemFromClient + .getNumber("nodeid"); + Component componentMatch = nodeIdToComponent + .get(rootLevelItemNodeId); + childrenComponents.add(componentMatch); + // Reorder the widgets in sections separately + if (componentMatch instanceof DashboardSection sectionMatch) { + reorderSectionWidgets(sectionMatch, rootLevelItemFromClient); + } + } + } + + private void reorderSectionWidgets(DashboardSection section, + JsonObject rootLevelItem) { + // Keep references to the widgets before clearing them + Map nodeIdToWidget = section.getWidgets() + .stream() + .collect(Collectors.toMap( + widget -> widget.getElement().getNode().getId(), + Function.identity())); + // Remove all widgets and add them back using the node IDs from client + // items + section.removeAll(); + JsonArray sectionWidgetsFromClient = rootLevelItem.getArray("items"); + for (int sectionWidgetIdx = 0; sectionWidgetIdx < sectionWidgetsFromClient + .length(); sectionWidgetIdx++) { + int sectionItemNodeId = (int) sectionWidgetsFromClient + .getObject(sectionWidgetIdx).getNumber("nodeid"); + section.add(nodeIdToWidget.get(sectionItemNodeId)); + } + } + + private boolean areItemsInSync(JsonArray itemsFromClient) { + // Assert root level item count + if (itemsFromClient.length() != getChildren().count()) { + return false; + } + Set allNodeIds = new HashSet<>(); + for (int rootLevelItemIdx = 0; rootLevelItemIdx < itemsFromClient + .length(); rootLevelItemIdx++) { + JsonObject rootLevelItemFromClient = itemsFromClient + .getObject(rootLevelItemIdx); + int rootLevelItemNodeId = (int) rootLevelItemFromClient + .getNumber("nodeid"); + // Assert unique node ID + if (allNodeIds.contains(rootLevelItemNodeId)) { + return false; + } + allNodeIds.add(rootLevelItemNodeId); + Optional childMatch = getChildren().filter(child -> child + .getElement().getNode().getId() == rootLevelItemNodeId) + .findAny(); + // Assert root level item node ID + if (childMatch.isEmpty()) { + return false; + } + // Assert section contents + if (childMatch.get() instanceof DashboardSection sectionMatch + && !areSectionItemsInSync(sectionMatch, + rootLevelItemFromClient, allNodeIds)) { + return false; + } + } + return true; + } + + private boolean areSectionItemsInSync(DashboardSection section, + JsonObject sectionFromClient, Set allNodeIds) { + // Assert section has items array + if (!sectionFromClient.hasKey("items")) { + return false; + } + JsonArray sectionItemsFromClient = sectionFromClient.getArray("items"); + // Assert section child item count + if (sectionItemsFromClient.length() != section.getWidgets().size()) { + return false; + } + for (int sectionItemIdx = 0; sectionItemIdx < sectionItemsFromClient + .length(); sectionItemIdx++) { + int sectionItemNodeId = (int) sectionItemsFromClient + .getObject(sectionItemIdx).getNumber("nodeid"); + // Assert unique node ID + if (allNodeIds.contains(sectionItemNodeId)) { + return false; + } + allNodeIds.add(sectionItemNodeId); + // Assert section child item node ID + if (section.getWidgets().stream().noneMatch(child -> child + .getElement().getNode().getId() == sectionItemNodeId)) { + return false; + } + } + return true; + } + + private void customizeItemReorderEndEvent() { + getElement().executeJs( + """ + this.addEventListener('dashboard-item-reorder-end', (e) => { + const itemsCopy = []; + for (let rootLevelIdx = 0; rootLevelIdx < this.items; rootLevelIdx++) { + const item = this.items[rootLevelIdx]; + const itemCopy = { nodeid: item.nodeid }; + if (item.items) { + const sectionItemsCopy = []; + const sectionItems = item.items; + for (let sectionItemIdx = 0; sectionItemIdx < sectionItems; sectionItemIdx++) { + const sectionItemCopy = { nodeid: sectionItems[sectionItemIdx].nodeid }; + sectionItemsCopy.push(sectionItemCopy); + } + itemCopy.items = sectionItemsCopy; + } + itemsCopy.push(itemCopy); + } + const flowReorderEvent = new CustomEvent('dashboard-item-reorder-end-flow', { + detail: { items: itemsCopy }, + }); + this.dispatchEvent(flowReorderEvent); + });"""); + } } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardItemReorderEndEvent.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardItemReorderEndEvent.java new file mode 100644 index 00000000000..22ff4dbf79e --- /dev/null +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardItemReorderEndEvent.java @@ -0,0 +1,52 @@ +/** + * Copyright 2000-2024 Vaadin Ltd. + * + * This program is available under Vaadin Commercial License and Service Terms. + * + * See {@literal } for the full + * license. + */ +package com.vaadin.flow.component.dashboard; + +import com.vaadin.flow.component.ComponentEvent; +import com.vaadin.flow.component.ComponentEventListener; +import com.vaadin.flow.component.DomEvent; +import com.vaadin.flow.component.EventData; + +import elemental.json.JsonArray; + +/** + * Widget or section reorder end event of {@link Dashboard}. + * + * @author Vaadin Ltd. + * @see Dashboard#addItemReorderEndListener(ComponentEventListener) + */ +@DomEvent("dashboard-item-reorder-end-flow") +public class DashboardItemReorderEndEvent extends ComponentEvent { + + private final JsonArray items; + + /** + * Creates a dashboard item reorder end event. + * + * @param source + * Dashboard that contains the item that was dragged + * @param fromClient + * true if the event originated from the client + * side, false otherwise + */ + public DashboardItemReorderEndEvent(Dashboard source, boolean fromClient, + @EventData("event.detail.items") JsonArray items) { + super(source, fromClient); + this.items = items; + } + + /** + * Returns the ordered items from the client side + * + * @return items the ordered items as a {@link JsonArray} + */ + public JsonArray getItems() { + return items; + } +} diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardItemReorderStartEvent.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardItemReorderStartEvent.java new file mode 100644 index 00000000000..d2910539cfb --- /dev/null +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardItemReorderStartEvent.java @@ -0,0 +1,37 @@ +/** + * Copyright 2000-2024 Vaadin Ltd. + * + * This program is available under Vaadin Commercial License and Service Terms. + * + * See {@literal } for the full + * license. + */ +package com.vaadin.flow.component.dashboard; + +import com.vaadin.flow.component.ComponentEvent; +import com.vaadin.flow.component.ComponentEventListener; +import com.vaadin.flow.component.DomEvent; + +/** + * Widget or section reorder start event of {@link Dashboard}. + * + * @author Vaadin Ltd. + * @see Dashboard#addItemReorderStartListener(ComponentEventListener) + */ +@DomEvent("dashboard-item-reorder-start") +public class DashboardItemReorderStartEvent extends ComponentEvent { + + /** + * Creates a dashboard item reorder start event. + * + * @param source + * Dashboard that contains the item that was dragged + * @param fromClient + * true if the event originated from the client + * side, false otherwise + */ + public DashboardItemReorderStartEvent(Dashboard source, + boolean fromClient) { + super(source, fromClient); + } +} diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropTest.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropTest.java new file mode 100644 index 00000000000..a6bfe99d437 --- /dev/null +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropTest.java @@ -0,0 +1,216 @@ +/** + * Copyright 2000-2024 Vaadin Ltd. + * + * This program is available under Vaadin Commercial License and Service Terms. + * + * See {@literal } for the full + * license. + */ +package com.vaadin.flow.component.dashboard.tests; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.vaadin.flow.component.ComponentUtil; +import com.vaadin.flow.component.dashboard.Dashboard; +import com.vaadin.flow.component.dashboard.DashboardItemReorderEndEvent; +import com.vaadin.flow.component.dashboard.DashboardSection; +import com.vaadin.flow.component.dashboard.DashboardWidget; + +import elemental.json.Json; +import elemental.json.JsonArray; +import elemental.json.JsonObject; + +public class DashboardDragDropTest extends DashboardTestBase { + private Dashboard dashboard; + + private JsonArray itemsArray; + + @Before + @Override + public void setup() { + super.setup(); + dashboard = new Dashboard(); + dashboard.add(new DashboardWidget(), new DashboardWidget()); + DashboardSection section = dashboard.addSection(); + section.add(new DashboardWidget(), new DashboardWidget()); + getUi().add(dashboard); + fakeClientCommunication(); + itemsArray = getItemsArray(dashboard.getChildren().toList()); + } + + @Test + public void reorderWidget_orderIsUpdated() { + assertRootLevelItemReorder(0, 1); + } + + @Test + public void reorderSection_orderIsUpdated() { + assertRootLevelItemReorder(2, 1); + } + + @Test + public void reorderWidgetInSection_orderIsUpdated() { + assertSectionWidgetReorder(2, 0, 1); + } + + @Test + public void invalidNumberOfRootLevelItemsInEvent_itemsAreUnchanged() { + JsonObject newItem = Json.createObject(); + newItem.put("nodeid", 50); + itemsArray.set(itemsArray.length(), newItem); + assertInvalidItemsAreDisregarded(); + } + + @Test + public void invalidNumberOfWidgetsInSectionInEvent_itemsAreUnchanged() { + JsonObject sectionItem = itemsArray.get(2); + JsonArray sectionItems = sectionItem.getArray("items"); + JsonObject newItem = Json.createObject(); + sectionItems.set(sectionItems.length(), newItem); + assertInvalidItemsAreDisregarded(); + } + + @Test + public void invalidRootLevelItemNodeIdInEvent_itemsAreUnchanged() { + JsonObject item = itemsArray.get(0); + item.put("nodeid", -2); + assertInvalidItemsAreDisregarded(); + } + + @Test + public void duplicateRootLevelItemNodeIdInEvent_itemsAreUnchanged() { + JsonObject item1 = itemsArray.get(0); + JsonObject item2 = itemsArray.get(1); + item2.put("nodeid", item1.getNumber("nodeid")); + assertInvalidItemsAreDisregarded(); + } + + @Test + public void invalidWidgetInSectionNodeIdInEvent_itemsAreUnchanged() { + JsonObject sectionItem = itemsArray.get(2); + JsonArray sectionItems = sectionItem.getArray("items"); + JsonObject item = sectionItems.get(0); + item.put("nodeid", -2); + assertInvalidItemsAreDisregarded(); + } + + @Test + public void duplicateWidgetInSectionNodeIdInEvent_itemsAreUnchanged() { + JsonObject sectionItem = itemsArray.get(2); + JsonArray sectionItems = sectionItem.getArray("items"); + JsonObject item1 = sectionItems.get(0); + JsonObject item2 = sectionItems.get(1); + item2.put("nodeid", item1.getNumber("nodeid")); + assertInvalidItemsAreDisregarded(); + } + + @Test + public void duplicateNodeIdForRootLevelItemAndWidgetInSectionInEvent_itemsAreUnchanged() { + JsonObject rootLevelItem = itemsArray.get(0); + JsonObject sectionItem = itemsArray.get(2); + JsonArray sectionItems = sectionItem.getArray("items"); + JsonObject itemInSection = sectionItems.get(0); + itemInSection.put("nodeid", rootLevelItem.getNumber("nodeid")); + assertInvalidItemsAreDisregarded(); + } + + private void fireItemReorderEndEvent() { + ComponentUtil.fireEvent(dashboard, + new DashboardItemReorderEndEvent(dashboard, false, itemsArray)); + } + + private List getSectionWidgetNodeIds(int sectionIndex) { + DashboardSection section = (DashboardSection) dashboard.getChildren() + .toList().get(sectionIndex); + return section.getWidgets().stream() + .map(component -> component.getElement().getNode().getId()) + .collect(Collectors.toCollection(ArrayList::new)); + } + + private List getRootLevelNodeIds() { + return dashboard.getChildren() + .map(component -> component.getElement().getNode().getId()) + .collect(Collectors.toCollection(ArrayList::new)); + } + + private List getExpectedSectionWidgetNodeIds(int sectionIndex, + int initialIndex, int finalIndex) { + List expectedSectionWidgetNodeIds = getSectionWidgetNodeIds( + sectionIndex); + int nodeId = expectedSectionWidgetNodeIds.get(initialIndex); + expectedSectionWidgetNodeIds.remove((Object) nodeId); + expectedSectionWidgetNodeIds.add(finalIndex, nodeId); + return expectedSectionWidgetNodeIds; + } + + private List getExpectedRootLevelItemNodeIds(int initialIndex, + int finalIndex) { + List expectedRootLevelNodeIds = getRootLevelNodeIds(); + int nodeId = expectedRootLevelNodeIds.get(initialIndex); + expectedRootLevelNodeIds.remove((Object) nodeId); + expectedRootLevelNodeIds.add(finalIndex, nodeId); + return expectedRootLevelNodeIds; + } + + private void reorderSectionWidget(int sectionIndex, int initialIndex, + int finalIndex) { + JsonObject sectionItem = itemsArray.get(sectionIndex); + JsonArray sectionItems = sectionItem.getArray("items"); + sectionItem.put("items", + reorderItemInJsonArray(initialIndex, finalIndex, sectionItems)); + } + + private void reorderRootLevelItem(int initialIndex, int finalIndex) { + itemsArray = reorderItemInJsonArray(initialIndex, finalIndex, + itemsArray); + } + + private void assertSectionWidgetReorder(int sectionIndex, int initialIndex, + int finalIndex) { + reorderSectionWidget(sectionIndex, initialIndex, finalIndex); + List expectedSectionWidgetNodeIds = getExpectedSectionWidgetNodeIds( + sectionIndex, initialIndex, finalIndex); + fireItemReorderEndEvent(); + Assert.assertEquals(expectedSectionWidgetNodeIds, + getSectionWidgetNodeIds(sectionIndex)); + } + + private void assertRootLevelItemReorder(int initialIndex, int finalIndex) { + + reorderRootLevelItem(initialIndex, finalIndex); + List expectedRootLevelNodeIds = getExpectedRootLevelItemNodeIds( + initialIndex, finalIndex); + fireItemReorderEndEvent(); + Assert.assertEquals(expectedRootLevelNodeIds, getRootLevelNodeIds()); + } + + private void assertInvalidItemsAreDisregarded() { + reorderRootLevelItem(0, 1); + List initialRootLevelNodeIds = getRootLevelNodeIds(); + fireItemReorderEndEvent(); + Assert.assertEquals(initialRootLevelNodeIds, getRootLevelNodeIds()); + } + + private static JsonArray reorderItemInJsonArray(int initialIndex, + int finalIndex, JsonArray initialArray) { + JsonObject itemToMove = initialArray.get(initialIndex); + initialArray.remove(initialIndex); + JsonArray newArray = Json.createArray(); + for (int i = 0; i < finalIndex; i++) { + JsonObject currentItem = initialArray.get(i); + newArray.set(i, currentItem); + } + newArray.set(finalIndex, itemToMove); + for (int i = finalIndex; i < initialArray.length(); i++) { + JsonObject currentItem = initialArray.get(i); + newArray.set(i + 1, currentItem); + } + return newArray; + } +} 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 c48d59c39eb..639401b9438 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,44 +8,27 @@ */ package com.vaadin.flow.component.dashboard.tests; -import java.util.ArrayList; -import java.util.Arrays; -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.DashboardSection; import com.vaadin.flow.component.dashboard.DashboardWidget; import com.vaadin.flow.component.html.Div; -import com.vaadin.flow.server.VaadinSession; -public class DashboardTest { - private final UI ui = new UI(); +public class DashboardTest extends DashboardTestBase { private Dashboard dashboard; @Before + @Override public void setup() { - UI.setCurrent(ui); - VaadinSession session = Mockito.mock(VaadinSession.class); - Mockito.when(session.hasLock()).thenReturn(true); - ui.getInternals().setSession(session); + super.setup(); dashboard = new Dashboard(); - ui.add(dashboard); + getUi().add(dashboard); fakeClientCommunication(); } - @After - public void tearDown() { - UI.setCurrent(null); - } - @Test public void addWidget_widgetIsAdded() { DashboardWidget widget1 = new DashboardWidget(); @@ -176,7 +159,7 @@ public void addWidgetsSeparately_removeOneFromParent_widgetIsRemoved() { @Test public void addWidgetFromLayoutToDashboard_widgetIsMoved() { Div parent = new Div(); - ui.add(parent); + getUi().add(parent); DashboardWidget widget = new DashboardWidget(); parent.add(widget); fakeClientCommunication(); @@ -192,7 +175,7 @@ public void addWidgetFromDashboardToLayout_widgetIsMoved() { dashboard.add(widget); fakeClientCommunication(); Div parent = new Div(); - ui.add(parent); + getUi().add(parent); parent.add(widget); fakeClientCommunication(); assertChildComponents(dashboard); @@ -205,7 +188,7 @@ public void addWidgetToAnotherDashboard_widgetIsMoved() { dashboard.add(widget); fakeClientCommunication(); Dashboard newDashboard = new Dashboard(); - ui.add(newDashboard); + getUi().add(newDashboard); newDashboard.add(widget); fakeClientCommunication(); assertChildComponents(dashboard); @@ -545,7 +528,7 @@ public void addWidgetsSeparatelyToSection_removeOneFromParent_widgetIsRemoved() public void addWidgetFromLayoutToSection_widgetIsMoved() { DashboardSection section = dashboard.addSection(); Div parent = new Div(); - ui.add(parent); + getUi().add(parent); DashboardWidget widget = new DashboardWidget(); parent.add(widget); fakeClientCommunication(); @@ -563,7 +546,7 @@ public void addWidgetFromSectionToLayout_widgetIsMoved() { section.add(widget); fakeClientCommunication(); Div parent = new Div(); - ui.add(parent); + getUi().add(parent); parent.add(widget); fakeClientCommunication(); assertSectionWidgets(section); @@ -795,40 +778,21 @@ public void setGapNull_valueIsCorrectlyRetrieved() { Assert.assertNull(dashboard.getGap()); } - private void fakeClientCommunication() { - ui.getInternals().getStateTree().runExecutionsBeforeClientResponse(); - ui.getInternals().getStateTree().collectChanges(ignore -> { - }); - } - - private static void assertChildComponents(Dashboard dashboard, - Component... expectedChildren) { - List expectedWidgets = getExpectedWidgets( - expectedChildren); - Assert.assertEquals(expectedWidgets, dashboard.getWidgets()); - Assert.assertEquals(Arrays.asList(expectedChildren), - dashboard.getChildren().toList()); - } - - private static List getExpectedWidgets( - Component... expectedChildren) { - List expectedWidgets = new ArrayList<>(); - for (Component child : expectedChildren) { - if (child instanceof DashboardSection section) { - expectedWidgets.addAll(section.getWidgets()); - } else if (child instanceof DashboardWidget widget) { - expectedWidgets.add(widget); - } else { - throw new IllegalArgumentException( - "A dashboard can only contain widgets or sections."); - } - } - return expectedWidgets; + @Test + public void dashboardIsEditableByDefault() { + Assert.assertTrue(dashboard.isEditable()); } - private static void assertSectionWidgets(DashboardSection section, - DashboardWidget... expectedWidgets) { - Assert.assertEquals(Arrays.asList(expectedWidgets), - section.getWidgets()); + @Test + public void setEditableFalse_valueIsCorrectlyRetrieved() { + dashboard.setEditable(false); + Assert.assertFalse(dashboard.isEditable()); + } + + @Test + public void setEditableTrue_valueIsCorrectlyRetrieved() { + dashboard.setEditable(false); + dashboard.setEditable(true); + Assert.assertTrue(dashboard.isEditable()); } } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTestBase.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTestBase.java new file mode 100644 index 00000000000..cc72fad938d --- /dev/null +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardTestBase.java @@ -0,0 +1,111 @@ +/** + * Copyright 2000-2024 Vaadin Ltd. + * + * This program is available under Vaadin Commercial License and Service Terms. + * + * See {@literal } for the full + * license. + */ +package com.vaadin.flow.component.dashboard.tests; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +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.DashboardSection; +import com.vaadin.flow.component.dashboard.DashboardWidget; +import com.vaadin.flow.server.VaadinSession; + +import elemental.json.Json; +import elemental.json.JsonArray; +import elemental.json.JsonObject; + +public class DashboardTestBase { + + 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); + fakeClientCommunication(); + } + + @After + public void tearDown() { + UI.setCurrent(null); + } + + protected UI getUi() { + return ui; + } + + protected void fakeClientCommunication() { + ui.getInternals().getStateTree().runExecutionsBeforeClientResponse(); + ui.getInternals().getStateTree().collectChanges(ignore -> { + }); + } + + protected static JsonArray getItemsArray( + List rootLevelComponents) { + JsonArray itemsArray = Json.createArray(); + rootLevelComponents.forEach(child -> { + JsonObject rootLevelItem = Json.createObject(); + rootLevelItem.put("nodeid", child.getElement().getNode().getId()); + if (child instanceof DashboardSection section) { + JsonArray sectionItemsArray = Json.createArray(); + section.getWidgets().forEach(widget -> { + JsonObject sectionItem = Json.createObject(); + sectionItem.put("nodeid", + widget.getElement().getNode().getId()); + sectionItemsArray.set(sectionItemsArray.length(), + sectionItem); + }); + rootLevelItem.put("items", sectionItemsArray); + } + itemsArray.set(itemsArray.length(), rootLevelItem); + }); + return itemsArray; + } + + protected static void assertChildComponents(Dashboard dashboard, + Component... expectedChildren) { + List expectedWidgets = getExpectedWidgets( + expectedChildren); + Assert.assertEquals(expectedWidgets, dashboard.getWidgets()); + Assert.assertEquals(Arrays.asList(expectedChildren), + dashboard.getChildren().toList()); + } + + protected static List getExpectedWidgets( + Component... expectedChildren) { + List expectedWidgets = new ArrayList<>(); + for (Component child : expectedChildren) { + if (child instanceof DashboardSection section) { + expectedWidgets.addAll(section.getWidgets()); + } else if (child instanceof DashboardWidget widget) { + expectedWidgets.add(widget); + } else { + throw new IllegalArgumentException( + "A dashboard can only contain widgets or sections."); + } + } + return expectedWidgets; + } + + protected static void assertSectionWidgets(DashboardSection section, + DashboardWidget... expectedWidgets) { + Assert.assertEquals(Arrays.asList(expectedWidgets), + section.getWidgets()); + } +} 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 d5be4ef5180..a13d5f7049a 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,39 +8,41 @@ */ 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.component.html.Span; -import com.vaadin.flow.server.VaadinSession; -public class DashboardWidgetTest { +public class DashboardWidgetTest extends DashboardTestBase { - private final UI ui = new UI(); + @Test + public void assertDefaultTitle() { + DashboardWidget widget = new DashboardWidget(); + Assert.assertNull(widget.getTitle()); + } - @Before - public void setup() { - UI.setCurrent(ui); - VaadinSession session = Mockito.mock(VaadinSession.class); - Mockito.when(session.hasLock()).thenReturn(true); - ui.getInternals().setSession(session); + @Test + public void setTitle_returnsCorrectTitle() { + String valueToSet = "New title"; + DashboardWidget widget = new DashboardWidget(); + widget.setTitle(valueToSet); + Assert.assertEquals(valueToSet, widget.getTitle()); } - @After - public void tearDown() { - UI.setCurrent(null); + @Test + public void setTitleNull_returnsEmptyTitle() { + DashboardWidget widget = new DashboardWidget(); + widget.setTitle("New title"); + widget.setTitle(null); + Assert.assertNull(widget.getTitle()); } @Test public void addWidgetToLayout_widgetIsAdded() { Div layout = new Div(); - ui.add(layout); + getUi().add(layout); DashboardWidget widget = new DashboardWidget(); layout.add(widget); fakeClientCommunication(); @@ -50,7 +52,7 @@ public void addWidgetToLayout_widgetIsAdded() { @Test public void removeWidgetFromLayout_widgetIsRemoved() { Div layout = new Div(); - ui.add(layout); + getUi().add(layout); DashboardWidget widget = new DashboardWidget(); layout.add(widget); fakeClientCommunication(); @@ -62,7 +64,7 @@ public void removeWidgetFromLayout_widgetIsRemoved() { @Test public void addWidgetToLayout_removeFromParent_widgetIsRemoved() { Div layout = new Div(); - ui.add(layout); + getUi().add(layout); DashboardWidget widget = new DashboardWidget(); layout.add(widget); fakeClientCommunication(); @@ -74,12 +76,12 @@ public void addWidgetToLayout_removeFromParent_widgetIsRemoved() { @Test public void addWidgetFromLayoutToAnotherLayout_widgetIsMoved() { Div parent = new Div(); - ui.add(parent); + getUi().add(parent); DashboardWidget widget = new DashboardWidget(); parent.add(widget); fakeClientCommunication(); Div newParent = new Div(); - ui.add(newParent); + getUi().add(newParent); newParent.add(widget); fakeClientCommunication(); Assert.assertTrue(parent.getChildren().noneMatch(widget::equals)); @@ -247,10 +249,4 @@ public void setContentToWidgetWithHeader_contentAndHeaderCorrectlyRetrieved() { Assert.assertEquals(content, widget.getContent()); Assert.assertEquals(header, widget.getHeader()); } - - private void fakeClientCommunication() { - ui.getInternals().getStateTree().runExecutionsBeforeClientResponse(); - ui.getInternals().getStateTree().collectChanges(ignore -> { - }); - } } From 780d2b29cede09619e2fdbb5008fa8bc9646ba02 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 10 Sep 2024 19:50:38 +0300 Subject: [PATCH 02/11] chore: run formatter --- .../java/com/vaadin/flow/component/dashboard/Dashboard.java | 3 ++- 1 file changed, 2 insertions(+), 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 361a1074c13..c47fdb9513c 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 @@ -395,7 +395,8 @@ private void updateClientItems() { private static String getWidgetRepresentation(DashboardWidget widget, int itemIndex) { return "{ component: $%d, colspan: %d, rowspan: %d, nodeid: %d }" - .formatted(itemIndex, widget.getColspan(), widget.getRowspan(), widget.getElement().getNode().getId()); + .formatted(itemIndex, widget.getColspan(), widget.getRowspan(), + widget.getElement().getNode().getId()); } private void doRemoveAll() { From 1e6a911c9a1aa8505e9f75d5cf4438f81f06487e Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Wed, 11 Sep 2024 19:08:44 +0300 Subject: [PATCH 03/11] test: fix unit test assertion --- .../flow/component/dashboard/tests/DashboardWidgetTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a13d5f7049a..91924e1f74d 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 @@ -36,7 +36,7 @@ public void setTitleNull_returnsEmptyTitle() { DashboardWidget widget = new DashboardWidget(); widget.setTitle("New title"); widget.setTitle(null); - Assert.assertNull(widget.getTitle()); + Assert.assertEquals("", widget.getTitle()); } @Test From 67de7919c683fe7cc2f2c2f743fcf8d90be98292 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 12 Sep 2024 00:09:17 +0300 Subject: [PATCH 04/11] test: update test route names to align with others --- .../flow/component/dashboard/tests/DashboardSectionPage.java | 2 +- .../flow/component/dashboard/tests/DashboardWidgetPage.java | 2 +- .../flow/component/dashboard/tests/DashboardSectionIT.java | 2 +- .../flow/component/dashboard/tests/DashboardWidgetIT.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardSectionPage.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardSectionPage.java index 79f882d0b68..5833a39dcd2 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardSectionPage.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardSectionPage.java @@ -21,7 +21,7 @@ /** * @author Vaadin Ltd */ -@Route("vaadin-dashboard-section") +@Route("vaadin-dashboard/section") public class DashboardSectionPage extends Div { public DashboardSectionPage() { diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetPage.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetPage.java index 27eb5c834e5..749963a6e5f 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetPage.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetPage.java @@ -20,7 +20,7 @@ /** * @author Vaadin Ltd */ -@Route("vaadin-dashboard-widget") +@Route("vaadin-dashboard/widget") public class DashboardWidgetPage extends Div { public DashboardWidgetPage() { diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardSectionIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardSectionIT.java index 31e275b8561..171d2eec48c 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardSectionIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardSectionIT.java @@ -24,7 +24,7 @@ /** * @author Vaadin Ltd */ -@TestPath("vaadin-dashboard-section") +@TestPath("vaadin-dashboard/section") public class DashboardSectionIT extends AbstractComponentIT { private DashboardElement dashboardElement; diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetIT.java index 02d31dee991..e3455bd93ac 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardWidgetIT.java @@ -22,7 +22,7 @@ /** * @author Vaadin Ltd */ -@TestPath("vaadin-dashboard-widget") +@TestPath("vaadin-dashboard/widget") public class DashboardWidgetIT extends AbstractComponentIT { private DashboardElement dashboardElement; From 8f319f32921bd1eb5506c6bb7834eb45cf790571 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 12 Sep 2024 00:09:56 +0300 Subject: [PATCH 05/11] test: add its that test the server client communication --- .../tests/DashboardDragDropPage.java | 55 ++++++++++++ .../dashboard/tests/DashboardDragDropIT.java | 86 +++++++++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java create mode 100644 vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java new file mode 100644 index 00000000000..4c34e5b47d2 --- /dev/null +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java @@ -0,0 +1,55 @@ +/** + * Copyright 2000-2024 Vaadin Ltd. + * + * This program is available under Vaadin Commercial License and Service Terms. + * + * See {@literal } for the full + * license. + */ +package com.vaadin.flow.component.dashboard.tests; + +import com.vaadin.flow.component.dashboard.Dashboard; +import com.vaadin.flow.component.dashboard.DashboardSection; +import com.vaadin.flow.component.dashboard.DashboardWidget; +import com.vaadin.flow.component.html.Div; +import com.vaadin.flow.router.Route; + +/** + * @author Vaadin Ltd + */ +@Route("vaadin-dashboard/drag-drop") +public class DashboardDragDropPage extends Div { + + public DashboardDragDropPage() { + Dashboard dashboard = new Dashboard(); + + DashboardWidget widget1 = new DashboardWidget(); + widget1.setTitle("Widget 1"); + + DashboardWidget widget2 = new DashboardWidget(); + widget2.setTitle("Widget 2"); + + dashboard.add(widget1, widget2); + + DashboardWidget widget1InSection1 = new DashboardWidget(); + widget1InSection1.setTitle("Widget 1 in Section 1"); + + DashboardWidget widget2InSection1 = new DashboardWidget(); + widget2InSection1.setTitle("Widget 2 in Section 1"); + + DashboardSection section1 = new DashboardSection("Section 1"); + section1.add(widget1InSection1, widget2InSection1); + + dashboard.addSection(section1); + + DashboardWidget widgetInSection2 = new DashboardWidget(); + widgetInSection2.setTitle("Widget in Section 2"); + + DashboardSection section2 = new DashboardSection("Section 2"); + section2.add(widgetInSection2); + + dashboard.addSection(section2); + + add(dashboard); + } +} diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java new file mode 100644 index 00000000000..70bca210d8b --- /dev/null +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java @@ -0,0 +1,86 @@ +/** + * Copyright 2000-2024 Vaadin Ltd. + * + * This program is available under Vaadin Commercial License and Service Terms. + * + * See {@literal } for the full + * license. + */ +package com.vaadin.flow.component.dashboard.tests; + +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.DashboardSectionElement; +import com.vaadin.flow.component.dashboard.testbench.DashboardWidgetElement; +import com.vaadin.flow.testutil.TestPath; +import com.vaadin.tests.AbstractComponentIT; + +/** + * @author Vaadin Ltd + */ +@TestPath("vaadin-dashboard/drag-drop") +public class DashboardDragDropIT extends AbstractComponentIT { + + private DashboardElement dashboardElement; + + @Before + public void init() { + open(); + dashboardElement = $(DashboardElement.class).waitForFirst(); + } + + @Test + public void reorderWidgetOnClientSide_itemsAreReorderedCorrectly() { + DashboardWidgetElement widgetToReorder = dashboardElement.getWidgets() + .get(0); + reorderRootLevelItem(1, 0); + Assert.assertEquals(widgetToReorder.getTitle(), + dashboardElement.getWidgets().get(1).getTitle()); + } + + @Test + public void reorderSectionOnClientSide_itemsAreReorderedCorrectly() { + DashboardSectionElement sectionToReorder = dashboardElement + .getSections().get(0); + reorderRootLevelItem(2, 3); + Assert.assertEquals(sectionToReorder.getTitle(), + dashboardElement.getSections().get(1).getTitle()); + } + + @Test + public void reorderWidgetInSectionOnClientSide_itemsAreReorderedCorrectly() { + DashboardSectionElement firstSection = dashboardElement.getSections() + .get(0); + DashboardWidgetElement widgetToReorder = firstSection.getWidgets() + .get(1); + reorderWidgetInSection(2, 0, 1); + firstSection = dashboardElement.getSections().get(0); + Assert.assertEquals(widgetToReorder.getTitle(), + firstSection.getWidgets().get(0).getTitle()); + } + + private void reorderWidgetInSection(int sectionIndex, int initialIndex, + int targetIndex) { + executeScript( + """ + const sectionIndex = %d; + const reorderedItem = arguments[0].items[sectionIndex].items.splice(%d, 1)[0]; + arguments[0].items[sectionIndex].items.splice(%d, 0, reorderedItem); + arguments[0].dispatchEvent(new CustomEvent('dashboard-item-reorder-end'));""" + .formatted(sectionIndex, initialIndex, targetIndex), + dashboardElement); + } + + private void reorderRootLevelItem(int initialIndex, int targetIndex) { + executeScript( + """ + const reorderedItem = arguments[0].items.splice(%d, 1)[0]; + arguments[0].items.splice(%d, 0, reorderedItem); + arguments[0].dispatchEvent(new CustomEvent('dashboard-item-reorder-end'));""" + .formatted(initialIndex, targetIndex), + dashboardElement); + } +} From bc9e0e529d693bc38b26c595a6726179c6990c6c Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 12 Sep 2024 00:10:32 +0300 Subject: [PATCH 06/11] refactor: simplify js event workaround --- .../flow/component/dashboard/Dashboard.java | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 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 c47fdb9513c..eb5e844da8e 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 @@ -574,23 +574,14 @@ private void customizeItemReorderEndEvent() { getElement().executeJs( """ this.addEventListener('dashboard-item-reorder-end', (e) => { - const itemsCopy = []; - for (let rootLevelIdx = 0; rootLevelIdx < this.items; rootLevelIdx++) { - const item = this.items[rootLevelIdx]; - const itemCopy = { nodeid: item.nodeid }; - if (item.items) { - const sectionItemsCopy = []; - const sectionItems = item.items; - for (let sectionItemIdx = 0; sectionItemIdx < sectionItems; sectionItemIdx++) { - const sectionItemCopy = { nodeid: sectionItems[sectionItemIdx].nodeid }; - sectionItemsCopy.push(sectionItemCopy); - } - itemCopy.items = sectionItemsCopy; - } - itemsCopy.push(itemCopy); + function mapItems(items) { + return items.map(({nodeid, items}) => ({ + nodeid, + ...(items && { items: mapItems(items) }) + })); } const flowReorderEvent = new CustomEvent('dashboard-item-reorder-end-flow', { - detail: { items: itemsCopy }, + detail: { items: mapItems(this.items) } }); this.dispatchEvent(flowReorderEvent); });"""); From ed57ef92daad06c8a5211bbef1f7a80b1592adbd Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 12 Sep 2024 11:01:01 +0300 Subject: [PATCH 07/11] fix: do not remove widgets on detach --- .../tests/DashboardDragDropPage.java | 12 ++++++++- .../dashboard/tests/DashboardDragDropIT.java | 8 ++++++ .../flow/component/dashboard/Dashboard.java | 10 ++------ .../DashboardChildDetachHandler.java | 25 +++++++++++++++---- .../component/dashboard/DashboardSection.java | 8 +----- .../dashboard/tests/DashboardTest.java | 20 +++++++++++++++ 6 files changed, 62 insertions(+), 21 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java index 4c34e5b47d2..be491d79504 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java @@ -12,6 +12,7 @@ import com.vaadin.flow.component.dashboard.DashboardSection; 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; /** @@ -50,6 +51,15 @@ public DashboardDragDropPage() { dashboard.addSection(section2); - add(dashboard); + NativeButton toggleAttached = new NativeButton("Toggle attached", e -> { + if (dashboard.getParent().isPresent()) { + dashboard.removeFromParent(); + } else { + add(dashboard); + } + }); + toggleAttached.setId("toggle-attached"); + + add(toggleAttached, dashboard); } } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java index 70bca210d8b..c8d751db49b 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java @@ -62,6 +62,14 @@ public void reorderWidgetInSectionOnClientSide_itemsAreReorderedCorrectly() { firstSection.getWidgets().get(0).getTitle()); } + @Test + public void detachReattach_reorderWidgetOnClientSide_itemsAreReorderedCorrectly() { + clickElementWithJs("toggle-attached"); + clickElementWithJs("toggle-attached"); + dashboardElement = $(DashboardElement.class).waitForFirst(); + reorderWidgetOnClientSide_itemsAreReorderedCorrectly(); + } + private void reorderWidgetInSection(int sectionIndex, int initialIndex, int targetIndex) { executeScript( 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 eb5e844da8e..70a4d258b35 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 @@ -9,7 +9,6 @@ package com.vaadin.flow.component.dashboard; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -58,7 +57,6 @@ public class Dashboard extends Component implements HasWidgets { */ public Dashboard() { childDetachHandler = getChildDetachHandler(); - customizeItemReorderEndEvent(); setEditable(true); addItemReorderEndListener(this::onItemReorderEnd); } @@ -341,6 +339,7 @@ protected void onAttach(AttachEvent attachEvent) { super.onAttach(attachEvent); getElement().executeJs( "Vaadin.FlowComponentHost.patchVirtualContainer(this);"); + customizeItemReorderEndEvent(); doUpdateClient(); } @@ -435,17 +434,12 @@ private void doRemoveSection(DashboardSection section) { } private DashboardChildDetachHandler getChildDetachHandler() { - return new DashboardChildDetachHandler() { + return new DashboardChildDetachHandler(this) { @Override void removeChild(Component child) { childrenComponents.remove(child); updateClient(); } - - @Override - Collection getDirectChildren() { - return Dashboard.this.getChildren().toList(); - } }; } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardChildDetachHandler.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardChildDetachHandler.java index 64cda277be1..ae5dc345078 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardChildDetachHandler.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardChildDetachHandler.java @@ -8,7 +8,6 @@ */ package com.vaadin.flow.component.dashboard; -import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.Objects; @@ -17,17 +16,27 @@ import com.vaadin.flow.dom.Element; import com.vaadin.flow.dom.ElementDetachEvent; import com.vaadin.flow.dom.ElementDetachListener; +import com.vaadin.flow.internal.NullOwner; import com.vaadin.flow.shared.Registration; public abstract class DashboardChildDetachHandler implements ElementDetachListener { + private final Component component; + private final Map childDetachListenerMap = new HashMap<>(); + DashboardChildDetachHandler(Component component) { + this.component = component; + } + @Override public void onDetach(ElementDetachEvent e) { + if (isComponentDetaching()) { + return; + } var detachedElement = e.getSource(); - getDirectChildren().stream() + component.getChildren() .filter(childComponent -> Objects.equals(detachedElement, childComponent.getElement())) .findAny().ifPresent(detachedChild -> { @@ -42,8 +51,16 @@ public void onDetach(ElementDetachEvent e) { }); } + private boolean isComponentDetaching() { + return component.isAttached() + && !NullOwner.get() + .equals(component.getElement().getNode().getOwner()) + && !component.getElement().getNode().getOwner() + .hasNode(component.getElement().getNode()); + } + void refreshListeners() { - getDirectChildren().forEach(child -> { + component.getChildren().forEach(child -> { Element childElement = child.getElement(); if (!childDetachListenerMap.containsKey(childElement)) { childDetachListenerMap.put(childElement, @@ -53,6 +70,4 @@ void refreshListeners() { } abstract void removeChild(Component child); - - abstract Collection getDirectChildren(); } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardSection.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardSection.java index f899904b324..a3064abfe1b 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardSection.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardSection.java @@ -9,7 +9,6 @@ package com.vaadin.flow.component.dashboard; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Objects; @@ -177,17 +176,12 @@ void updateClient() { } private DashboardChildDetachHandler getChildDetachHandler() { - return new DashboardChildDetachHandler() { + return new DashboardChildDetachHandler(this) { @Override void removeChild(Component child) { widgets.remove(child); updateClient(); } - - @Override - Collection getDirectChildren() { - return DashboardSection.this.getChildren().toList(); - } }; } } 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 639401b9438..446d0f1fd6e 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 @@ -795,4 +795,24 @@ public void setEditableTrue_valueIsCorrectlyRetrieved() { dashboard.setEditable(true); Assert.assertTrue(dashboard.isEditable()); } + + @Test + public void addWidget_detachDashboard_widgetIsRetained() { + DashboardWidget widget = new DashboardWidget(); + dashboard.add(widget); + fakeClientCommunication(); + getUi().remove(dashboard); + fakeClientCommunication(); + assertChildComponents(dashboard, widget); + } + + @Test + public void detachDashboard_addWidget_reattachDashboard_widgetIsAdded() { + getUi().remove(dashboard); + fakeClientCommunication(); + DashboardWidget widget = new DashboardWidget(); + dashboard.add(widget); + fakeClientCommunication(); + assertChildComponents(dashboard, widget); + } } From 32c2a3b4979ce486e4a6f10c1e245232cdbfea32 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 12 Sep 2024 11:52:02 +0300 Subject: [PATCH 08/11] refactor: remove item sanitization on drag reorder --- .../flow/component/dashboard/Dashboard.java | 73 +------------------ .../tests/DashboardDragDropTest.java | 68 ----------------- 2 files changed, 1 insertion(+), 140 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 70a4d258b35..aff7e92d814 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,12 +10,9 @@ import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; -import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.stream.Collectors; @@ -447,12 +444,7 @@ private void onItemReorderEnd( DashboardItemReorderEndEvent dashboardItemReorderEndEvent) { JsonArray orderedItemsFromClient = dashboardItemReorderEndEvent .getItems(); - if (areItemsInSync(orderedItemsFromClient)) { - reorderItems(orderedItemsFromClient); - } else { - LoggerFactory.getLogger(getClass()) - .debug("Dashboard items do not match those on the server."); - } + reorderItems(orderedItemsFromClient); updateClient(); } @@ -501,69 +493,6 @@ private void reorderSectionWidgets(DashboardSection section, } } - private boolean areItemsInSync(JsonArray itemsFromClient) { - // Assert root level item count - if (itemsFromClient.length() != getChildren().count()) { - return false; - } - Set allNodeIds = new HashSet<>(); - for (int rootLevelItemIdx = 0; rootLevelItemIdx < itemsFromClient - .length(); rootLevelItemIdx++) { - JsonObject rootLevelItemFromClient = itemsFromClient - .getObject(rootLevelItemIdx); - int rootLevelItemNodeId = (int) rootLevelItemFromClient - .getNumber("nodeid"); - // Assert unique node ID - if (allNodeIds.contains(rootLevelItemNodeId)) { - return false; - } - allNodeIds.add(rootLevelItemNodeId); - Optional childMatch = getChildren().filter(child -> child - .getElement().getNode().getId() == rootLevelItemNodeId) - .findAny(); - // Assert root level item node ID - if (childMatch.isEmpty()) { - return false; - } - // Assert section contents - if (childMatch.get() instanceof DashboardSection sectionMatch - && !areSectionItemsInSync(sectionMatch, - rootLevelItemFromClient, allNodeIds)) { - return false; - } - } - return true; - } - - private boolean areSectionItemsInSync(DashboardSection section, - JsonObject sectionFromClient, Set allNodeIds) { - // Assert section has items array - if (!sectionFromClient.hasKey("items")) { - return false; - } - JsonArray sectionItemsFromClient = sectionFromClient.getArray("items"); - // Assert section child item count - if (sectionItemsFromClient.length() != section.getWidgets().size()) { - return false; - } - for (int sectionItemIdx = 0; sectionItemIdx < sectionItemsFromClient - .length(); sectionItemIdx++) { - int sectionItemNodeId = (int) sectionItemsFromClient - .getObject(sectionItemIdx).getNumber("nodeid"); - // Assert unique node ID - if (allNodeIds.contains(sectionItemNodeId)) { - return false; - } - allNodeIds.add(sectionItemNodeId); - // Assert section child item node ID - if (section.getWidgets().stream().noneMatch(child -> child - .getElement().getNode().getId() == sectionItemNodeId)) { - return false; - } - } - return true; - } - private void customizeItemReorderEndEvent() { getElement().executeJs( """ diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropTest.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropTest.java index a6bfe99d437..a1924235508 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropTest.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropTest.java @@ -59,67 +59,6 @@ public void reorderWidgetInSection_orderIsUpdated() { assertSectionWidgetReorder(2, 0, 1); } - @Test - public void invalidNumberOfRootLevelItemsInEvent_itemsAreUnchanged() { - JsonObject newItem = Json.createObject(); - newItem.put("nodeid", 50); - itemsArray.set(itemsArray.length(), newItem); - assertInvalidItemsAreDisregarded(); - } - - @Test - public void invalidNumberOfWidgetsInSectionInEvent_itemsAreUnchanged() { - JsonObject sectionItem = itemsArray.get(2); - JsonArray sectionItems = sectionItem.getArray("items"); - JsonObject newItem = Json.createObject(); - sectionItems.set(sectionItems.length(), newItem); - assertInvalidItemsAreDisregarded(); - } - - @Test - public void invalidRootLevelItemNodeIdInEvent_itemsAreUnchanged() { - JsonObject item = itemsArray.get(0); - item.put("nodeid", -2); - assertInvalidItemsAreDisregarded(); - } - - @Test - public void duplicateRootLevelItemNodeIdInEvent_itemsAreUnchanged() { - JsonObject item1 = itemsArray.get(0); - JsonObject item2 = itemsArray.get(1); - item2.put("nodeid", item1.getNumber("nodeid")); - assertInvalidItemsAreDisregarded(); - } - - @Test - public void invalidWidgetInSectionNodeIdInEvent_itemsAreUnchanged() { - JsonObject sectionItem = itemsArray.get(2); - JsonArray sectionItems = sectionItem.getArray("items"); - JsonObject item = sectionItems.get(0); - item.put("nodeid", -2); - assertInvalidItemsAreDisregarded(); - } - - @Test - public void duplicateWidgetInSectionNodeIdInEvent_itemsAreUnchanged() { - JsonObject sectionItem = itemsArray.get(2); - JsonArray sectionItems = sectionItem.getArray("items"); - JsonObject item1 = sectionItems.get(0); - JsonObject item2 = sectionItems.get(1); - item2.put("nodeid", item1.getNumber("nodeid")); - assertInvalidItemsAreDisregarded(); - } - - @Test - public void duplicateNodeIdForRootLevelItemAndWidgetInSectionInEvent_itemsAreUnchanged() { - JsonObject rootLevelItem = itemsArray.get(0); - JsonObject sectionItem = itemsArray.get(2); - JsonArray sectionItems = sectionItem.getArray("items"); - JsonObject itemInSection = sectionItems.get(0); - itemInSection.put("nodeid", rootLevelItem.getNumber("nodeid")); - assertInvalidItemsAreDisregarded(); - } - private void fireItemReorderEndEvent() { ComponentUtil.fireEvent(dashboard, new DashboardItemReorderEndEvent(dashboard, false, itemsArray)); @@ -190,13 +129,6 @@ private void assertRootLevelItemReorder(int initialIndex, int finalIndex) { Assert.assertEquals(expectedRootLevelNodeIds, getRootLevelNodeIds()); } - private void assertInvalidItemsAreDisregarded() { - reorderRootLevelItem(0, 1); - List initialRootLevelNodeIds = getRootLevelNodeIds(); - fireItemReorderEndEvent(); - Assert.assertEquals(initialRootLevelNodeIds, getRootLevelNodeIds()); - } - private static JsonArray reorderItemInJsonArray(int initialIndex, int finalIndex, JsonArray initialArray) { JsonObject itemToMove = initialArray.get(initialIndex); From 7217a2357e923e68fb8b998d0d6c5e44d24227f4 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 12 Sep 2024 12:17:54 +0300 Subject: [PATCH 09/11] refactor: check children of element and remove workaround --- .../DashboardChildDetachHandler.java | 35 ++++++------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardChildDetachHandler.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardChildDetachHandler.java index ae5dc345078..b8ff68b842f 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardChildDetachHandler.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/DashboardChildDetachHandler.java @@ -16,7 +16,6 @@ import com.vaadin.flow.dom.Element; import com.vaadin.flow.dom.ElementDetachEvent; import com.vaadin.flow.dom.ElementDetachListener; -import com.vaadin.flow.internal.NullOwner; import com.vaadin.flow.shared.Registration; public abstract class DashboardChildDetachHandler @@ -32,31 +31,17 @@ public abstract class DashboardChildDetachHandler @Override public void onDetach(ElementDetachEvent e) { - if (isComponentDetaching()) { - return; - } var detachedElement = e.getSource(); - component.getChildren() - .filter(childComponent -> Objects.equals(detachedElement, - childComponent.getElement())) - .findAny().ifPresent(detachedChild -> { - // The child was removed from the component - - // Remove the registration for the child detach listener - childDetachListenerMap.get(detachedChild.getElement()) - .remove(); - childDetachListenerMap.remove(detachedChild.getElement()); - - removeChild(detachedChild); - }); - } - - private boolean isComponentDetaching() { - return component.isAttached() - && !NullOwner.get() - .equals(component.getElement().getNode().getOwner()) - && !component.getElement().getNode().getOwner() - .hasNode(component.getElement().getNode()); + var childDetachedFromContainer = component.getElement().getChildren() + .noneMatch(containerChild -> Objects.equals(detachedElement, + containerChild)); + if (childDetachedFromContainer) { + // The child was removed from the component + // Remove the registration for the child detach listener + childDetachListenerMap.get(detachedElement).remove(); + childDetachListenerMap.remove(detachedElement); + detachedElement.getComponent().ifPresent(this::removeChild); + } } void refreshListeners() { From 08d5355970d317e6312ee95b7fe6dcfa38cb8af8 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 12 Sep 2024 12:58:31 +0300 Subject: [PATCH 10/11] refactor: make dashboard not editable by default --- .../flow/component/dashboard/tests/DashboardDragDropPage.java | 1 + .../java/com/vaadin/flow/component/dashboard/Dashboard.java | 1 - .../vaadin/flow/component/dashboard/tests/DashboardTest.java | 4 ++-- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java index be491d79504..9bcb9e6e93b 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java @@ -23,6 +23,7 @@ public class DashboardDragDropPage extends Div { public DashboardDragDropPage() { Dashboard dashboard = new Dashboard(); + dashboard.setEditable(true); DashboardWidget widget1 = new DashboardWidget(); widget1.setTitle("Widget 1"); 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 aff7e92d814..b15ae3bff47 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 @@ -54,7 +54,6 @@ public class Dashboard extends Component implements HasWidgets { */ public Dashboard() { childDetachHandler = getChildDetachHandler(); - setEditable(true); addItemReorderEndListener(this::onItemReorderEnd); } 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 446d0f1fd6e..babd21ee882 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 @@ -779,8 +779,8 @@ public void setGapNull_valueIsCorrectlyRetrieved() { } @Test - public void dashboardIsEditableByDefault() { - Assert.assertTrue(dashboard.isEditable()); + public void dashboardIsNotEditableByDefault() { + Assert.assertFalse(dashboard.isEditable()); } @Test From f892bd60bd1271f4ad51dcac2d355bfcc6f82cd7 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 12 Sep 2024 13:49:55 +0300 Subject: [PATCH 11/11] test: use selenium for drag drop and add tests for editable --- .../tests/DashboardDragDropPage.java | 8 +- .../dashboard/tests/DashboardDragDropIT.java | 89 +++++++++++-------- 2 files changed, 60 insertions(+), 37 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java index 9bcb9e6e93b..71fd88d187d 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/main/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropPage.java @@ -24,6 +24,8 @@ public class DashboardDragDropPage extends Div { public DashboardDragDropPage() { Dashboard dashboard = new Dashboard(); dashboard.setEditable(true); + dashboard.setMinimumRowHeight("100px"); + dashboard.setMaximumColumnWidth("400px"); DashboardWidget widget1 = new DashboardWidget(); widget1.setTitle("Widget 1"); @@ -61,6 +63,10 @@ public DashboardDragDropPage() { }); toggleAttached.setId("toggle-attached"); - add(toggleAttached, dashboard); + NativeButton toggleEditable = new NativeButton("Toggle editable", + e -> dashboard.setEditable(!dashboard.isEditable())); + toggleEditable.setId("toggle-editable"); + + add(toggleAttached, toggleEditable, dashboard); } } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java index c8d751db49b..3812195cb5a 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragDropIT.java @@ -11,11 +11,11 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.openqa.selenium.interactions.Actions; import com.vaadin.flow.component.dashboard.testbench.DashboardElement; -import com.vaadin.flow.component.dashboard.testbench.DashboardSectionElement; -import com.vaadin.flow.component.dashboard.testbench.DashboardWidgetElement; import com.vaadin.flow.testutil.TestPath; +import com.vaadin.testbench.TestBenchElement; import com.vaadin.tests.AbstractComponentIT; /** @@ -34,32 +34,31 @@ public void init() { @Test public void reorderWidgetOnClientSide_itemsAreReorderedCorrectly() { - DashboardWidgetElement widgetToReorder = dashboardElement.getWidgets() - .get(0); - reorderRootLevelItem(1, 0); - Assert.assertEquals(widgetToReorder.getTitle(), + var draggedWidget = dashboardElement.getWidgets().get(0); + var targetWidget = dashboardElement.getWidgets().get(1); + dragDropElement(draggedWidget, targetWidget); + Assert.assertEquals(draggedWidget.getTitle(), dashboardElement.getWidgets().get(1).getTitle()); } @Test public void reorderSectionOnClientSide_itemsAreReorderedCorrectly() { - DashboardSectionElement sectionToReorder = dashboardElement - .getSections().get(0); - reorderRootLevelItem(2, 3); - Assert.assertEquals(sectionToReorder.getTitle(), - dashboardElement.getSections().get(1).getTitle()); + var draggedSection = dashboardElement.getSections().get(1); + var targetWidget = dashboardElement.getWidgets().get(0); + dragDropElement(draggedSection, targetWidget); + Assert.assertEquals(draggedSection.getTitle(), + dashboardElement.getSections().get(0).getTitle()); } @Test public void reorderWidgetInSectionOnClientSide_itemsAreReorderedCorrectly() { - DashboardSectionElement firstSection = dashboardElement.getSections() - .get(0); - DashboardWidgetElement widgetToReorder = firstSection.getWidgets() - .get(1); - reorderWidgetInSection(2, 0, 1); + var firstSection = dashboardElement.getSections().get(0); + var draggedWidget = firstSection.getWidgets().get(0); + var targetWidget = firstSection.getWidgets().get(1); + dragDropElement(draggedWidget, targetWidget); firstSection = dashboardElement.getSections().get(0); - Assert.assertEquals(widgetToReorder.getTitle(), - firstSection.getWidgets().get(0).getTitle()); + Assert.assertEquals(draggedWidget.getTitle(), + firstSection.getWidgets().get(1).getTitle()); } @Test @@ -70,25 +69,43 @@ public void detachReattach_reorderWidgetOnClientSide_itemsAreReorderedCorrectly( reorderWidgetOnClientSide_itemsAreReorderedCorrectly(); } - private void reorderWidgetInSection(int sectionIndex, int initialIndex, - int targetIndex) { - executeScript( - """ - const sectionIndex = %d; - const reorderedItem = arguments[0].items[sectionIndex].items.splice(%d, 1)[0]; - arguments[0].items[sectionIndex].items.splice(%d, 0, reorderedItem); - arguments[0].dispatchEvent(new CustomEvent('dashboard-item-reorder-end'));""" - .formatted(sectionIndex, initialIndex, targetIndex), - dashboardElement); + @Test + public void setDashboardNotEditable_widgetCannotBeDragged() { + var widget = dashboardElement.getWidgets().get(0); + Assert.assertTrue(isHeaderActionsVisible(widget)); + clickElementWithJs("toggle-editable"); + Assert.assertFalse(isHeaderActionsVisible(widget)); + } + + @Test + public void setDashboardEditable_widgetCanBeDragged() { + clickElementWithJs("toggle-editable"); + clickElementWithJs("toggle-editable"); + Assert.assertTrue( + isHeaderActionsVisible(dashboardElement.getWidgets().get(0))); + } + + private void dragDropElement(TestBenchElement draggedElement, + TestBenchElement targetElement) { + var dragHandle = getDragHandle(draggedElement); + + var yOffset = draggedElement.getLocation().getY() < targetElement + .getLocation().getY() ? 10 : -10; + var xOffset = draggedElement.getLocation().getX() < targetElement + .getLocation().getX() ? 10 : -10; + + new Actions(driver).clickAndHold(dragHandle) + .moveToElement(targetElement, xOffset, yOffset) + .release(targetElement).build().perform(); + } + + private static boolean isHeaderActionsVisible(TestBenchElement element) { + TestBenchElement headerActions = element.$("*").withId("header-actions") + .first(); + return !"none".equals(headerActions.getCssValue("display")); } - private void reorderRootLevelItem(int initialIndex, int targetIndex) { - executeScript( - """ - const reorderedItem = arguments[0].items.splice(%d, 1)[0]; - arguments[0].items.splice(%d, 0, reorderedItem); - arguments[0].dispatchEvent(new CustomEvent('dashboard-item-reorder-end'));""" - .formatted(initialIndex, targetIndex), - dashboardElement); + private static TestBenchElement getDragHandle(TestBenchElement element) { + return element.$("*").withClassName("drag-handle").first(); } }