This is an automated email from the ASF dual-hosted git repository.

mattcasters pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git


The following commit(s) were added to refs/heads/main by this push:
     new 82aec8e80f Issue #8117 : Lay out annotated widget groups as boxes 
(#8636)
82aec8e80f is described below

commit 82aec8e80f0b1f830dc4677eae3f30c0492c60b5
Author: Matt Casters <[email protected]>
AuthorDate: Sun Sep 27 16:49:58 2026 +0200

    Issue #8117 : Lay out annotated widget groups as boxes (#8636)
    
    * Issue #8117 : Lay out annotated widget groups as boxes
    
    * Issue #8117 : Document annotation derived widgets
    
    * Issue #8117 : Size widget boxes from their content
    
    Percentage bands reported no preferred height, so a stack of boxes grew to 
the tallest box times the number of boxes. A single-column grid keeps the 
preferred height at the sum of the boxes, shares extra space, and scrolls 
inside a box that is squeezed.
---
 .../hop/core/gui/plugin/GuiWidgetGroupType.java    |   5 +-
 .../hop/core/gui/plugin/GuiWidgetGroupsTest.java   |   8 +
 docs/hop-dev-manual/modules/ROOT/nav.adoc          |   1 +
 .../ROOT/pages/annotation-derived-widgets.adoc     | 383 ++++++++++++++++++++
 .../modules/ROOT/pages/plugin-types/gui.adoc       |   3 +-
 .../ui/core/gui/GuiCompositeWidgetsGroupTest.java  | 393 +++++++++++++++++++++
 .../hop/ui/core/gui/GuiCompositeWidgets.java       | 110 ++++--
 7 files changed, 865 insertions(+), 38 deletions(-)

diff --git 
a/core/src/main/java/org/apache/hop/core/gui/plugin/GuiWidgetGroupType.java 
b/core/src/main/java/org/apache/hop/core/gui/plugin/GuiWidgetGroupType.java
index b56836b12b..554fb8d5fc 100644
--- a/core/src/main/java/org/apache/hop/core/gui/plugin/GuiWidgetGroupType.java
+++ b/core/src/main/java/org/apache/hop/core/gui/plugin/GuiWidgetGroupType.java
@@ -19,7 +19,10 @@ package org.apache.hop.core.gui.plugin;
 
 /**
  * How {@link GuiWidgetElement} fields that share a {@link 
GuiWidgetElement#group()} are laid out
- * inside one {@code parentId} tree. {@link #NONE} keeps the current 
single-form layout.
+ * inside one {@code parentId} tree. {@link #NONE} keeps the single-form 
layout. {@link #BOXES}
+ * stacks one box per group. The stack's preferred height is the boxes put 
together, extra space in
+ * the parent is shared, and a box that is squeezed scrolls its own fields. 
{@link #LIST} is not
+ * implemented and falls back to tabs.
  */
 public enum GuiWidgetGroupType {
   NONE,
diff --git 
a/core/src/test/java/org/apache/hop/core/gui/plugin/GuiWidgetGroupsTest.java 
b/core/src/test/java/org/apache/hop/core/gui/plugin/GuiWidgetGroupsTest.java
index 89b5e7e97c..7cccc8661b 100644
--- a/core/src/test/java/org/apache/hop/core/gui/plugin/GuiWidgetGroupsTest.java
+++ b/core/src/test/java/org/apache/hop/core/gui/plugin/GuiWidgetGroupsTest.java
@@ -59,6 +59,14 @@ class GuiWidgetGroupsTest {
     assertEquals("Tab", buckets.get(1).getLabel());
   }
 
+  @Test
+  void boxesStayBoxes() {
+    GuiElements first = element("first", "One", "10", 
GuiWidgetGroupType.BOXES);
+    GuiElements second = element("second", "Two", "20", 
GuiWidgetGroupType.BOXES);
+    assertFalse(GuiWidgetGroups.hasMixedTypes(List.of(first, second)));
+    assertEquals(GuiWidgetGroupType.BOXES, 
GuiWidgetGroups.typeOf(List.of(first, second)));
+  }
+
   @Test
   void mixedTypesFallBackToTabs() {
     GuiElements tabs = element("a", "A", "10", GuiWidgetGroupType.TABS);
diff --git a/docs/hop-dev-manual/modules/ROOT/nav.adoc 
b/docs/hop-dev-manual/modules/ROOT/nav.adoc
index 57bca75883..dc29c9e4d3 100644
--- a/docs/hop-dev-manual/modules/ROOT/nav.adoc
+++ b/docs/hop-dev-manual/modules/ROOT/nav.adoc
@@ -51,6 +51,7 @@ under the License.
 *** xref:database/testing.adoc[Testing a database plugin]
 *** xref:database/migrating-from-variants.adoc[Migrating from isXVariant()]
 ** xref:gui-plugins-toolbars.adoc[GUI plugins and toolbars]
+** xref:annotation-derived-widgets.adoc[Annotation derived widgets]
 ** xref:engine-compatibility.adoc[Engine compatibility]
 ** xref:lineage.adoc[Lineage observation hub]
 ** xref:internationalisation.adoc[Internationalisation (i18n)]
diff --git 
a/docs/hop-dev-manual/modules/ROOT/pages/annotation-derived-widgets.adoc 
b/docs/hop-dev-manual/modules/ROOT/pages/annotation-derived-widgets.adoc
new file mode 100644
index 0000000000..eda3e952b5
--- /dev/null
+++ b/docs/hop-dev-manual/modules/ROOT/pages/annotation-derived-widgets.adoc
@@ -0,0 +1,383 @@
+////
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+  http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+////
+:description: How to build a metadata editor, transform dialog, or action 
dialog from @GuiWidgetElement annotations and GuiCompositeWidgets.
+[[AnnotationDerivedWidgets]]
+= Annotation derived widgets
+
+A metadata editor, a transform dialog, or an action dialog does not have to 
lay its fields out by hand.
+Put `@GuiWidgetElement` on the fields of the object being edited, and let 
`GuiCompositeWidgets` build the controls.
+
+The annotated class holds the values.
+The editor or dialog creates the shell, the name line, and the buttons, then 
asks `GuiCompositeWidgets` to fill the space between the name and the buttons.
+On OK, one call copies the widgets back onto the object.
+Do not copy the fields yourself.
+
+`@GuiWidgetElement` is one of the contribution annotations on a `@GuiPlugin`.
+The xref:plugin-types/gui.adoc[GUI plugin types] page lists the others (menus, 
toolbars, shortcuts).
+This page is only about the widgets those annotations produce.
+
+== Annotating the class
+
+The class needs `@GuiPlugin`, so the Hop GUI scans it at startup, and getters 
and setters for every annotated field.
+Lombok `@Getter` and `@Setter` are the usual way to get those.
+A field that is saved also carries `@HopMetadataProperty`.
+The widget annotation does not persist anything.
+
+Give the class a parent-id constant and put that same string on every widget.
+`GuiCompositeWidgets` looks widgets up by the runtime class name and this id, 
so the object you pass in has to be an instance of the `@GuiPlugin` class.
+Fields declared on a superclass are picked up, but they are registered under 
the plugin class.
+
+[source,java]
+----
+@Getter
+@Setter
+@GuiPlugin
+public class DataSetOutputMeta extends BaseTransformMeta<DataSetOutput, 
DataSetOutputData> {
+
+  public static final String GUI_PLUGIN_ELEMENT_PARENT_ID = 
"DATA_SET_OUTPUT_DIALOG_OPTIONS";
+
+  @GuiWidgetElement(
+      id = "dataSetName",
+      order = "0100",
+      type = GuiElementType.METADATA,
+      metadata = DataSet.class,
+      label = "i18n::DataSetOutputMeta.DataSetName.Label",
+      toolTip = "i18n::DataSetOutputMeta.DataSetName.Tooltip",
+      parentId = GUI_PLUGIN_ELEMENT_PARENT_ID,
+      groupType = GuiWidgetGroupType.BOXES,
+      group = "Data Set")
+  @HopMetadataProperty(hopMetadataPropertyType = 
HopMetadataPropertyType.PIPELINE_DATA_SET)
+  private String dataSetName;
+}
+----
+
+`id`::
+Stable name of this widget.
+Dialog code uses it to find the control, and a distribution can hide the 
widget by listing the id in `disabledGuiElements`.
+See xref:manual::hop-gui/disable-ui-elements.adoc[Disabling UI elements].
+
+`order`::
+Sort key among the widgets in this parent, compared as a string.
+`0100`, `0200`, `0300` sort numerically.
+`10` sorts after `100`.
+
+`label` and `toolTip`::
+Shown next to the control.
+Use an `i18n::` key, as described in 
xref:internationalisation.adoc[Internationalisation].
+In the properties file, quote a variable expression with single quotes: 
`Folder='${PROJECT_HOME}'`.
+
+`parentId`::
+The form this widget belongs to.
+One class can have several forms by using several parent ids.
+The same parent-id string can be reused on a different class, because lookup 
is by class and id together.
+Variable resolvers all use `VariableResolver.GUI_PLUGIN_ELEMENT_PARENT_ID` for 
that reason.
+
+A `@GuiPlugin` that is also metadata and lives in a plugin classloader group 
must set `classLoaderGroup` on `@GuiPlugin`.
+Without it the GUI loads a second copy of the class.
+xref:plugin-types/gui.adoc[GUI] describes the failure.
+
+== Widget types
+
+`type` selects the control.
+
+[cols="1,3"]
+|===
+|Type |What is built
+
+|`TEXT`
+|Single-line text.
+`password = true` masks it.
+`variables` defaults to true and wraps the text in a variable-aware field; set 
it to false for a plain value such as a configuration checkbox's companion 
number.
+
+|`MULTI_LINE_TEXT`
+|Multi-line text.
+`multiLineTextHeight` is the height in lines, not pixels.
+
+|`FILENAME`
+|Text with a browse button that asks for a file.
+`typeFilename` can supply extensions and filters.
+
+|`FOLDER`
+|Text with a browse button that asks for a folder.
+
+|`COMBO`
+|A combo box.
+If the field type is an enum, the items are the enum constants' `toString()` 
and the selected item is read back with `Enum.valueOf`.
+Do not give the enum a custom `toString()`: the displayed text would no longer 
be the constant name, and the previous value would be kept.
+For a list that is not an enum, set `comboValuesMethod` to a method on the 
object that returns `String[]`.
+A dialog can also fill the combo later with 
`GuiCompositeWidgets.setComboValues(id, items)`, which is how a transform 
dialog lists the incoming stream fields.
+
+|`CHECKBOX`
+|A checkbox bound to a `boolean`.
+
+|`METADATA`
+|A metadata selection line.
+`metadata` is the metadata class.
+`metadataKey` is the alternative when the dialog must not compile against that 
plugin: pass the metadata type key, for example `ai-provider`.
+When that plugin is not installed the widget is left out.
+
+|`BUTTON`
+|A push button, annotated on a method rather than a field: `public void 
chooseColor(Object object)`.
+The method is called on the object being edited, and that same object is the 
argument.
+When the method returns, the widgets are filled from the object again, so a 
change to a field shows up next to the button.
+
+|`LINK`
+|An underlined link, annotated on a method.
+`public void overviewLink(Event event)` receives the SWT selection event; 
`event.text` is the text of the `<a>` anchor in the label.
+The method is called on a new instance of the class, not on the object being 
edited.
+
+|`COMPOSITE`
+|A method `public void welcome(Composite parent)` that paints its own controls 
into the composite it is given.
+Use this when the annotation types above cannot express the control.
+|===
+
+`namingSchemeType` marks a text widget as a name (`file`, `folder`, 
`hop-variable`, and so on) so the naming-scheme indicator is shown.
+`FILENAME` and `FOLDER` infer `file` and `folder`.
+An empty value on any other type means the widget is not a name.
+
+== Groups
+
+By default the widgets of one `parentId` are a single flat form.
+Set `group` and the widgets are placed in a container instead.
+Repeat `groupType` on every field.
+The first value that is not `NONE` wins.
+If two fields ask for different types, the GUI logs an error and shows tabs.
+
+[cols="1,3"]
+|===
+|`groupType` |Layout
+
+|`NONE`
+|Flat form.
+This is the default, and it only applies when no sibling has a group.
+
+|`TABS`
+|One tab per group, in a tab folder that fills the area between the name line 
and the buttons.
+Each tab scrolls its own fields.
+`groupImage` is the tab icon.
+Parquet output and the dbt action use tabs.
+
+|`BOXES`
+|One SWT group box per group name.
+The boxes stack in the area between the name line and the buttons.
+Each box starts at the height of its own fields, and extra space in that area 
is shared between the boxes.
+A box scrolls its own fields when the area is too short for them.
+A single group fills the whole area: Data set output is one box titled "Data 
Set", and the Database perspective options are one box titled "SQL".
+Text chunker uses three boxes (Input, Chunking, Chunk metadata).
+
+|`LIST`
+|Not implemented.
+The GUI logs that and shows tabs.
+|===
+
+`group` is the title.
+An `i18n::` key is translated.
+`groupOrder` sorts the groups, again as a string, the same way `order` sorts 
fields.
+A field with an empty `group` lands in a group titled "General" when any 
sibling has a group.
+
+== Transform and action dialogs
+
+Extend `BaseTransformDialog` or `ActionDialog`.
+In `open()`, create the shell, build the button bar, then add the widgets 
between the name and the OK button.
+
+[source,java]
+----
+@Override
+public String open() {
+  createShell(BaseMessages.getString(PKG, "DataSetOutputDialog.Shell.Title"));
+  buildButtonBar().ok(e -> ok()).cancel(e -> cancel()).build();
+
+  widgets =
+      GuiCompositeWidgets.addScrolledComposite(
+          shell,
+          variables,
+          wTransformName,
+          wOk,
+          DataSetOutputMeta.GUI_PLUGIN_ELEMENT_PARENT_ID,
+          input);
+
+  BaseDialog.defaultShellHandling(shell, c -> ok(), c -> cancel());
+  return transformName;
+}
+
+private void ok() {
+  if (Utils.isEmpty(wTransformName.getText())) {
+    return;
+  }
+  widgets.getWidgetsContents(input, 
DataSetOutputMeta.GUI_PLUGIN_ELEMENT_PARENT_ID);
+  transformName = wTransformName.getText();
+  dispose();
+}
+----
+
+`addScrolledComposite` attaches a scrolled composite below the control you 
pass as the top (`wTransformName` in a transform dialog, `wName` in an action 
dialog) and above the control you pass as the bottom (`wOk`).
+The annotated widgets are created inside it, and the current values are copied 
in.
+`getWidgetsContents` on OK copies them back.
+
+That is what keeps the dialog resizable with the buttons pinned under the 
fields.
+A flat row of `FormAttachment`s on the shell fights the button bar for the 
same vertical space, and the buttons end up on top of the lower widgets.
+Groups make this reliable.
+The tab folder fills the scrolled area, and each tab scrolls its own fields.
+The stack of boxes fills that same area.
+Its preferred height is the height of the fields put together, so a scrolled 
parent shows a scrollbar only when the fields themselves do not fit.
+Extra space is shared between the boxes, and a box scrolls its own fields when 
the dialog is too short for them.
+
+Pass `createCompositeWidgets` a composite that is stretched to the bottom of 
the available area, with a bottom attachment at 100 percent.
+`addScrolledComposite` does this, from the name line down to the OK button.
+That stretch gives the boxes the extra space to share, and it squeezes a box 
until the box scrolls, when the dialog is short.
+A composite that only wraps the preferred height leaves every box at the 
height of its fields.
+
+An action dialog has the same shape.
+`ActionDbtDialog` is the worked example for tabs plus an extra table.
+
+A class annotated with `@ConfigPlugin` is laid out with the label above the 
control and the control at full width.
+Transform and action metadata keep the label on the left.
+The Database perspective options use the label-above layout because that class 
is a configuration plugin.
+Data set output uses the label on the left.
+
+== Metadata editors
+
+A metadata editor extends `MetadataEditor`.
+The name field is still created by the editor.
+The rest of the fields come from the metadata class, which is both 
`@HopMetadata` and `@GuiPlugin`.
+
+`NamingSchemeEditor` is a compact example.
+It builds the name, registers one extra group, then creates the annotated 
widgets under the name:
+
+[source,java]
+----
+widgets = new GuiCompositeWidgets(hopGui.getVariables());
+widgets.createCompositeWidgets(
+    getMetadata(), null, parent, NamingScheme.GUI_PLUGIN_ELEMENT_PARENT_ID, 
wName);
+----
+
+`setWidgetsContent()` calls `widgets.setWidgetsContents(metadata, parent, 
parentId)`.
+`getWidgetsContent(metadata)` calls `widgets.getWidgetsContents(metadata, 
parentId)`.
+
+Variable resolvers, database connection options, and the other metadata types 
that contribute fields to a shared editor use the same pair of annotations.
+The editor already owns the composite; the plugin only annotates its fields 
and sets `parentId` to the constant that editor looks up.
+For a variable resolver that constant is 
`VariableResolver.GUI_PLUGIN_ELEMENT_PARENT_ID`.
+
+== Configuration perspective
+
+A `@ConfigPlugin` can expose the same options in the Configuration perspective.
+Annotate the picocli fields with `@GuiPlugin` on the class and 
`@GuiWidgetElement` on each field, and use 
`ConfigPluginOptionsTab.GUI_WIDGETS_PARENT_ID` as the parent id.
+The class needs a static `getInstance()` that returns the object holding the 
current values.
+The perspective calls it, builds the widgets, and saves through the widget 
listener.
+
+`DatabasePerspectiveConfigPlugin` is one box, group `"SQL"`, `groupType = 
BOXES`: the auto-connect checkbox, the select-executed-SQL checkbox, and the 
query row limit.
+xref:plugin-types/metadata-configuration.adoc[Metadata and configuration] 
covers the command-line side of a configuration plugin.
+
+== Controls the annotation cannot express
+
+A `TableView`, a preview, or a painter does not fit a `@GuiWidgetElement` 
field.
+Register it as an extra group *before* the widgets are created, and build it 
into the composite the callback receives.
+That composite is the same one the annotated fields of that group are on, 
inside the tab or the box, so a `FormAttachment` to one of those fields is 
valid.
+A table that should fill the group sets its bottom attachment to 100 percent.
+
+[source,java]
+----
+widgets =
+    GuiCompositeWidgets.addScrolledComposite(
+        shell,
+        variables,
+        wTransformName,
+        wOk,
+        ParquetOutputMeta.GUI_PLUGIN_ELEMENT_PARENT_ID,
+        input,
+        w -> {
+          widgets = w;
+          w.registerExtraGroup(
+              BaseMessages.getString(PKG, "ParquetOutputMeta.Group.Fields"),
+              "0400",
+              null,
+              this::addFieldsTable);
+        });
+----
+
+The `beforeCreate` argument exists because `addScrolledComposite` creates the 
fields before it returns.
+`registerExtraGroup` after that call is too late.
+The first argument is the group title, the second is the `groupOrder`, and the 
third is an optional image (tab icon).
+Pass `null` for the image when the group is a box.
+
+`ActionDbtDialog` puts two name/value tables in an extra group.
+`NamingSchemeEditor` puts its preview there.
+`AiProviderEditor` puts the per-role model table there.
+
+== Values, listeners, and hidden rows
+
+`setWidgetsContents(source, parent, parentId)` copies the object into the 
widgets.
+`getWidgetsContents(source, parentId)` copies the widgets back.
+`getWidgetsMap()` is the control for each id.
+`getLabelsMap()` is the label.
+`getActionWidgetsMap()` is the extra control on the right of a field, today 
the browse button of a `FILENAME` or `FOLDER`.
+
+Set a listener with `setWidgetsListener`.
+`GuiCompositeWidgetsAdapter` implements the four methods as no-ops, so a 
dialog overrides only what it needs:
+
+`widgetModified`::
+A field changed.
+Mark the transform, action, or metadata object changed, and enable or disable 
other fields.
+Compare `widgetId` with the id constants on the meta class.
+
+`widgetsPopulated`::
+Values have been copied in, just before the dialog is shown.
+
+`widgetsCreated`::
+The controls exist.
+Do not create fields here.
+
+`persistContents`::
+The listener wants the extra state written back.
+Tables registered as extra groups are not annotated fields, so the dialog 
reads them itself, usually from `persistContents` or from the OK handler.
+
+There is one listener slot.
+An editor that needs the callback for its own change tracking, and a plugin 
object that also implements `IGuiPluginCompositeWidgetsListener`, has to 
forward the calls.
+`VariableResolverEditor` does this.
+
+`setWidgetsHidden(source, hiddenIds)` hides the named widgets and closes the 
gap they would leave.
+Call it from `widgetModified` when an option only applies to some settings.
+Hiding a field in one tab or one box does not move the fields in another.
+Vault resolvers use it to show only the credentials of the selected 
authentication type.
+
+== Where to look
+
+[cols="2,3"]
+|===
+|Example |What it shows
+
+|`DataSetOutputMeta`, `DataSetOutputDialog`
+|One box in a transform dialog, metadata picker, folder, text, checkboxes.
+
+|`TextChunkerMeta`, `TextChunkerDialog`
+|Three boxes, and a listener that enables fields from a combo.
+
+|`ActionDbt`, `ActionDbtDialog`
+|Tabs in an action dialog, plus an extra group that holds two tables.
+
+|`ParquetOutputMeta`, `ParquetOutputDialog`
+|Tabs and extra groups whose tables fill the tab (`bottom` at 100 percent).
+
+|`NamingScheme`, `NamingSchemeEditor`
+|A metadata editor: name field, annotated fields, extra preview group.
+
+|`DatabasePerspectiveConfigPlugin`
+|A configuration-perspective page, one box, label above the control.
+
+|`BaseVaultVariableResolver`
+|Several boxes on a metadata object, and rows hidden from a listener.
+|===
diff --git a/docs/hop-dev-manual/modules/ROOT/pages/plugin-types/gui.adoc 
b/docs/hop-dev-manual/modules/ROOT/pages/plugin-types/gui.adoc
index cc67723c5c..1b6d6a5aae 100644
--- a/docs/hop-dev-manual/modules/ROOT/pages/plugin-types/gui.adoc
+++ b/docs/hop-dev-manual/modules/ROOT/pages/plugin-types/gui.adoc
@@ -39,7 +39,8 @@ Most contribution annotations live in 
`org.apache.hop.core.gui.plugin` and its s
 
 `@GuiMenuElement`:: an item in the main menu
 `@GuiToolbarElement` and `@GuiToolbarElementFilter`:: a toolbar button, and a 
rule for when it is shown
-`@GuiWidgetElement`:: a widget on a settings page or metadata editor.
+`@GuiWidgetElement`:: a widget on a metadata editor, a transform dialog, an 
action dialog, or a settings page.
+xref:annotation-derived-widgets.adoc[Annotation derived widgets] is the guide 
for building those screens with `GuiCompositeWidgets`.
 For a metadata combo without a compile dependency on that plugin, set 
`metadataKey` (for example `ai-provider`) instead of `metadata`.
 When the plugin is not installed the widget is omitted.
 `@GuiContextAction` and `@GuiContextActionFilter` (in 
`org.apache.hop.core.action`):: an entry in a context dialog or right-click menu
diff --git 
a/rcp/src/test/java/org/apache/hop/ui/core/gui/GuiCompositeWidgetsGroupTest.java
 
b/rcp/src/test/java/org/apache/hop/ui/core/gui/GuiCompositeWidgetsGroupTest.java
index 775636d5f7..0d785d948e 100644
--- 
a/rcp/src/test/java/org/apache/hop/ui/core/gui/GuiCompositeWidgetsGroupTest.java
+++ 
b/rcp/src/test/java/org/apache/hop/ui/core/gui/GuiCompositeWidgetsGroupTest.java
@@ -26,6 +26,8 @@ import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Set;
 import lombok.Getter;
 import lombok.Setter;
@@ -37,11 +39,18 @@ import org.apache.hop.core.gui.plugin.GuiWidgetGroupType;
 import org.apache.hop.core.variables.Variables;
 import org.apache.hop.ui.core.widget.TextVar;
 import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.eclipse.swt.SWT;
 import org.eclipse.swt.custom.CTabFolder;
+import org.eclipse.swt.custom.ScrolledComposite;
+import org.eclipse.swt.graphics.Point;
+import org.eclipse.swt.graphics.Rectangle;
+import org.eclipse.swt.layout.FormAttachment;
 import org.eclipse.swt.layout.FormData;
 import org.eclipse.swt.layout.FormLayout;
+import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Group;
 import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Shell;
 import org.junit.jupiter.api.BeforeAll;
@@ -53,11 +62,17 @@ class GuiCompositeWidgetsGroupTest extends SwtBotTestBase {
 
   private static final String FLAT_PARENT = 
"GuiCompositeWidgetsGroupTest-flat";
   private static final String GROUPED_PARENT = 
"GuiCompositeWidgetsGroupTest-grouped";
+  private static final String BOXES_PARENT = 
"GuiCompositeWidgetsGroupTest-boxes";
+  private static final String SINGLE_BOX_PARENT = 
"GuiCompositeWidgetsGroupTest-single-box";
+  private static final String UNEVEN_PARENT = 
"GuiCompositeWidgetsGroupTest-uneven";
 
   @BeforeAll
   static void registerSampleWidgets() {
     register(FlatSample.class);
     register(GroupedSample.class);
+    register(BoxesSample.class);
+    register(SingleBoxSample.class);
+    register(UnevenBoxesSample.class);
   }
 
   @Test
@@ -225,6 +240,251 @@ class GuiCompositeWidgetsGroupTest extends SwtBotTestBase 
{
     }
   }
 
+  @Test
+  void boxedWidgetsAreGroupsThatScrollAndRoundTripValues() {
+    Shell shell = new Shell(display);
+    shell.setLayout(new FormLayout());
+    try {
+      BoxesSample source = new BoxesSample();
+      source.setFirst("one");
+      source.setSecond("two");
+      GuiCompositeWidgets widgets = new GuiCompositeWidgets(new Variables());
+      widgets.createCompositeWidgets(source, null, shell, BOXES_PARENT, null);
+      widgets.setWidgetsContents(source, shell, BOXES_PARENT);
+
+      assertNull(findTabFolderDeep(shell));
+      List<Group> groups = findGroups(shell);
+      assertEquals(2, groups.size());
+      assertEquals("First box", groups.get(0).getText());
+      assertEquals("Second box", groups.get(1).getText());
+
+      Control first = widgets.getWidgetsMap().get("first");
+      Control second = widgets.getWidgetsMap().get("second");
+      assertNotNull(first);
+      assertNotNull(second);
+      assertInstanceOf(ScrolledComposite.class, first.getParent().getParent());
+      assertInstanceOf(ScrolledComposite.class, 
second.getParent().getParent());
+      assertEquals(groups.get(0), first.getParent().getParent().getParent());
+      assertEquals(groups.get(1), second.getParent().getParent().getParent());
+
+      source.setFirst("uno");
+      source.setSecond("dos");
+      widgets.setWidgetsContents(source, shell, BOXES_PARENT);
+      widgets.getWidgetsContents(source, BOXES_PARENT);
+      assertEquals("uno", source.getFirst());
+      assertEquals("dos", source.getSecond());
+    } finally {
+      shell.dispose();
+    }
+  }
+
+  @Test
+  void extraGroupBecomesAnotherBox() {
+    Shell shell = new Shell(display);
+    shell.setLayout(new FormLayout());
+    try {
+      BoxesSample source = new BoxesSample();
+      Label[] created = new Label[1];
+      GuiCompositeWidgets widgets = new GuiCompositeWidgets(new Variables());
+      widgets.registerExtraGroup(
+          "Extra", "30", null, parent -> created[0] = new Label(parent, 
SWT.NONE));
+      widgets.createCompositeWidgets(source, null, shell, BOXES_PARENT, null);
+
+      assertNull(findTabFolderDeep(shell));
+      List<Group> groups = findGroups(shell);
+      assertEquals(3, groups.size());
+      assertEquals("Extra", groups.get(2).getText());
+      assertNotNull(created[0]);
+      assertInstanceOf(ScrolledComposite.class, 
created[0].getParent().getParent());
+      assertEquals(groups.get(2), 
created[0].getParent().getParent().getParent());
+    } finally {
+      shell.dispose();
+    }
+  }
+
+  @Test
+  void boxesKeepTheHeightOfTheirFields() {
+    Shell shell = new Shell(display);
+    shell.setLayout(new FormLayout());
+    try {
+      Label header = new Label(shell, SWT.LEFT);
+      header.setText("Header");
+      FormData fdHeader = new FormData();
+      fdHeader.left = new FormAttachment(0, 0);
+      fdHeader.top = new FormAttachment(0, 0);
+      fdHeader.right = new FormAttachment(100, 0);
+      header.setLayoutData(fdHeader);
+
+      Button ok = new Button(shell, SWT.PUSH);
+      ok.setText("OK");
+      FormData fdOk = new FormData();
+      fdOk.right = new FormAttachment(100, 0);
+      fdOk.bottom = new FormAttachment(100, 0);
+      ok.setLayoutData(fdOk);
+
+      GuiCompositeWidgets.addScrolledComposite(
+          shell, new Variables(), header, ok, UNEVEN_PARENT, new 
UnevenBoxesSample());
+      shell.setSize(900, 650);
+      shell.layout(true, true);
+
+      List<Group> groups = findGroups(shell);
+      assertEquals(2, groups.size());
+      Group tall = groups.get(0);
+      Group shortBox = groups.get(1);
+      assertEquals("Tall", tall.getText());
+      assertEquals("Short", shortBox.getText());
+
+      Composite stack = tall.getParent();
+      int width = Math.max(stack.getClientArea().width, 1);
+      int tallPreferred = tall.computeSize(width, SWT.DEFAULT).y;
+      int shortPreferred = shortBox.computeSize(width, SWT.DEFAULT).y;
+      int stackPreferred = stack.computeSize(width, SWT.DEFAULT).y;
+      assertTrue(
+          tallPreferred > shortPreferred, "tall " + tallPreferred + " short " 
+ shortPreferred);
+      assertTrue(
+          stackPreferred < tallPreferred * 2, "stack " + stackPreferred + " 
tall " + tallPreferred);
+      assertTrue(
+          stackPreferred >= tallPreferred + shortPreferred - 4,
+          "stack " + stackPreferred + " tall " + tallPreferred + " short " + 
shortPreferred);
+
+      int gap = tall.getBounds().height - shortBox.getBounds().height;
+      int preferredGap = tallPreferred - shortPreferred;
+      assertTrue(Math.abs(gap - preferredGap) <= 2, "gap " + gap + " 
preferredGap " + preferredGap);
+      assertTrue(tall.getBounds().y + tall.getBounds().height <= 
shortBox.getBounds().y);
+      assertEquals(
+          stack.getClientArea().height, shortBox.getBounds().y + 
shortBox.getBounds().height);
+
+      int contentHeight = stack.getParent().getSize().y;
+      assertTrue(
+          contentHeight < tall.getBounds().height * 2,
+          "content " + contentHeight + " tall " + tall.getBounds().height);
+    } finally {
+      shell.dispose();
+    }
+  }
+
+  @Test
+  void squeezedBoxScrollsInsideItself() {
+    Shell shell = new Shell(display);
+    shell.setLayout(new FormLayout());
+    shell.setSize(500, 400);
+    try {
+      Composite host = new Composite(shell, SWT.NONE);
+      host.setLayout(new FormLayout());
+      FormData fdHost = new FormData();
+      fdHost.left = new FormAttachment(0, 0);
+      fdHost.top = new FormAttachment(0, 0);
+      fdHost.right = new FormAttachment(100, 0);
+      host.setLayoutData(fdHost);
+
+      new GuiCompositeWidgets(new Variables())
+          .createCompositeWidgets(new UnevenBoxesSample(), null, host, 
UNEVEN_PARENT, null);
+
+      Group tall = findGroups(host).get(0);
+      Composite stack = tall.getParent();
+      int preferred = stack.computeSize(480, SWT.DEFAULT).y;
+      assertTrue(preferred > 80, "preferred " + preferred);
+      fdHost.height = Math.max(40, preferred / 2);
+      shell.layout(true, true);
+
+      assertTrue(tall.getBounds().height > 0, "box " + 
tall.getBounds().height);
+      assertTrue(
+          tall.getBounds().height < tall.computeSize(480, SWT.DEFAULT).y,
+          "box " + tall.getBounds().height);
+      ScrolledComposite inner = (ScrolledComposite) tall.getChildren()[0];
+      assertTrue(
+          inner.getClientArea().height < inner.getMinHeight(),
+          "client " + inner.getClientArea().height + " min " + 
inner.getMinHeight());
+    } finally {
+      shell.dispose();
+    }
+  }
+
+  @Test
+  void singleBoxFillsTheSpaceBetweenHeaderAndButtons() {
+    Shell shell = new Shell(display);
+    shell.setLayout(new FormLayout());
+    shell.setSize(500, 400);
+    try {
+      Label header = new Label(shell, SWT.LEFT);
+      header.setText("Header");
+      FormData fdHeader = new FormData();
+      fdHeader.left = new FormAttachment(0, 0);
+      fdHeader.top = new FormAttachment(0, 0);
+      fdHeader.right = new FormAttachment(100, 0);
+      header.setLayoutData(fdHeader);
+
+      Button ok = new Button(shell, SWT.PUSH);
+      ok.setText("OK");
+      FormData fdOk = new FormData();
+      fdOk.right = new FormAttachment(100, 0);
+      fdOk.bottom = new FormAttachment(100, 0);
+      ok.setLayoutData(fdOk);
+
+      SingleBoxSample source = new SingleBoxSample();
+      source.setName("alpha");
+      GuiCompositeWidgets.addScrolledComposite(
+          shell, new Variables(), header, ok, SINGLE_BOX_PARENT, source);
+      shell.layout(true, true);
+
+      List<Group> groups = findGroups(shell);
+      assertEquals(1, groups.size());
+      assertEquals("Only", groups.get(0).getText());
+      assertNull(findTabFolderDeep(shell));
+
+      Rectangle box = absoluteBounds(groups.get(0));
+      Rectangle headerBounds = absoluteBounds(header);
+      Rectangle okBounds = absoluteBounds(ok);
+      assertTrue(box.y >= headerBounds.y + headerBounds.height);
+      assertTrue(box.y + box.height <= okBounds.y);
+      int band = okBounds.y - (headerBounds.y + headerBounds.height);
+      assertTrue(band > 0);
+      assertTrue(box.height * 100 / band >= 60);
+    } finally {
+      shell.dispose();
+    }
+  }
+
+  @Test
+  void hidingAFieldInOneBoxShrinksThatBoxOnly() {
+    Shell shell = new Shell(display);
+    shell.setLayout(new FormLayout());
+    try {
+      BoxesSample source = new BoxesSample();
+      source.setFirst("one");
+      source.setSecond("two");
+      GuiCompositeWidgets widgets = new GuiCompositeWidgets(new Variables());
+      widgets.createCompositeWidgets(source, null, shell, BOXES_PARENT, null);
+      widgets.setWidgetsContents(source, shell, BOXES_PARENT);
+
+      Control first = widgets.getWidgetsMap().get("first");
+      Control second = widgets.getWidgetsMap().get("second");
+      assertNotNull(first);
+      assertNotNull(second);
+      ScrolledComposite firstScroll = (ScrolledComposite) 
first.getParent().getParent();
+      ScrolledComposite secondScroll = (ScrolledComposite) 
second.getParent().getParent();
+      int firstMinBefore = firstScroll.getMinHeight();
+      int secondMinBefore = secondScroll.getMinHeight();
+
+      widgets.setWidgetsHidden(source, Set.of("first"));
+
+      assertFalse(first.getVisible());
+      assertTrue(second.getVisible());
+      assertInstanceOf(FormData.class, second.getLayoutData());
+      FormData secondData = (FormData) second.getLayoutData();
+      assertTrue(secondData.height == -1 || secondData.height > 0);
+      assertTrue(firstScroll.getMinHeight() < firstMinBefore);
+      assertEquals(secondMinBefore, secondScroll.getMinHeight());
+
+      source.setSecond("dos");
+      widgets.setWidgetsContents(source, shell, BOXES_PARENT);
+      widgets.getWidgetsContents(source, BOXES_PARENT);
+      assertEquals("dos", source.getSecond());
+    } finally {
+      shell.dispose();
+    }
+  }
+
   private static void register(Class<?> type) {
     GuiRegistry registry = GuiRegistry.getInstance();
     String parentId = null;
@@ -265,6 +525,44 @@ class GuiCompositeWidgetsGroupTest extends SwtBotTestBase {
     return count;
   }
 
+  private static CTabFolder findTabFolderDeep(Composite parent) {
+    for (Control child : parent.getChildren()) {
+      if (child instanceof CTabFolder folder) {
+        return folder;
+      }
+      if (child instanceof Composite composite) {
+        CTabFolder nested = findTabFolderDeep(composite);
+        if (nested != null) {
+          return nested;
+        }
+      }
+    }
+    return null;
+  }
+
+  private static List<Group> findGroups(Composite parent) {
+    List<Group> groups = new ArrayList<>();
+    collectGroups(parent, groups);
+    return groups;
+  }
+
+  private static void collectGroups(Composite parent, List<Group> groups) {
+    for (Control child : parent.getChildren()) {
+      if (child instanceof Group group) {
+        groups.add(group);
+      }
+      if (child instanceof Composite composite) {
+        collectGroups(composite, groups);
+      }
+    }
+  }
+
+  private static Rectangle absoluteBounds(Control control) {
+    Rectangle bounds = control.getBounds();
+    Point origin = control.getParent().toDisplay(bounds.x, bounds.y);
+    return new Rectangle(origin.x, origin.y, bounds.width, bounds.height);
+  }
+
   @GuiPlugin
   @Getter
   @Setter
@@ -308,4 +606,99 @@ class GuiCompositeWidgetsGroupTest extends SwtBotTestBase {
         groupType = GuiWidgetGroupType.TABS)
     private String second;
   }
+
+  @GuiPlugin
+  @Getter
+  @Setter
+  public static class BoxesSample {
+    @GuiWidgetElement(
+        id = "first",
+        parentId = BOXES_PARENT,
+        type = GuiElementType.TEXT,
+        label = "First",
+        group = "First box",
+        groupOrder = "10",
+        groupType = GuiWidgetGroupType.BOXES)
+    private String first;
+
+    @GuiWidgetElement(
+        id = "second",
+        parentId = BOXES_PARENT,
+        type = GuiElementType.TEXT,
+        label = "Second",
+        group = "Second box",
+        groupOrder = "20",
+        groupType = GuiWidgetGroupType.BOXES)
+    private String second;
+  }
+
+  @GuiPlugin
+  @Getter
+  @Setter
+  public static class SingleBoxSample {
+    @GuiWidgetElement(
+        id = "name",
+        parentId = SINGLE_BOX_PARENT,
+        type = GuiElementType.TEXT,
+        label = "Name",
+        group = "Only",
+        groupType = GuiWidgetGroupType.BOXES)
+    private String name;
+  }
+
+  /** Four fields in one box and one field in the other. */
+  @GuiPlugin
+  @Getter
+  @Setter
+  public static class UnevenBoxesSample {
+    @GuiWidgetElement(
+        id = "a",
+        parentId = UNEVEN_PARENT,
+        type = GuiElementType.TEXT,
+        label = "A",
+        group = "Tall",
+        groupOrder = "10",
+        groupType = GuiWidgetGroupType.BOXES)
+    private String a;
+
+    @GuiWidgetElement(
+        id = "b",
+        parentId = UNEVEN_PARENT,
+        type = GuiElementType.TEXT,
+        label = "B",
+        group = "Tall",
+        groupOrder = "10",
+        groupType = GuiWidgetGroupType.BOXES)
+    private String b;
+
+    @GuiWidgetElement(
+        id = "c",
+        parentId = UNEVEN_PARENT,
+        type = GuiElementType.TEXT,
+        label = "C",
+        group = "Tall",
+        groupOrder = "10",
+        groupType = GuiWidgetGroupType.BOXES)
+    private String c;
+
+    @GuiWidgetElement(
+        id = "d",
+        parentId = UNEVEN_PARENT,
+        type = GuiElementType.TEXT,
+        label = "D",
+        group = "Tall",
+        groupOrder = "10",
+        groupType = GuiWidgetGroupType.BOXES)
+    private String d;
+
+    @GuiWidgetElement(
+        id = "only",
+        parentId = UNEVEN_PARENT,
+        type = GuiElementType.TEXT,
+        label = "Only",
+        group = "Short",
+        groupOrder = "20",
+        groupType = GuiWidgetGroupType.BOXES)
+    private String only;
+  }
 }
diff --git 
a/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java 
b/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java
index f52d1e0231..dbf11f6390 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java
@@ -65,11 +65,14 @@ import org.eclipse.swt.custom.CTabFolder;
 import org.eclipse.swt.custom.CTabItem;
 import org.eclipse.swt.custom.ScrolledComposite;
 import org.eclipse.swt.graphics.Image;
+import org.eclipse.swt.graphics.Point;
 import org.eclipse.swt.graphics.Rectangle;
 import org.eclipse.swt.layout.FillLayout;
 import org.eclipse.swt.layout.FormAttachment;
 import org.eclipse.swt.layout.FormData;
 import org.eclipse.swt.layout.FormLayout;
+import org.eclipse.swt.layout.GridData;
+import org.eclipse.swt.layout.GridLayout;
 import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Combo;
 import org.eclipse.swt.widgets.Composite;
@@ -273,35 +276,43 @@ public class GuiCompositeWidgets {
 
   private void layoutBoxes(
       Object sourceData, Composite parent, List<WidgetGroup> groups, boolean 
useNewLayout) {
-    Control last = widgetsFirstLastControl;
-    int margin = PropsUi.getMargin();
+    // A control passed in above the groups stays outside this filler. Putting 
the boxes on the
+    // parent itself would start at the top and cover that control. The 
column's preferred height
+    // is the boxes put together. Extra space in the parent is shared, and a 
box that is squeezed
+    // scrolls its own fields.
+    Composite filler = new Composite(parent, SWT.NONE);
+    PropsUi.setLook(filler);
+    GridLayout grid = new GridLayout(1, false);
+    grid.marginWidth = 0;
+    grid.marginHeight = 0;
+    grid.verticalSpacing = PropsUi.getMargin();
+    filler.setLayout(grid);
+    FormData fdFiller = new FormData();
+    fdFiller.left = new FormAttachment(0, 0);
+    fdFiller.right = new FormAttachment(100, 0);
+    fdFiller.top =
+        widgetsFirstLastControl == null
+            ? new FormAttachment(0, 0)
+            : new FormAttachment(widgetsFirstLastControl, PropsUi.getMargin());
+    fdFiller.bottom = new FormAttachment(100, 0);
+    filler.setLayoutData(fdFiller);
+
     for (WidgetGroup group : groups) {
-      Group box = new Group(parent, SWT.SHADOW_ETCHED_IN);
+      Group box = new Group(filler, SWT.SHADOW_ETCHED_IN);
       PropsUi.setLook(box);
       box.setText(Const.NVL(group.label, ""));
-      FormLayout boxLayout = new FormLayout();
-      boxLayout.marginWidth = PropsUi.getFormMargin();
-      boxLayout.marginHeight = PropsUi.getFormMargin();
-      box.setLayout(boxLayout);
-
-      FormData fdBox = new FormData();
-      fdBox.left = new FormAttachment(0, 0);
-      fdBox.right = new FormAttachment(100, 0);
-      if (last == null) {
-        fdBox.top = new FormAttachment(0, 0);
-      } else {
-        fdBox.top = new FormAttachment(last, margin);
-      }
-      box.setLayoutData(fdBox);
+      box.setLayout(new FillLayout());
+      box.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));
 
+      Composite content = createScrolledContent(box);
       Control lastInBox = null;
       for (GuiElements child : group.elements) {
-        lastInBox = addCompositeWidgets(sourceData, box, child, lastInBox, 
useNewLayout);
+        lastInBox = addCompositeWidgets(sourceData, content, child, lastInBox, 
useNewLayout);
       }
       for (Consumer<Composite> extra : group.extras) {
-        extra.accept(box);
+        extra.accept(content);
       }
-      last = box;
+      updateScrolledMinSize(content);
     }
   }
 
@@ -331,18 +342,8 @@ public class GuiCompositeWidgets {
         item.setImage(image);
       }
 
-      ScrolledComposite scrolled = new ScrolledComposite(folder, SWT.V_SCROLL 
| SWT.H_SCROLL);
-      scrolled.setLayout(new FillLayout());
-      Composite composite = new Composite(scrolled, SWT.NONE);
-      PropsUi.setLook(composite);
-      FormLayout layout = new FormLayout();
-      layout.marginWidth = PropsUi.getFormMargin();
-      layout.marginHeight = PropsUi.getFormMargin();
-      composite.setLayout(layout);
-      scrolled.setContent(composite);
-      scrolled.setExpandHorizontal(true);
-      scrolled.setExpandVertical(true);
-      item.setControl(scrolled);
+      Composite composite = createScrolledContent(folder);
+      item.setControl(composite.getParent());
 
       Control last = null;
       for (GuiElements child : group.elements) {
@@ -351,10 +352,7 @@ public class GuiCompositeWidgets {
       for (Consumer<Composite> extra : group.extras) {
         extra.accept(composite);
       }
-      composite.pack();
-      Rectangle bounds = composite.getBounds();
-      scrolled.setMinWidth(bounds.width);
-      scrolled.setMinHeight(bounds.height);
+      updateScrolledMinSize(composite);
     }
 
     if (folder.getItemCount() > 0) {
@@ -362,6 +360,45 @@ public class GuiCompositeWidgets {
     }
   }
 
+  /**
+   * Scrollable form inside a tab or a box. The returned composite is the 
parent for fields and
+   * extra-group contents; its parent is the {@link ScrolledComposite}.
+   */
+  private Composite createScrolledContent(Composite host) {
+    ScrolledComposite scrolled = new ScrolledComposite(host, SWT.V_SCROLL | 
SWT.H_SCROLL);
+    scrolled.setLayout(new FillLayout());
+    Composite composite = new Composite(scrolled, SWT.NONE);
+    PropsUi.setLook(composite);
+    FormLayout layout = new FormLayout();
+    layout.marginWidth = PropsUi.getFormMargin();
+    layout.marginHeight = PropsUi.getFormMargin();
+    composite.setLayout(layout);
+    scrolled.setContent(composite);
+    scrolled.setExpandHorizontal(true);
+    scrolled.setExpandVertical(true);
+    return composite;
+  }
+
+  /**
+   * Point the scroll range at the content's preferred size. The previous 
minimum is cleared first:
+   * with expand on, the scrolled composite holds the content at the old 
minimum, so hiding a row
+   * would not shrink the range.
+   */
+  private void updateScrolledMinSize(Composite content) {
+    if (content == null
+        || content.isDisposed()
+        || !(content.getParent() instanceof ScrolledComposite scrolled)
+        || scrolled.isDisposed()) {
+      return;
+    }
+    scrolled.setMinWidth(0);
+    scrolled.setMinHeight(0);
+    content.layout(true, true);
+    Point preferred = content.computeSize(SWT.DEFAULT, SWT.DEFAULT, true);
+    scrolled.setMinWidth(preferred.x);
+    scrolled.setMinHeight(preferred.y);
+  }
+
   private Image loadGroupImage(Composite parent, String filename) {
     if (StringUtils.isEmpty(filename)) {
       return null;
@@ -503,6 +540,7 @@ public class GuiCompositeWidgets {
       }
       if (parent != null && !parent.isDisposed()) {
         parent.layout(true, true);
+        updateScrolledMinSize(parent);
       }
     }
 

Reply via email to