Merge pull request 'fix: undo closing a widget (#40)' (#48) from ai/issue-40-undo-closing-a-widget into main

Reviewed-on: #48

src/main/java/com/example/components/GridStackItem.java

@@ -312,10 +312,26 @@ public class GridStackItem extends Div {
     }
 
     private void close(boolean fromClient) {
+        // Captured before detaching: once removeFromParent() runs, the grid no
+        // longer reports this item, so its position could not be read back from
+        // there afterwards. The gs-* attributes themselves are untouched by the
+        // detach, so a caller that keeps this item around and re-adds it later
+        // (e.g. to undo the close) lands it back at exactly this spot.
+        Position position = capturePosition();
         // Detach first so listeners observe the layout they are about to see —
         // getLayout() on the grid no longer counts this item.
         getElement().removeFromParent();
-        fireEvent(new CloseEvent(this, fromClient));
+        fireEvent(new CloseEvent(this, fromClient, position));
+    }
+
+    private Position capturePosition() {
+        return new Position(getItemId(),
+                attrInt("gs-x"), attrInt("gs-y"), attrInt("gs-w"), attrInt("gs-h"));
+    }
+
+    private int attrInt(String name) {
+        String value = getElement().getAttribute(name);
+        return value == null ? 0 : Integer.parseInt(value);
     }
 
     /** Shows or hides the close button (shown by default). */
@@ -346,10 +362,23 @@ public class GridStackItem extends Div {
         return addListener(CloseEvent.class, listener);
     }
 
-    /** Fired after the item has been removed from its {@link GridStackLayout}. */
+    /** Fired after the item has been removed from its {@link GridStackLayout}.
+     *  Carries the {@link Position} it held right before detaching, so a
+     *  listener can offer to undo the close (re-adding the item at that exact
+     *  spot) without having to track positions itself. {@link #isFromClient()}
+     *  tells apart a user-initiated close (button/menu) from a programmatic
+     *  {@link GridStackItem#close()} — an undo affordance only makes sense for
+     *  the former. */
     public static class CloseEvent extends ComponentEvent<GridStackItem> {
-        CloseEvent(GridStackItem source, boolean fromClient) {
+        private final Position position;
+
+        CloseEvent(GridStackItem source, boolean fromClient, Position position) {
             super(source, fromClient);
+            this.position = position;
+        }
+
+        public Position getPosition() {
+            return position;
         }
     }
 

src/main/java/com/example/views/DashboardView.java

@@ -19,6 +19,7 @@ import com.vaadin.flow.component.button.ButtonVariant;
 import com.vaadin.flow.component.dialog.Dialog;
 import com.vaadin.flow.component.html.Paragraph;
 import com.vaadin.flow.component.html.Span;
+import com.vaadin.flow.component.notification.Notification;
 import com.vaadin.flow.component.orderedlayout.HorizontalLayout;
 import com.vaadin.flow.component.orderedlayout.VerticalLayout;
 import com.vaadin.flow.router.HasDynamicTitle;
@@ -40,7 +41,8 @@ import java.util.Optional;
  * layout is persisted per browser, plus controls to add widgets and reset the
  * layout at runtime. Widgets are removed by their own close button (see
  * {@link GridStackItem#setClosable(boolean)}), not from the toolbar, and are
- * added back from the picker over {@link WidgetRegistry}.
+ * added back from the picker over {@link WidgetRegistry} — or undone straight
+ * from the toast every close spawns (see {@link #offerUndo}).
  * <p>
  * All widget numbers come from {@link ChartDataService} and every chart widget
  * is built by the registry; this view only decides where a widget sits and
@@ -97,17 +99,18 @@ public class DashboardView extends VerticalLayout implements HasDynamicTitle {
                     refreshKpiTile(kpi.id());
                 }
             });
-            grid.add(item);
+            addWidgetToGrid(item);
         }
         context.addFilterChangeListener(this::updateKpiTiles);
 
-        grid.add(
+        List.of(
                 defaultWidget(WidgetRegistry.REVENUE_TREND, 0, 1, 6, 3),
                 defaultWidget(WidgetRegistry.REVENUE_MONTH, 6, 1, 6, 3),
                 defaultWidget(WidgetRegistry.REVENUE_REGION, 0, 4, 5, 3),
                 new GridStackItem("hint", 5, 4, 7, 3,
                         new Card(getTranslation("card.gridstackHint"),
-                                new Paragraph(getTranslation("gridstack.hint")))));
+                                new Paragraph(getTranslation("gridstack.hint")))))
+                .forEach(this::addWidgetToGrid);
 
         status.setText(getTranslation("gridstack.statusInitial"));
         status.addClassName("dialect-muted");
@@ -160,10 +163,57 @@ public class DashboardView extends VerticalLayout implements HasDynamicTitle {
         // widget must not hand its id to the next one, or the new widget would
         // inherit the closed one's saved position.
         extraWidgetCount++;
-        grid.add(widget(definition.type() + "-" + extraWidgetCount, definition,
+        addWidgetToGrid(widget(definition.type() + "-" + extraWidgetCount, definition,
                 0, 0, definition.width(), definition.height()));
     }
 
+    /** Adds an item to the grid and wires the undo toast onto its close — every
+     *  widget goes through here exactly once, so the close listener (and thus
+     *  the undo offer) is registered once per item, not once per re-add: an
+     *  undo hands the very same instance back to {@link #grid} directly. */
+    private void addWidgetToGrid(GridStackItem item) {
+        item.addCloseListener(this::offerUndo);
+        grid.add(item);
+    }
+
+    /**
+     * Closing a widget is one misclick away from losing its position, and a
+     * confirmation dialog on every close would be worse than the problem — so
+     * this shows a dismissible undo toast instead. Skipped for programmatic
+     * closes ({@link GridStackItem#close()}, e.g. from duplicate cleanup),
+     * which is exactly what {@link GridStackItem.CloseEvent#isFromClient()}
+     * tells apart.
+     * <p>
+     * Undo re-adds the very item that was closed — not a rebuilt copy — at the
+     * {@link GridStackItem.Position} the event captured before detaching, so it
+     * keeps its {@code gs-id} and its exact size/position rather than landing
+     * in the grid's next free slot.
+     */
+    private void offerUndo(GridStackItem.CloseEvent event) {
+        if (!event.isFromClient()) {
+            return;
+        }
+        GridStackItem item = event.getSource();
+        GridStackItem.Position position = event.getPosition();
+
+        Notification toast = new Notification();
+        toast.setDuration(8000);
+        toast.setPosition(Notification.Position.BOTTOM_START);
+
+        Button undo = new Button(getTranslation("gridstack.undo"), e -> {
+            item.setPosition(position.x(), position.y(), position.w(), position.h());
+            grid.add(item);
+            toast.close();
+        });
+        undo.addThemeVariants(ButtonVariant.LUMO_TERTIARY_INLINE);
+
+        HorizontalLayout content = new HorizontalLayout(
+                new Span(getTranslation("gridstack.closed")), undo);
+        content.setAlignItems(Alignment.CENTER);
+        toast.add(content);
+        toast.open();
+    }
+
     /** A widget of the initial set. Its id is the plain type — never handed out
      *  again by {@link #addWidget(WidgetDefinition)}, which always suffixes a
      *  counter — and it keeps the dashboard's own placement rather than the

src/main/resources/vaadin-i18n/translations.properties

@@ -37,6 +37,8 @@ gridstack.restore=Wiederherstellen
 gridstack.duplicate=Duplizieren
 gridstack.remove=Entfernen
 gridstack.export=Als CSV exportieren
+gridstack.closed=Widget entfernt
+gridstack.undo=Rückgängig
 
 # Spaltenüberschrift der CSV-Exporte; die Wertspalte trägt den Serien-Namen.
 export.category=Kategorie

src/main/resources/vaadin-i18n/translations_en.properties

@@ -37,6 +37,8 @@ gridstack.restore=Restore
 gridstack.duplicate=Duplicate
 gridstack.remove=Remove
 gridstack.export=Export as CSV
+gridstack.closed=Widget removed
+gridstack.undo=Undo
 
 # Column header of the CSV exports; the value column carries the series name.
 export.category=Category

src/main/resources/vaadin-i18n/translations_es.properties

@@ -37,6 +37,8 @@ gridstack.restore=Restaurar
 gridstack.duplicate=Duplicar
 gridstack.remove=Eliminar
 gridstack.export=Exportar como CSV
+gridstack.closed=Widget eliminado
+gridstack.undo=Deshacer
 
 # Encabezado de columna de las exportaciones CSV; la columna de valores lleva
 # el nombre de la serie.

src/test/java/com/example/e2e/WidgetUndoClosePlaywrightTest.java

@@ -0,0 +1,65 @@
+package com.example.e2e;
+
+import com.microsoft.playwright.Locator;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static com.microsoft.playwright.assertions.PlaywrightAssertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * End-to-end coverage for the undo-on-close toast (see
+ * {@code DashboardView#offerUndo}): a real browser is what actually confirms
+ * the item lands back in its exact grid cell, gridstack animation included.
+ */
+class WidgetUndoClosePlaywrightTest extends PlaywrightTestBase {
+
+    @BeforeEach
+    void openDashboard() {
+        navigate("");
+    }
+
+    @Test
+    void close_thenUndo_restoresTheExactSlot() {
+        Locator item = widget();
+        String before = slot(item);
+
+        closeButton(item).click();
+        assertThat(widget()).not().isAttached();
+
+        undoButton().click();
+
+        Locator restored = widget();
+        assertThat(restored).isAttached();
+        page.waitForCondition(() -> before.equals(slot(restored)));
+        assertEquals(before, slot(restored), "undo must restore the exact position and size");
+    }
+
+    /** Targeted by its stable {@code gs-id} rather than by position: closing it
+     *  changes which item a positional selector would resolve to first. */
+    private Locator widget() {
+        return page.locator(".grid-stack-item[gs-id='revenue-trend']");
+    }
+
+    private Locator closeButton(Locator item) {
+        return item.locator("> .dialect-close-button");
+    }
+
+    private Locator undoButton() {
+        return page.locator("vaadin-notification-card").getByRole(
+                com.microsoft.playwright.options.AriaRole.BUTTON,
+                new Locator.GetByRoleOptions().setName("Undo"));
+    }
+
+    /** The item's box measured against the grid, not the viewport — same as
+     *  {@code WidgetActionMenuPlaywrightTest#slot}. */
+    private String slot(Locator item) {
+        return (String) item.evaluate("""
+                el => {
+                  const grid = el.closest('.grid-stack').getBoundingClientRect();
+                  const box = el.getBoundingClientRect();
+                  return [box.left - grid.left, box.top - grid.top, box.width, box.height]
+                      .map(Math.round).join(',');
+                }""");
+    }
+}

src/test/java/com/example/views/DashboardViewTest.java

@@ -25,6 +25,7 @@ import com.vaadin.flow.component.contextmenu.MenuItem;
 import com.vaadin.flow.component.dialog.Dialog;
 import com.vaadin.flow.component.html.Anchor;
 import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.component.notification.Notification;
 import com.vaadin.flow.dom.DomEvent;
 import com.vaadin.flow.internal.JacksonUtils;
 import org.junit.jupiter.api.Test;
@@ -267,6 +268,66 @@ class DashboardViewTest extends SpringBrowserlessTest {
         assertEquals(DEFAULT_WIDGETS - 1, grid.getLayout().size());
     }
 
+    @Test
+    void closingAWidget_showsAnUndoToast() {
+        navigate(DashboardView.class);
+
+        clickCloseButton(itemById("revenue-trend"));
+
+        Notification toast = $(Notification.class).first();
+        assertNotNull(toast, "closing a widget must offer a way back");
+        assertNotNull($(Button.class).from(toast).withText(translate("gridstack.undo")).first());
+    }
+
+    @Test
+    void undo_restoresTheWidgetAtItsOriginalPositionAndId() {
+        navigate(DashboardView.class);
+        GridStackLayout grid = $view(GridStackLayout.class).first();
+        GridStackItem.Position before = positionOf("revenue-trend");
+
+        clickCloseButton(itemById("revenue-trend"));
+        assertEquals(DEFAULT_WIDGETS - 1, grid.getLayout().size());
+
+        clickUndo();
+
+        assertEquals(DEFAULT_WIDGETS, grid.getLayout().size());
+        assertEquals(before, positionOf("revenue-trend"),
+                "undo must restore the exact position and size, not the next free slot");
+        assertEquals(1, $view(LineChart.class).all().size(), "the chart itself is back too");
+    }
+
+    /** Closing the same widget twice in a row — once, undoing, then again —
+     *  must still offer an undo the second time: the close listener is
+     *  registered once per item, not once per (re-)add. */
+    @Test
+    void undoneWidget_offersAnUndoAgainWhenClosedAnotherTime() {
+        navigate(DashboardView.class);
+
+        clickCloseButton(itemById("revenue-trend"));
+        clickUndo();
+        clickCloseButton(itemById("revenue-trend"));
+
+        assertNotNull($(Button.class).from($(Notification.class).first())
+                .withText(translate("gridstack.undo")).first());
+    }
+
+    /** A close triggered from code (not the button or the menu) is not a
+     *  misclick, so it must not spawn a toast the user never asked for. */
+    @Test
+    void programmaticClose_doesNotShowAnUndoToast() {
+        navigate(DashboardView.class);
+
+        itemById("revenue-trend").close();
+
+        assertTrue($(Notification.class).all().isEmpty(),
+                "a programmatic close must not offer an undo");
+    }
+
+    private void clickUndo() {
+        $(Button.class).from($(Notification.class).first())
+                .withText(translate("gridstack.undo")).first().click();
+    }
+
     @Test
     void nonClosableItem_hasNoCloseButton() {
         navigate(DashboardView.class);