Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: return reordered items and parent from event #6657

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand All @@ -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());
}
Expand All @@ -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());
Expand All @@ -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);

Expand All @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
tomivirkki marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
*/
Expand Down Expand Up @@ -482,9 +477,8 @@ private void onItemReorderEnd(
if (!isEditable()) {
return;
}
JsonArray orderedItemsFromClient = dashboardItemReorderEndEvent
.getItems();
reorderItems(orderedItemsFromClient);
reorderItems(dashboardItemReorderEndEvent.getReorderedItemsParent(),
dashboardItemReorderEndEvent.getReorderedItems());
updateClient();
}

Expand All @@ -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<Integer, Component> 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<Integer, DashboardWidget> 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<Component> items) {
if (reorderedItemParent instanceof DashboardSection parentSection) {
parentSection.removeAll();
items.stream().map(DashboardWidget.class::cast)
.forEach(parentSection::add);
} else {
childrenComponents.clear();
childrenComponents.addAll(items);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand All @@ -24,7 +32,11 @@
@DomEvent("dashboard-item-reorder-end-flow")
public class DashboardItemReorderEndEvent extends ComponentEvent<Dashboard> {

private final JsonArray items;
private List<Component> reorderedItems;

private JsonArray reorderedItemsFromClient;

private HasWidgets reorderedItemsParent;

/**
* Creates a dashboard item reorder end event.
Expand All @@ -34,19 +46,99 @@ public class DashboardItemReorderEndEvent extends ComponentEvent<Dashboard> {
* @param fromClient
* <code>true</code> if the event originated from the client
* side, <code>false</code> 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);
if (reorderedItemsParent == null) {
// No reordering
reorderedItemsParent = source;
reorderedItems = source.getChildren().toList();
} else {
setReorderedItems();
}
}

/**
* Returns the parent of the reordered items. Either a dashboard or a
* section.
Comment on lines +67 to +68
Copy link
Member

Choose a reason for hiding this comment

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

I tried this locally and I'm always getting the Dashboard instance, even when reordering inside 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<Component> getReorderedItems() {
return reorderedItems;
}

private void setReorderedItemParent(Dashboard source,
JsonArray itemsFromClient) {
List<Component> 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<Integer, Component> nodeIdToItems = ((Component) reorderedItemsParent)
.getChildren()
.collect(Collectors.toMap(
item -> item.getElement().getNode().getId(),
Function.identity()));
List<Component> 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<Component> 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<Component> 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down