Skip to content

Commit

Permalink
refactor: use id instead of nodeid in client items array (#6671)
Browse files Browse the repository at this point in the history 
* test: add tests for keyboard move and resize

* fix: only update client items when necessary

* refactor: use id in items array instead of nodeid

* refactor: reintroduce removed client item updates

* refactor: use id instead of nodeid in item state changed events

* refactor: remove unnecessary equality check
  • Loading branch information
@ugur-vaadin
ugur-vaadin authored Sep 26, 2024
1 parent cd1b906 commit eca62d3
Show file tree
Hide file tree
Showing 11 changed files with 147 additions and 79 deletions.
6 changes: 3 additions & 3 deletions ...board/tests/DashboardDragReorderPage.java → ...ashboard/tests/DashboardItemMovePage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
/**
* @author Vaadin Ltd
*/
@Route("vaadin-dashboard/drag-reorder")
public class DashboardDragReorderPage extends Div {
@Route("vaadin-dashboard/item-move")
public class DashboardItemMovePage extends Div {

public DashboardDragReorderPage() {
public DashboardItemMovePage() {
Dashboard dashboard = new Dashboard();
dashboard.setEditable(true);
dashboard.setMinimumRowHeight("100px");
Expand Down
6 changes: 3 additions & 3 deletions ...hboard/tests/DashboardDragResizePage.java → ...hboard/tests/DashboardItemResizePage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
/**
* @author Vaadin Ltd
*/
@Route("vaadin-dashboard/drag-resize")
public class DashboardDragResizePage extends Div {
@Route("vaadin-dashboard/item-resize")
public class DashboardItemResizePage extends Div {

public DashboardDragResizePage() {
public DashboardItemResizePage() {
Dashboard dashboard = new Dashboard();
dashboard.setEditable(true);
dashboard.setMinimumRowHeight("200px");
Expand Down
67 changes: 55 additions & 12 deletions ...shboard/tests/DashboardDragReorderIT.java → .../dashboard/tests/DashboardItemMoveIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.openqa.selenium.Keys;
import org.openqa.selenium.interactions.Actions;

import com.vaadin.flow.component.dashboard.testbench.DashboardElement;
Expand All @@ -21,8 +22,8 @@
/**
* @author Vaadin Ltd
*/
@TestPath("vaadin-dashboard/drag-reorder")
public class DashboardDragReorderIT extends AbstractComponentIT {
@TestPath("vaadin-dashboard/item-move")
public class DashboardItemMoveIT extends AbstractComponentIT {

private DashboardElement dashboardElement;

Expand All @@ -33,43 +34,85 @@ public void init() {
}

@Test
public void reorderWidgetOnClientSide_itemsAreReorderedCorrectly() {
public void dragMoveWidget_itemIsMovedCorrectly() {
var draggedWidget = dashboardElement.getWidgets().get(0);
var targetWidget = dashboardElement.getWidgets().get(1);
dragReorderElement(draggedWidget, targetWidget);
dragMoveElement(draggedWidget, targetWidget);
Assert.assertEquals(draggedWidget.getTitle(),
dashboardElement.getWidgets().get(1).getTitle());
}

@Test
public void reorderSectionOnClientSide_itemsAreReorderedCorrectly() {
public void dragMoveSection_itemIsMovedCorrectly() {
var draggedSection = dashboardElement.getSections().get(1);
var targetWidget = dashboardElement.getWidgets().get(0);
dragReorderElement(draggedSection, targetWidget);
dragMoveElement(draggedSection, targetWidget);
Assert.assertEquals(draggedSection.getTitle(),
dashboardElement.getSections().get(0).getTitle());
}

@Test
public void reorderWidgetInSectionOnClientSide_itemsAreReorderedCorrectly() {
public void dragMoveWidgetInSection_itemIsMovedCorrectly() {
var firstSection = dashboardElement.getSections().get(0);
var draggedWidget = firstSection.getWidgets().get(0);
var targetWidget = firstSection.getWidgets().get(1);
dragReorderElement(draggedWidget, targetWidget);
dragMoveElement(draggedWidget, targetWidget);
firstSection = dashboardElement.getSections().get(0);
Assert.assertEquals(draggedWidget.getTitle(),
firstSection.getWidgets().get(1).getTitle());
}

@Test
public void detachReattach_reorderWidgetOnClientSide_itemsAreReorderedCorrectly() {
public void keyboardMoveWidget_itemIsMovedCorrectly() {
var widgetToMove = dashboardElement.getWidgets().get(0);
var expectedTitle = widgetToMove.getTitle();
// Select and move the widget
widgetToMove.sendKeys(Keys.ENTER, Keys.RIGHT);
widgetToMove = dashboardElement.getWidgets().get(1);
Assert.assertEquals(expectedTitle, widgetToMove.getTitle());
// Move the widget back
widgetToMove.sendKeys(Keys.LEFT);
Assert.assertEquals(expectedTitle,
dashboardElement.getWidgets().get(0).getTitle());
}

@Test
public void keyboardMoveSection_itemIsMovedCorrectly() {
var sectionToMove = dashboardElement.getSections().get(0);
var expectedTitle = sectionToMove.getTitle();
// Select and move the section
sectionToMove.sendKeys(Keys.ENTER, Keys.RIGHT);
sectionToMove = dashboardElement.getSections().get(1);
Assert.assertEquals(expectedTitle, sectionToMove.getTitle());
// Move the section back
sectionToMove.sendKeys(Keys.LEFT);
Assert.assertEquals(expectedTitle,
dashboardElement.getSections().get(0).getTitle());
}

@Test
public void keyboardMoveWidgetInSection_itemIsMovedCorrectly() {
var widgetToMove = dashboardElement.getWidgets().get(2);
var expectedTitle = widgetToMove.getTitle();
// Select and move the widget
widgetToMove.sendKeys(Keys.ENTER, Keys.RIGHT);
widgetToMove = dashboardElement.getWidgets().get(3);
Assert.assertEquals(expectedTitle, widgetToMove.getTitle());
// Move the widget back
widgetToMove.sendKeys(Keys.LEFT);
Assert.assertEquals(expectedTitle,
dashboardElement.getWidgets().get(2).getTitle());
}

@Test
public void detachReattach_dragMoveWidget_itemIsMovedCorrectly() {
clickElementWithJs("toggle-attached");
clickElementWithJs("toggle-attached");
dashboardElement = $(DashboardElement.class).waitForFirst();
reorderWidgetOnClientSide_itemsAreReorderedCorrectly();
dragMoveWidget_itemIsMovedCorrectly();
}

private void dragReorderElement(TestBenchElement draggedElement,
private void dragMoveElement(TestBenchElement draggedElement,
TestBenchElement targetElement) {
var dragHandle = getDragHandle(draggedElement);

Expand All @@ -78,7 +121,7 @@ private void dragReorderElement(TestBenchElement draggedElement,
var xOffset = draggedElement.getLocation().getX() < targetElement
.getLocation().getX() ? 10 : -10;

new Actions(driver).clickAndHold(dragHandle)
new Actions(getDriver()).clickAndHold(dragHandle)
.moveToElement(targetElement, xOffset, yOffset)
.release(targetElement).build().perform();
}
Expand Down
44 changes: 37 additions & 7 deletions ...ashboard/tests/DashboardDragResizeIT.java → ...ashboard/tests/DashboardItemResizeIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.junit.Before;
import org.junit.Test;
import org.openqa.selenium.Dimension;
import org.openqa.selenium.Keys;
import org.openqa.selenium.interactions.Actions;

import com.vaadin.flow.component.dashboard.testbench.DashboardElement;
Expand All @@ -23,8 +24,8 @@
/**
* @author Vaadin Ltd
*/
@TestPath("vaadin-dashboard/drag-resize")
public class DashboardDragResizeIT extends AbstractComponentIT {
@TestPath("vaadin-dashboard/item-resize")
public class DashboardItemResizeIT extends AbstractComponentIT {

private DashboardElement dashboardElement;

Expand All @@ -36,20 +37,49 @@ public void init() {
}

@Test
public void resizeWidgetBothHorizontallyAndVertically_widgetIsResizedCorrectly() {
public void dragResizeWidget_widgetIsResizedCorrectly() {
assertWidgetResized(0);
}

@Test
public void resizeWidgetInSectionBothHorizontallyAndVertically_widgetIsResizedCorrectly() {
public void dragResizeWidgetInSection_widgetIsResizedCorrectly() {
assertWidgetResized(1);
}

@Test
public void keyboardResizeWidget_widgetIsResizedCorrectly() {
assertWidgetKeyboardResized(0);
}

@Test
public void keyboardResizeWidgetInSection_widgetIsResizedCorrectly() {
assertWidgetKeyboardResized(1);
}

private void assertWidgetKeyboardResized(int widgetIndexToResize) {
var widgetToResize = dashboardElement.getWidgets()
.get(widgetIndexToResize);
var initialWidth = widgetToResize.getSize().getWidth();
// Select widget
widgetToResize.sendKeys(Keys.ENTER);
// Grow the widget
new Actions(getDriver()).keyDown(Keys.SHIFT).sendKeys(Keys.RIGHT)
.build().perform();
var delta = 20;
Assert.assertEquals(initialWidth * 2,
widgetToResize.getSize().getWidth(), delta);
// Shrink the widget back
new Actions(getDriver()).keyDown(Keys.SHIFT).sendKeys(Keys.LEFT).build()
.perform();
Assert.assertEquals(initialWidth, widgetToResize.getSize().getWidth(),
delta);
}

private void assertWidgetResized(int widgetIndexToResize) {
var widgetToResize = dashboardElement.getWidgets()
.get(widgetIndexToResize);
int xResizeRatio = 2;
int yResizeRatio = 2;
var xResizeRatio = 2;
var yResizeRatio = 2;
var expectedWidth = widgetToResize.getSize().getWidth() * xResizeRatio;
var expectedHeight = widgetToResize.getSize().getHeight()
* yResizeRatio;
Expand All @@ -72,7 +102,7 @@ private void resizeWidget(int widgetIndexToResize, double xResizeRatio,
var yOffset = (int) (widgetToResize.getSize().getHeight()
* (yResizeRatio - 1));
TestBenchElement resizeHandle = getResizeHandle(widgetToResize);
int trackStartOffset = 5;
var trackStartOffset = 5;
new Actions(driver).moveToElement(resizeHandle).clickAndHold()
// This is necessary for the Polymer track event to be fired.
.moveByOffset(trackStartOffset, trackStartOffset)
Expand Down
54 changes: 25 additions & 29 deletions ...nt/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/Dashboard.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.vaadin.flow.component.dependency.JsModule;
import com.vaadin.flow.component.dependency.NpmPackage;
import com.vaadin.flow.dom.DomEvent;
import com.vaadin.flow.dom.DomListenerRegistration;
import com.vaadin.flow.dom.Element;
import com.vaadin.flow.internal.JsonSerializer;
import com.vaadin.flow.shared.Registration;
Expand Down Expand Up @@ -489,7 +488,7 @@ private void updateClientItems() {
.map(widget -> getWidgetRepresentation(widget,
itemIndex.getAndIncrement()))
.collect(Collectors.joining(","));
itemRepresentation = "{ component: $%d, items: [ %s ], nodeid: %d }"
itemRepresentation = "{ component: $%d, items: [ %s ], id: %d }"
.formatted(sectionIndex, sectionWidgetsRepresentation,
section.getElement().getNode().getId());
} else {
Expand Down Expand Up @@ -529,7 +528,7 @@ private void removeNullValuesFromJsonObject(JsonObject jsonObject) {

private static String getWidgetRepresentation(DashboardWidget widget,
int itemIndex) {
return "{ component: $%d, colspan: %d, rowspan: %d, nodeid: %d }"
return "{ component: $%d, colspan: %d, rowspan: %d, id: %d }"
.formatted(itemIndex, widget.getColspan(), widget.getRowspan(),
widget.getElement().getNode().getId());
}
Expand Down Expand Up @@ -613,9 +612,8 @@ private void handleItemMovedClientEvent(DomEvent e, String itemKey,
.map(DashboardSection.class::cast).findAny().orElseThrow();
reorderedItems = getReorderedItemsList(
getSectionItems(itemsNodeIds, sectionNodeId), section);
section.removeAll();
reorderedItems.stream().map(DashboardWidget.class::cast)
.forEach(section::add);
section.reorderWidgets(reorderedItems.stream()
.map(DashboardWidget.class::cast).toList());
}
Component movedItem = reorderedItems.stream().filter(
item -> itemNodeId == item.getElement().getNode().getId())
Expand All @@ -625,22 +623,22 @@ private void handleItemMovedClientEvent(DomEvent e, String itemKey,
}

private void initItemResizedClientEventListener() {
String nodeIdKey = "event.detail.item.nodeid";
String idKey = "event.detail.item.id";
String colspanKey = "event.detail.item.colspan";
String rowspanKey = "event.detail.item.rowspan";
getElement().addEventListener("dashboard-item-resized", e -> {
if (!isEditable()) {
return;
}
handleItemResizedClientEvent(e, nodeIdKey, colspanKey, rowspanKey);
handleItemResizedClientEvent(e, idKey, colspanKey, rowspanKey);
updateClient();
}).addEventData(nodeIdKey).addEventData(colspanKey)
}).addEventData(idKey).addEventData(colspanKey)
.addEventData(rowspanKey);
}

private void handleItemResizedClientEvent(DomEvent e, String nodeIdKey,
private void handleItemResizedClientEvent(DomEvent e, String idKey,
String colspanKey, String rowspanKey) {
int nodeId = (int) e.getEventData().getNumber(nodeIdKey);
int nodeId = (int) e.getEventData().getNumber(idKey);
int colspan = (int) e.getEventData().getNumber(colspanKey);
int rowspan = (int) e.getEventData().getNumber(rowspanKey);
DashboardWidget resizedWidget = getWidgets().stream()
Expand All @@ -653,20 +651,18 @@ private void handleItemResizedClientEvent(DomEvent e, String nodeIdKey,
}

private void initItemRemovedClientEventListener() {
String nodeIdKey = "event.detail.item.nodeid";
DomListenerRegistration registration = getElement()
.addEventListener("dashboard-item-removed", e -> {
if (!isEditable()) {
return;
}
handleItemRemovedClientEvent(e, nodeIdKey);
updateClient();
});
registration.addEventData(nodeIdKey);
String idKey = "event.detail.item.id";
getElement().addEventListener("dashboard-item-removed", e -> {
if (!isEditable()) {
return;
}
handleItemRemovedClientEvent(e, idKey);
updateClient();
}).addEventData(idKey);
}

private void handleItemRemovedClientEvent(DomEvent e, String nodeIdKey) {
int nodeId = (int) e.getEventData().getNumber(nodeIdKey);
private void handleItemRemovedClientEvent(DomEvent e, String idKey) {
int nodeId = (int) e.getEventData().getNumber(idKey);
Component removedItem = getItem(nodeId);
removedItem.removeFromParent();
fireEvent(new DashboardItemRemovedEvent(this, true, removedItem,
Expand All @@ -678,16 +674,16 @@ private void customizeItemMovedEvent() {
"""
this.addEventListener('dashboard-item-moved', (e) => {
function mapItems(items) {
return items.map(({nodeid, items}) => ({
nodeid,
return items.map(({id, items}) => ({
id,
...(items && { items: mapItems(items) })
}));
}
const flowItemMovedEvent = new CustomEvent('dashboard-item-moved-flow', {
detail: {
item: e.detail.item.nodeid,
item: e.detail.item.id,
items: mapItems(e.detail.items),
section: e.detail.section?.nodeid
section: e.detail.section?.id
}
});
this.dispatchEvent(flowItemMovedEvent);
Expand All @@ -707,7 +703,7 @@ private static List<Component> getReorderedItemsList(
for (int index = 0; index < reorderedItemsFromClient
.length(); index++) {
int nodeIdFromClient = (int) ((JsonObject) reorderedItemsFromClient
.get(index)).getNumber("nodeid");
.get(index)).getNumber("id");
items.add(nodeIdToItems.get(nodeIdFromClient));
}
return items;
Expand All @@ -718,7 +714,7 @@ private static JsonArray getSectionItems(JsonArray items,
for (int rootLevelIdx = 0; rootLevelIdx < items
.length(); rootLevelIdx++) {
JsonObject item = items.get(rootLevelIdx);
int itemNodeId = (int) item.getNumber("nodeid");
int itemNodeId = (int) item.getNumber("id");
if (sectionNodeId == itemNodeId) {
JsonObject sectionObj = items.get(rootLevelIdx);
return sectionObj.getArray("items");
Expand Down
10 changes: 4 additions & 6 deletions .../src/main/java/com/vaadin/flow/component/dashboard/DashboardItemMoveModeChangedEvent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,16 @@ public class DashboardItemMoveModeChangedEvent
* @param fromClient
* {@code true} if the event originated from the client side,
* {@code false} otherwise
* @param itemNodeId
* The node ID of the item of which the move mode state has
* changed
* @param itemId
* The ID of the item of which the move mode state has changed
* @param moveMode
* Whether the item is in move mode
*/
public DashboardItemMoveModeChangedEvent(Dashboard source,
boolean fromClient,
@EventData("event.detail.item.nodeid") int itemNodeId,
boolean fromClient, @EventData("event.detail.item.id") int itemId,
@EventData("event.detail.value") boolean moveMode) {
super(source, fromClient);
this.item = source.getItem(itemNodeId);
this.item = source.getItem(itemId);
this.moveMode = moveMode;
}

Expand Down
Loading

0 comments on commit eca62d3

Please sign in to comment.