fix: undo closing a widget (#40) #48

Merged
pitfriedrich merged 1 commits from ai/issue-40-undo-closing-a-widget into main 2026-07-28 21:30:54 +00:00
7 changed files with 219 additions and 8 deletions
Showing only changes of commit 4878a7fc06 - Show all commits
@@ -312,10 +312,26 @@ public class GridStackItem extends Div {
} }
private void close(boolean fromClient) { 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 — // Detach first so listeners observe the layout they are about to see —
// getLayout() on the grid no longer counts this item. // getLayout() on the grid no longer counts this item.
getElement().removeFromParent(); 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). */ /** Shows or hides the close button (shown by default). */
@@ -346,10 +362,23 @@ public class GridStackItem extends Div {
return addListener(CloseEvent.class, listener); 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> { 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); super(source, fromClient);
this.position = position;
}
public Position getPosition() {
return position;
} }
} }
@@ -19,6 +19,7 @@ import com.vaadin.flow.component.button.ButtonVariant;
import com.vaadin.flow.component.dialog.Dialog; import com.vaadin.flow.component.dialog.Dialog;
import com.vaadin.flow.component.html.Paragraph; import com.vaadin.flow.component.html.Paragraph;
import com.vaadin.flow.component.html.Span; 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.HorizontalLayout;
import com.vaadin.flow.component.orderedlayout.VerticalLayout; import com.vaadin.flow.component.orderedlayout.VerticalLayout;
import com.vaadin.flow.router.HasDynamicTitle; 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 is persisted per browser, plus controls to add widgets and reset the
* layout at runtime. Widgets are removed by their own close button (see * layout at runtime. Widgets are removed by their own close button (see
* {@link GridStackItem#setClosable(boolean)}), not from the toolbar, and are * {@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> * <p>
* All widget numbers come from {@link ChartDataService} and every chart widget * 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 * 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()); refreshKpiTile(kpi.id());
} }
}); });
grid.add(item); addWidgetToGrid(item);
} }
context.addFilterChangeListener(this::updateKpiTiles); context.addFilterChangeListener(this::updateKpiTiles);
grid.add( List.of(
defaultWidget(WidgetRegistry.REVENUE_TREND, 0, 1, 6, 3), defaultWidget(WidgetRegistry.REVENUE_TREND, 0, 1, 6, 3),
defaultWidget(WidgetRegistry.REVENUE_MONTH, 6, 1, 6, 3), defaultWidget(WidgetRegistry.REVENUE_MONTH, 6, 1, 6, 3),
defaultWidget(WidgetRegistry.REVENUE_REGION, 0, 4, 5, 3), defaultWidget(WidgetRegistry.REVENUE_REGION, 0, 4, 5, 3),
new GridStackItem("hint", 5, 4, 7, 3, new GridStackItem("hint", 5, 4, 7, 3,
new Card(getTranslation("card.gridstackHint"), new Card(getTranslation("card.gridstackHint"),
new Paragraph(getTranslation("gridstack.hint"))))); new Paragraph(getTranslation("gridstack.hint")))))
.forEach(this::addWidgetToGrid);
status.setText(getTranslation("gridstack.statusInitial")); status.setText(getTranslation("gridstack.statusInitial"));
status.addClassName("dialect-muted"); 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 // widget must not hand its id to the next one, or the new widget would
// inherit the closed one's saved position. // inherit the closed one's saved position.
extraWidgetCount++; extraWidgetCount++;
grid.add(widget(definition.type() + "-" + extraWidgetCount, definition, addWidgetToGrid(widget(definition.type() + "-" + extraWidgetCount, definition,
0, 0, definition.width(), definition.height())); 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 /** A widget of the initial set. Its id is the plain type — never handed out
* again by {@link #addWidget(WidgetDefinition)}, which always suffixes a * again by {@link #addWidget(WidgetDefinition)}, which always suffixes a
* counter — and it keeps the dashboard's own placement rather than the * counter — and it keeps the dashboard's own placement rather than the
@@ -37,6 +37,8 @@ gridstack.restore=Wiederherstellen
gridstack.duplicate=Duplizieren gridstack.duplicate=Duplizieren
gridstack.remove=Entfernen gridstack.remove=Entfernen
gridstack.export=Als CSV exportieren 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. # Spaltenüberschrift der CSV-Exporte; die Wertspalte trägt den Serien-Namen.
export.category=Kategorie export.category=Kategorie
@@ -37,6 +37,8 @@ gridstack.restore=Restore
gridstack.duplicate=Duplicate gridstack.duplicate=Duplicate
gridstack.remove=Remove gridstack.remove=Remove
gridstack.export=Export as CSV 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. # Column header of the CSV exports; the value column carries the series name.
export.category=Category export.category=Category
@@ -37,6 +37,8 @@ gridstack.restore=Restaurar
gridstack.duplicate=Duplicar gridstack.duplicate=Duplicar
gridstack.remove=Eliminar gridstack.remove=Eliminar
gridstack.export=Exportar como CSV gridstack.export=Exportar como CSV
gridstack.closed=Widget eliminado
gridstack.undo=Deshacer
# Encabezado de columna de las exportaciones CSV; la columna de valores lleva # Encabezado de columna de las exportaciones CSV; la columna de valores lleva
# el nombre de la serie. # el nombre de la serie.
@@ -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(',');
}""");
}
}
@@ -25,6 +25,7 @@ import com.vaadin.flow.component.contextmenu.MenuItem;
import com.vaadin.flow.component.dialog.Dialog; import com.vaadin.flow.component.dialog.Dialog;
import com.vaadin.flow.component.html.Anchor; import com.vaadin.flow.component.html.Anchor;
import com.vaadin.flow.component.html.Div; import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.component.notification.Notification;
import com.vaadin.flow.dom.DomEvent; import com.vaadin.flow.dom.DomEvent;
import com.vaadin.flow.internal.JacksonUtils; import com.vaadin.flow.internal.JacksonUtils;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
@@ -267,6 +268,66 @@ class DashboardViewTest extends SpringBrowserlessTest {
assertEquals(DEFAULT_WIDGETS - 1, grid.getLayout().size()); 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 @Test
void nonClosableItem_hasNoCloseButton() { void nonClosableItem_hasNoCloseButton() {
navigate(DashboardView.class); navigate(DashboardView.class);