Merge pull request 'feat: widget registry and "add widget" picker (#21)' (#35) from ai/issue-21-widget-registry-picker into main

Reviewed-on: #35

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

@@ -1,24 +1,19 @@
 package com.example.views;
 
-import com.example.components.ApexChart;
-import com.example.components.AxisChart;
-import com.example.components.BarChart;
 import com.example.components.Card;
 import com.example.components.Fa;
 import com.example.components.GridStackItem;
 import com.example.components.GridStackLayout;
 import com.example.components.KpiTile;
-import com.example.components.LineChart;
-import com.example.components.PieChart;
 import com.example.data.ChartDataService;
-import com.example.data.ChartSeries;
 import com.example.data.KpiData;
-import com.vaadin.flow.component.Component;
+import com.example.widgets.WidgetDefinition;
+import com.example.widgets.WidgetRegistry;
 import com.vaadin.flow.component.button.Button;
 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;
@@ -30,11 +25,12 @@ import java.util.List;
  * The dashboard: a {@link GridStackLayout} of draggable/resizable cards whose
  * 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.
+ * {@link GridStackItem#setClosable(boolean)}), not from the toolbar, and are
+ * added back from the picker over {@link WidgetRegistry}.
  * <p>
- * All widget numbers come from {@link ChartDataService}; this view only decides
+ * All widget numbers come from {@link ChartDataService} and every chart widget
- * where a widget sits and resolves the data's translation keys against the
+ * is built by the registry; this view only decides where a widget sits and
- * bundle.
+ * resolves the translation keys it is handed against the bundle.
  */
 @Route("")
 public class DashboardView extends VerticalLayout implements HasDynamicTitle {
@@ -44,13 +40,13 @@ public class DashboardView extends VerticalLayout implements HasDynamicTitle {
     /** A KPI tile is a quarter row wide, so the n-th one starts at 3n. */
     private static final int KPI_WIDTH = 3;
 
-    private final ChartDataService dataService;
+    private final WidgetRegistry widgets;
     private final GridStackLayout grid = new GridStackLayout();
     private final Span status = new Span();
     private int extraWidgetCount;
 
-    public DashboardView(ChartDataService dataService) {
+    public DashboardView(ChartDataService dataService, WidgetRegistry widgets) {
-        this.dataService = dataService;
+        this.widgets = widgets;
         addClassName("dialect-content");
 
         grid.setWidthFull();
@@ -70,12 +66,9 @@ public class DashboardView extends VerticalLayout implements HasDynamicTitle {
         }
 
         grid.add(
-                new GridStackItem("revenue-trend", 0, 1, 6, 3,
+                defaultWidget(WidgetRegistry.REVENUE_TREND, 0, 1, 6, 3),
-                        new Card(getTranslation("card.revenueTrend"), lineChart())),
+                defaultWidget(WidgetRegistry.REVENUE_MONTH, 6, 1, 6, 3),
-                new GridStackItem("revenue-month", 6, 1, 6, 3,
+                defaultWidget(WidgetRegistry.REVENUE_REGION, 0, 4, 5, 3),
-                        new Card(getTranslation("card.revenueByMonth"), barChart())),
-                new GridStackItem("revenue-region", 0, 4, 5, 3,
-                        new Card(getTranslation("card.revenueByRegion"), pieChart())),
                 new GridStackItem("hint", 5, 4, 7, 3,
                         new Card(getTranslation("card.gridstackHint"),
                                 new Paragraph(getTranslation("gridstack.hint")))));
@@ -88,7 +81,7 @@ public class DashboardView extends VerticalLayout implements HasDynamicTitle {
 
     private HorizontalLayout toolbar() {
         Button add = new Button(getTranslation("gridstack.addWidget"), Fa.ADD.create(),
-                e -> addWidget());
+                e -> openWidgetPicker());
         add.addThemeVariants(ButtonVariant.LUMO_PRIMARY);
 
         Button reset = new Button(getTranslation("gridstack.reset"), Fa.RESET.create(),
@@ -101,16 +94,52 @@ public class DashboardView extends VerticalLayout implements HasDynamicTitle {
         return toolbar;
     }
 
-    private void addWidget() {
+    /** Lists the registered widget types; picking one adds a widget of that
+     *  type, which is also how a closed widget is brought back. */
+    private void openWidgetPicker() {
+        Dialog picker = new Dialog(getTranslation("gridstack.pickerTitle"));
+
+        VerticalLayout choices = new VerticalLayout();
+        choices.setPadding(false);
+        for (WidgetDefinition definition : widgets.definitions()) {
+            Button choice = new Button(getTranslation(definition.titleKey()), e -> {
+                addWidget(definition);
+                picker.close();
+            });
+            choice.setWidthFull();
+            choices.add(choice);
+        }
+
+        picker.add(choices);
+        picker.getFooter().add(new Button(getTranslation("form.cancel"), e -> picker.close()));
+        picker.open();
+    }
+
+    /** Adds a widget at the top left in its type's default size; gridstack
+     *  floats it down into the first free slot. */
+    private void addWidget(WidgetDefinition definition) {
         // The id must stay stable across reloads for the saved layout to match
         // it again, so it is derived from a counter rather than a random UUID.
-        // The counter only ever grows: closing a widget must not hand its id to
+        // The counter only ever grows, and it is shared across types: closing a
-        // the next one, or the new widget would inherit the closed one's saved
+        // widget must not hand its id to the next one, or the new widget would
-        // position.
+        // inherit the closed one's saved position.
         extraWidgetCount++;
-        String title = getTranslation("gridstack.widget", extraWidgetCount);
+        grid.add(widget(definition.type() + "-" + extraWidgetCount, definition,
-        grid.add(new GridStackItem("extra-" + extraWidgetCount, 0, 0, 4, 2,
+                0, 0, definition.width(), definition.height()));
-                new Card(title, new Paragraph(getTranslation("gridstack.widgetText")))));
+    }
+
+    /** 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
+     *  type's default size. */
+    private GridStackItem defaultWidget(String type, int x, int y, int w, int h) {
+        return widget(type, widgets.require(type), x, y, w, h);
+    }
+
+    private GridStackItem widget(String id, WidgetDefinition definition,
+                                 int x, int y, int w, int h) {
+        return new GridStackItem(id, x, y, w, h,
+                new Card(getTranslation(definition.titleKey()), definition.factory().get()));
     }
 
     /** The displayed value comes from the bundle alongside the label, since it
@@ -123,48 +152,6 @@ public class DashboardView extends VerticalLayout implements HasDynamicTitle {
                 .setSparkline(label, kpi.trend());
     }
 
-    private Component lineChart() {
-        return axisChart(new LineChart());
-    }
-
-    private Component barChart() {
-        return axisChart(new BarChart());
-    }
-
-    private Component axisChart(AxisChart chart) {
-        ChartSeries series = dataService.revenueByMonth();
-        chart.setData(getTranslation(series.nameKey()), series.values(),
-                translate(series.categoryKeys()));
-        chart.addPointClickListener(e -> Notification.show(getTranslation(
-                "chart.pointClick", e.getSeriesIndex(), e.getDataPointIndex())));
-        return sizeFull(chart);
-    }
-
-    private Component pieChart() {
-        ChartSeries series = dataService.revenueByRegion();
-        PieChart chart = new PieChart();
-        // A pie has no series name — its categories are the slice labels.
-        chart.setData(series.values(), translate(series.categoryKeys()));
-        chart.addPointClickListener(e -> Notification.show(
-                getTranslation("chart.sliceClick", e.getDataPointIndex())));
-        return sizeFull(chart);
-    }
-
-    /** Charts fill their grid item instead of using a fixed pixel height, so
-     *  resizing a widget resizes the chart (grid-stack.ts fires a window
-     *  resize on resizestop, which ApexCharts reflows on). */
-    private Component sizeFull(ApexChart chart) {
-        chart.setWidthFull();
-        chart.setHeight("100%");
-        return chart;
-    }
-
-    /** Data sets carry translation keys, not display text — see
-     *  {@link ChartSeries}. */
-    private List<String> translate(List<String> keys) {
-        return keys.stream().map(this::getTranslation).toList();
-    }
-
     @Override
     public String getPageTitle() {
         return getTranslation("page.dashboard");

src/main/java/com/example/widgets/WidgetDefinition.java

@@ -0,0 +1,23 @@
+package com.example.widgets;
+
+import com.vaadin.flow.component.Component;
+
+import java.util.function.Supplier;
+
+/**
+ * One kind of dashboard widget: what it is called, how large it starts out, and
+ * how to build its content. Definitions live in {@link WidgetRegistry}; the
+ * dashboard turns one into a grid item, so a definition knows nothing about
+ * gridstack or about where its widget ends up sitting.
+ *
+ * @param type     stable id of the widget kind — grid item ids are derived from
+ *                 it, so it must not change once a layout has been persisted
+ * @param titleKey translation key for the card title
+ * @param width    default width in grid columns
+ * @param height   default height in grid rows
+ * @param factory  builds the card content; called once per added widget, since
+ *                 a Vaadin component cannot be attached in two places
+ */
+public record WidgetDefinition(String type, String titleKey, int width, int height,
+                               Supplier<Component> factory) {
+}

src/main/java/com/example/widgets/WidgetRegistry.java

@@ -0,0 +1,102 @@
+package com.example.widgets;
+
+import com.example.components.ApexChart;
+import com.example.components.AxisChart;
+import com.example.components.BarChart;
+import com.example.components.LineChart;
+import com.example.components.PieChart;
+import com.example.data.ChartDataService;
+import com.example.data.ChartSeries;
+import com.vaadin.flow.component.Component;
+import com.vaadin.flow.component.notification.Notification;
+import org.springframework.stereotype.Service;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * The widget types a dashboard can show. Holding them here rather than inline in
+ * the view is what lets the "add widget" picker offer real content and lets a
+ * closed widget be brought back: the view only decides <em>where</em> a widget
+ * sits, the registry decides what one <em>is</em>.
+ * <p>
+ * Numbers come from {@link ChartDataService}; the factories resolve the data's
+ * translation keys against the bundle through the chart component itself, so
+ * they follow the current UI locale without the registry being a component.
+ */
+@Service
+public class WidgetRegistry {
+
+    public static final String REVENUE_TREND = "revenue-trend";
+    public static final String REVENUE_MONTH = "revenue-month";
+    public static final String REVENUE_REGION = "revenue-region";
+
+    private final ChartDataService dataService;
+    /** Insertion-ordered: the picker lists definitions in registration order. */
+    private final Map<String, WidgetDefinition> definitions = new LinkedHashMap<>();
+
+    public WidgetRegistry(ChartDataService dataService) {
+        this.dataService = dataService;
+
+        register(new WidgetDefinition(REVENUE_TREND, "card.revenueTrend", 6, 3,
+                () -> axisChart(new LineChart())));
+        register(new WidgetDefinition(REVENUE_MONTH, "card.revenueByMonth", 6, 3,
+                () -> axisChart(new BarChart())));
+        register(new WidgetDefinition(REVENUE_REGION, "card.revenueByRegion", 5, 3,
+                this::pieChart));
+    }
+
+    /** Adds a definition, replacing any earlier one of the same type. */
+    public final void register(WidgetDefinition definition) {
+        definitions.put(definition.type(), definition);
+    }
+
+    public List<WidgetDefinition> definitions() {
+        return List.copyOf(definitions.values());
+    }
+
+    /** The definition for the given type. Unknown types are a programming
+     *  error, not user input — the picker only ever offers registered ones. */
+    public WidgetDefinition require(String type) {
+        WidgetDefinition definition = definitions.get(type);
+        if (definition == null) {
+            throw new IllegalArgumentException("unknown widget type: " + type);
+        }
+        return definition;
+    }
+
+    private Component axisChart(AxisChart chart) {
+        ChartSeries series = dataService.revenueByMonth();
+        chart.setData(chart.getTranslation(series.nameKey()), series.values(),
+                translate(chart, series.categoryKeys()));
+        chart.addPointClickListener(e -> Notification.show(chart.getTranslation(
+                "chart.pointClick", e.getSeriesIndex(), e.getDataPointIndex())));
+        return sizeFull(chart);
+    }
+
+    private Component pieChart() {
+        ChartSeries series = dataService.revenueByRegion();
+        PieChart chart = new PieChart();
+        // A pie has no series name — its categories are the slice labels.
+        chart.setData(series.values(), translate(chart, series.categoryKeys()));
+        chart.addPointClickListener(e -> Notification.show(
+                chart.getTranslation("chart.sliceClick", e.getDataPointIndex())));
+        return sizeFull(chart);
+    }
+
+    /** Charts fill their grid item instead of using a fixed pixel height, so
+     *  resizing a widget resizes the chart (grid-stack.ts fires a window
+     *  resize on resizestop, which ApexCharts reflows on). */
+    private Component sizeFull(ApexChart chart) {
+        chart.setWidthFull();
+        chart.setHeight("100%");
+        return chart;
+    }
+
+    /** Data sets carry translation keys, not display text — see
+     *  {@link ChartSeries}. */
+    private List<String> translate(Component component, List<String> keys) {
+        return keys.stream().map(component::getTranslation).toList();
+    }
+}

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

@@ -27,8 +27,7 @@ gridstack.addWidget=Widget hinzufügen
 gridstack.reset=Layout zurücksetzen
 gridstack.status=Layout geändert – {0} Widgets
 gridstack.statusInitial=Layout unverändert
-gridstack.widget=Widget {0}
+gridstack.pickerTitle=Widget auswählen
-gridstack.widgetText=Frei platzierbare Karte.
 gridstack.dragHandle=Verschieben
 gridstack.close=Schließen
 

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

@@ -27,8 +27,7 @@ gridstack.addWidget=Add widget
 gridstack.reset=Reset layout
 gridstack.status=Layout changed – {0} widgets
 gridstack.statusInitial=Layout unchanged
-gridstack.widget=Widget {0}
+gridstack.pickerTitle=Choose a widget
-gridstack.widgetText=Freely placeable card.
 gridstack.dragHandle=Move
 gridstack.close=Close
 

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

@@ -27,8 +27,7 @@ gridstack.addWidget=Añadir widget
 gridstack.reset=Restablecer diseño
 gridstack.status=Diseño modificado – {0} widgets
 gridstack.statusInitial=Diseño sin cambios
-gridstack.widget=Widget {0}
+gridstack.pickerTitle=Elegir un widget
-gridstack.widgetText=Tarjeta de colocación libre.
 gridstack.dragHandle=Mover
 gridstack.close=Cerrar
 

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

@@ -7,14 +7,18 @@ import com.example.components.GridStackLayout;
 import com.example.components.KpiTile;
 import com.example.components.LineChart;
 import com.example.components.PieChart;
+import com.example.widgets.WidgetDefinition;
+import com.example.widgets.WidgetRegistry;
 import com.vaadin.browserless.SpringBrowserlessTest;
 import com.vaadin.browserless.ViewPackages;
 import com.vaadin.browserless.internal.ElementUtilsKt;
 import com.vaadin.flow.component.button.Button;
+import com.vaadin.flow.component.dialog.Dialog;
 import com.vaadin.flow.component.html.Div;
 import com.vaadin.flow.dom.DomEvent;
 import com.vaadin.flow.internal.JacksonUtils;
 import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
 
 import java.util.List;
@@ -31,6 +35,9 @@ class DashboardViewTest extends SpringBrowserlessTest {
     /** Four KPI tiles plus three charts and the hint card. */
     private static final int DEFAULT_WIDGETS = 8;
 
+    @Autowired
+    private WidgetRegistry registry;
+
     @Test
     void view_rendersGridWithDefaultWidgets() {
         navigate(DashboardView.class);
@@ -92,12 +99,78 @@ class DashboardViewTest extends SpringBrowserlessTest {
         int initial = grid.getLayout().size();
 
         button("gridstack.addWidget").click();
+        assertEquals(initial, grid.getLayout().size(),
+                "opening the picker adds nothing on its own");
+
+        pickWidget("card.revenueByRegion");
         assertEquals(initial + 1, grid.getLayout().size());
 
         clickCloseButton($view(GridStackItem.class).last());
         assertEquals(initial, grid.getLayout().size());
     }
 
+    @Test
+    void addWidget_ofAChosenType_rendersRealContent() {
+        navigate(DashboardView.class);
+
+        button("gridstack.addWidget").click();
+        pickWidget("card.revenueByRegion");
+
+        assertEquals(2, $view(PieChart.class).all().size(),
+                "the added widget renders a chart, not a placeholder");
+        GridStackItem added = $view(GridStackItem.class).last();
+        assertTrue(added.getItemId().startsWith("revenue-region-"),
+                "unexpected id: " + added.getItemId());
+    }
+
+    @Test
+    void closedWidget_canBeAddedAgainWithAFreshId() {
+        navigate(DashboardView.class);
+
+        GridStackLayout grid = $view(GridStackLayout.class).first();
+        clickCloseButton(itemById("revenue-trend"));
+        assertEquals(0, $view(LineChart.class).all().size(), "the line chart is gone");
+
+        button("gridstack.addWidget").click();
+        pickWidget("card.revenueTrend");
+
+        List<GridStackItem.Position> layout = grid.getLayout();
+        assertEquals(DEFAULT_WIDGETS, layout.size());
+        assertEquals(1, $view(LineChart.class).all().size(), "the line chart is back");
+        // The closed widget's id must never be handed out again, or the new
+        // widget would inherit its persisted position.
+        assertFalse(layout.stream().anyMatch(p -> "revenue-trend".equals(p.id())));
+        assertTrue(layout.stream().anyMatch(p -> p.id().startsWith("revenue-trend-")));
+    }
+
+    @Test
+    void addingTheSameTypeTwice_yieldsDistinctIds() {
+        navigate(DashboardView.class);
+
+        button("gridstack.addWidget").click();
+        pickWidget("card.revenueByMonth");
+        button("gridstack.addWidget").click();
+        pickWidget("card.revenueByMonth");
+
+        List<String> ids = $view(GridStackLayout.class).first().getLayout().stream()
+                .map(GridStackItem.Position::id).toList();
+        assertEquals(ids.size(), ids.stream().distinct().count(), "duplicate ids: " + ids);
+    }
+
+    @Test
+    void picker_listsEveryRegisteredWidgetType() {
+        navigate(DashboardView.class);
+
+        button("gridstack.addWidget").click();
+
+        Dialog picker = $(Dialog.class).first();
+        for (WidgetDefinition definition : registry.definitions()) {
+            assertNotNull($(Button.class).from(picker)
+                            .withText(translate(definition.titleKey())).first(),
+                    "picker is missing " + definition.type());
+        }
+    }
+
     @Test
     void closeButton_removesTheClickedWidgetOnly() {
         navigate(DashboardView.class);
@@ -149,6 +222,17 @@ class DashboardViewTest extends SpringBrowserlessTest {
                 new DomEvent(closeButton.getElement(), "click", JacksonUtils.createObjectNode()));
     }
 
+    /** Clicks a widget type in the picker dialog "add widget" opened. */
+    private void pickWidget(String titleKey) {
+        Dialog picker = $(Dialog.class).first();
+        $(Button.class).from(picker).withText(translate(titleKey)).first().click();
+    }
+
+    private GridStackItem itemById(String id) {
+        return $view(GridStackItem.class)
+                .withCondition(item -> id.equals(item.getItemId())).single();
+    }
+
     private Button button(String translationKey) {
         return $view(Button.class).withText(translate(translationKey)).first();
     }

src/test/java/com/example/widgets/WidgetRegistryTest.java

@@ -0,0 +1,48 @@
+package com.example.widgets;
+
+import com.example.data.InMemoryChartDataService;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * The definitions themselves — the factories need a Vaadin UI to resolve
+ * translations, so they are exercised from {@code DashboardViewTest} instead.
+ */
+class WidgetRegistryTest {
+
+    private final WidgetRegistry registry = new WidgetRegistry(new InMemoryChartDataService());
+
+    @Test
+    void definitions_areListedInRegistrationOrder() {
+        List<String> types = registry.definitions().stream()
+                .map(WidgetDefinition::type).toList();
+
+        assertEquals(List.of(WidgetRegistry.REVENUE_TREND, WidgetRegistry.REVENUE_MONTH,
+                WidgetRegistry.REVENUE_REGION), types);
+        assertTrue(registry.definitions().stream()
+                        .allMatch(d -> d.width() > 0 && d.height() > 0),
+                "every definition carries a usable default size");
+    }
+
+    @Test
+    void require_rejectsAnUnknownType() {
+        assertThrows(IllegalArgumentException.class, () -> registry.require("nope"));
+    }
+
+    @Test
+    void register_replacesTheDefinitionOfTheSameType() {
+        int before = registry.definitions().size();
+        WidgetDefinition replacement = new WidgetDefinition(WidgetRegistry.REVENUE_TREND,
+                "card.revenueTrend", 2, 2, () -> null);
+
+        registry.register(replacement);
+
+        assertEquals(before, registry.definitions().size());
+        assertEquals(replacement, registry.require(WidgetRegistry.REVENUE_TREND));
+    }
+}