From c25dd36259634a92ad0e92ecc46202fb2c1d4e45 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 19 Sep 2024 16:30:38 +0300 Subject: [PATCH 1/4] refactor: return reordered items and parent from event --- .../flow/component/dashboard/Dashboard.java | 61 +++--------- .../DashboardItemReorderEndEvent.java | 98 +++++++++++++++++-- 2 files changed, 103 insertions(+), 56 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 98cd25c1fe2..07a27171ffb 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,10 +11,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -29,9 +27,6 @@ import com.vaadin.flow.dom.Element; import com.vaadin.flow.shared.Registration; -import elemental.json.JsonArray; -import elemental.json.JsonObject; - /** * @author Vaadin Ltd */ @@ -482,9 +477,8 @@ private void onItemReorderEnd( if (!isEditable()) { return; } - JsonArray orderedItemsFromClient = dashboardItemReorderEndEvent - .getItems(); - reorderItems(orderedItemsFromClient); + reorderItems(dashboardItemReorderEndEvent.getReorderedItemsParent(), + dashboardItemReorderEndEvent.getReorderedItems()); updateClient(); } @@ -507,48 +501,15 @@ private void onItemRemoved( dashboardItemRemovedEvent.getRemovedItem().removeFromParent(); } - 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 void reorderItems(HasWidgets reorderedItemParent, + List items) { + if (reorderedItemParent instanceof DashboardSection parentSection) { + parentSection.removeAll(); + items.stream().map(DashboardWidget.class::cast) + .forEach(parentSection::add); + } else { + childrenComponents.clear(); + childrenComponents.addAll(items); } } 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 index 22ff4dbf79e..efd6d67281f 100644 --- 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 @@ -8,12 +8,20 @@ */ package com.vaadin.flow.component.dashboard; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; + +import com.vaadin.flow.component.Component; 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; +import elemental.json.JsonObject; /** * Widget or section reorder end event of {@link Dashboard}. @@ -24,7 +32,11 @@ @DomEvent("dashboard-item-reorder-end-flow") public class DashboardItemReorderEndEvent extends ComponentEvent { - private final JsonArray items; + private List reorderedItems; + + private JsonArray reorderedItemsFromClient; + + private HasWidgets reorderedItemsParent; /** * Creates a dashboard item reorder end event. @@ -34,19 +46,93 @@ public class DashboardItemReorderEndEvent extends ComponentEvent { * @param fromClient * true if the event originated from the client * side, false otherwise + * @param items + * The ordered items represented by node IDs as a + * {@link JsonArray} */ public DashboardItemReorderEndEvent(Dashboard source, boolean fromClient, @EventData("event.detail.items") JsonArray items) { super(source, fromClient); - this.items = items; + setReorderedItemParent(source, items); + setReorderedItems(); + } + + /** + * Returns the parent of the reordered items. Either a dashboard or a + * section. + * + * @return the parent of the reordered items + */ + public HasWidgets getReorderedItemsParent() { + return reorderedItemsParent; } /** - * Returns the ordered items from the client side + * Returns the list of the reordered item and its sibling items * - * @return items the ordered items as a {@link JsonArray} + * @return the list of the reordered item and its sibling items */ - public JsonArray getItems() { - return items; + public List getReorderedItems() { + return reorderedItems; + } + + private void setReorderedItemParent(Dashboard source, + JsonArray itemsFromClient) { + List serverItems = source.getChildren().toList(); + for (int rootLevelIdx = 0; rootLevelIdx < itemsFromClient + .length(); rootLevelIdx++) { + if (isNodeIdDifferentForIndex(itemsFromClient, serverItems, + rootLevelIdx)) { + this.reorderedItemsParent = source; + this.reorderedItemsFromClient = itemsFromClient; + return; + } + if (serverItems + .get(rootLevelIdx) instanceof DashboardSection section + && isSectionItemReordered(section, + itemsFromClient.get(rootLevelIdx))) { + this.reorderedItemsParent = section; + this.reorderedItemsFromClient = ((JsonObject) itemsFromClient + .get(rootLevelIdx)).getArray("items"); + return; + } + } + } + + private void setReorderedItems() { + Map nodeIdToItems = ((Component) reorderedItemsParent) + .getChildren() + .collect(Collectors.toMap( + item -> item.getElement().getNode().getId(), + Function.identity())); + List items = new ArrayList<>(); + for (int index = 0; index < reorderedItemsFromClient + .length(); index++) { + int nodeIdFromClient = (int) ((JsonObject) reorderedItemsFromClient + .get(index)).getNumber("nodeid"); + items.add(nodeIdToItems.get(nodeIdFromClient)); + } + this.reorderedItems = items; + } + + private static boolean isSectionItemReordered(DashboardSection section, + JsonObject itemFromClient) { + List sectionItems = section.getChildren().toList(); + JsonArray clientSectionItems = itemFromClient.getArray("items"); + for (int index = 0; index < clientSectionItems.length(); index++) { + if (isNodeIdDifferentForIndex(clientSectionItems, sectionItems, + index)) { + return true; + } + } + return false; + } + + private static boolean isNodeIdDifferentForIndex(JsonArray clientItems, + List items, int index) { + JsonObject itemFromClient = clientItems.get(index); + int nodeIdFromClient = (int) itemFromClient.getNumber("nodeid"); + int nodeIdFromServer = items.get(index).getElement().getNode().getId(); + return nodeIdFromClient != nodeIdFromServer; } } From 1e22e76360c8331b400480e135a139fc18cf7537 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Fri, 20 Sep 2024 10:03:14 +0300 Subject: [PATCH 2/4] fix: handle the case where drag reorder returns the same items --- .../dashboard/DashboardItemReorderEndEvent.java | 8 +++++++- .../dashboard/tests/DashboardDragReorderTest.java | 10 ++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) 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 index efd6d67281f..7ff88f20ef5 100644 --- 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 @@ -54,7 +54,13 @@ public DashboardItemReorderEndEvent(Dashboard source, boolean fromClient, @EventData("event.detail.items") JsonArray items) { super(source, fromClient); setReorderedItemParent(source, items); - setReorderedItems(); + if (reorderedItemsParent == null) { + // No reordering + reorderedItemsParent = source; + reorderedItems = source.getChildren().toList(); + } else { + setReorderedItems(); + } } /** diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderTest.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderTest.java index bdddead6019..cc278f8d734 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderTest.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderTest.java @@ -50,6 +50,11 @@ public void reorderWidget_orderIsUpdated() { assertRootLevelItemReorder(0, 1); } + @Test + public void reorderWidgetToSamePosition_orderIsNotUpdated() { + assertRootLevelItemReorder(0, 0); + } + @Test public void reorderSection_orderIsUpdated() { assertRootLevelItemReorder(2, 1); @@ -60,6 +65,11 @@ public void reorderWidgetInSection_orderIsUpdated() { assertSectionWidgetReorder(2, 0, 1); } + @Test + public void reorderWidgetInSectionToSamePosition_orderIsNotUpdated() { + assertSectionWidgetReorder(2, 0, 0); + } + @Test public void setDashboardNotEditable_reorderWidget_orderIsNotUpdated() { dashboard.setEditable(false); From e835c28bc752101892edc9bbe07f08e2769cf83c Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Fri, 20 Sep 2024 10:03:35 +0300 Subject: [PATCH 3/4] chore: remove unused code --- .../component/dashboard/tests/DashboardDragReorderIT.java | 4 ---- .../component/dashboard/tests/DashboardDragResizeIT.java | 6 ------ 2 files changed, 10 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderIT.java index fb5cdc82a6b..c22ae08e003 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderIT.java @@ -83,10 +83,6 @@ private void dragResizeElement(TestBenchElement draggedElement, .release(targetElement).build().perform(); } - private static boolean isDragHandleVisible(TestBenchElement element) { - return !"none".equals(getDragHandle(element).getCssValue("display")); - } - private static TestBenchElement getDragHandle(TestBenchElement element) { return element.$("*").withClassName("drag-handle").first(); } diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragResizeIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragResizeIT.java index 371b641d566..32698305e27 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragResizeIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragResizeIT.java @@ -81,12 +81,6 @@ private void resizeWidget(int widgetIndexToResize, double xResizeRatio, .release().build().perform(); } - private boolean isResizeHandleVisible( - DashboardWidgetElement widgetElement) { - return !"none" - .equals(getResizeHandle(widgetElement).getCssValue("display")); - } - private static TestBenchElement getResizeHandle( DashboardWidgetElement widgetElement) { return widgetElement.$("*").withClassName("resize-handle").first(); From ef1cd8afa20a9f6a9412983f873337d03ae90e19 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Fri, 20 Sep 2024 10:06:28 +0300 Subject: [PATCH 4/4] chore: rename incorrectly named test method --- .../component/dashboard/tests/DashboardDragReorderIT.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderIT.java b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderIT.java index c22ae08e003..ac316882bb5 100644 --- a/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderIT.java +++ b/vaadin-dashboard-flow-parent/vaadin-dashboard-flow-integration-tests/src/test/java/com/vaadin/flow/component/dashboard/tests/DashboardDragReorderIT.java @@ -36,7 +36,7 @@ public void init() { public void reorderWidgetOnClientSide_itemsAreReorderedCorrectly() { var draggedWidget = dashboardElement.getWidgets().get(0); var targetWidget = dashboardElement.getWidgets().get(1); - dragResizeElement(draggedWidget, targetWidget); + dragReorderElement(draggedWidget, targetWidget); Assert.assertEquals(draggedWidget.getTitle(), dashboardElement.getWidgets().get(1).getTitle()); } @@ -45,7 +45,7 @@ public void reorderWidgetOnClientSide_itemsAreReorderedCorrectly() { public void reorderSectionOnClientSide_itemsAreReorderedCorrectly() { var draggedSection = dashboardElement.getSections().get(1); var targetWidget = dashboardElement.getWidgets().get(0); - dragResizeElement(draggedSection, targetWidget); + dragReorderElement(draggedSection, targetWidget); Assert.assertEquals(draggedSection.getTitle(), dashboardElement.getSections().get(0).getTitle()); } @@ -55,7 +55,7 @@ public void reorderWidgetInSectionOnClientSide_itemsAreReorderedCorrectly() { var firstSection = dashboardElement.getSections().get(0); var draggedWidget = firstSection.getWidgets().get(0); var targetWidget = firstSection.getWidgets().get(1); - dragResizeElement(draggedWidget, targetWidget); + dragReorderElement(draggedWidget, targetWidget); firstSection = dashboardElement.getSections().get(0); Assert.assertEquals(draggedWidget.getTitle(), firstSection.getWidgets().get(1).getTitle()); @@ -69,7 +69,7 @@ public void detachReattach_reorderWidgetOnClientSide_itemsAreReorderedCorrectly( reorderWidgetOnClientSide_itemsAreReorderedCorrectly(); } - private void dragResizeElement(TestBenchElement draggedElement, + private void dragReorderElement(TestBenchElement draggedElement, TestBenchElement targetElement) { var dragHandle = getDragHandle(draggedElement);