This is an automated email from the ASF dual-hosted git repository.
hansva 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 f6d9225fa5 Issue #8139 - Formula transform editor layout collapses
when reopening formula dialog (#8146)
f6d9225fa5 is described below
commit f6d9225fa58940fd8223555a7fb9b16c3a901533
Author: Sergio Ramazzina <[email protected]>
AuthorDate: Mon Aug 31 09:26:27 2026 +0200
Issue #8139 - Formula transform editor layout collapses when reopening
formula dialog (#8146)
* Issue apache#8139 Formula transform editor layout collapses when
reopening formula dialog
- The editor shell was created without any size or title, so the platform
sized every new instance from its cascaded origin to the screen edge and
each reopening came up smaller: give the shell a title and an explicit
default size, and let BaseTransformDialog.setSize() restore the geometry
saved on close (no minimum, so a smaller size the user picked is kept).
- ok()/cancel() dropped the shell without recording its geometry, so a
manual
resize was lost at every close: store the WindowProperty before disposing.
- rightSash weights {10, 80} left the expression editor 11% of the height,
no longer enough since the text composite gained a toolbar: use {40, 60}.
- SWTBot test covering that a reopening restores the size the user chose.
* Issue apache#8139 make the formula editor UI test survive a CI runner
without WebKitGTK
- The editor embeds an SWT Browser, which on Linux needs WebKitGTK. The CI
runner installs only xvfb and libgtk-3-0, so the widget throws while the
dialog is being built, the dialog never opens and SWTBot times out waiting
for a shell that will never appear: probe the Browser widget up front and
skip the test where it is unavailable.
- Building the editor (function library plus Browser start-up) takes longer
than SWTBot's 5 s default on a cold machine: raise the timeout for this
test.
---
.../transforms/formula/editor/FormulaEditor.java | 27 +++-
.../formula/messages/messages_en_US.properties | 1 +
.../formula/editor/FormulaEditorDialogTest.java | 138 +++++++++++++++++++++
.../transforms/janino/editor/FormulaEditor.java | 27 +++-
.../janino/messages/messages_en_US.properties | 1 +
5 files changed, 188 insertions(+), 6 deletions(-)
diff --git
a/plugins/transforms/formula/src/main/java/org/apache/hop/pipeline/transforms/formula/editor/FormulaEditor.java
b/plugins/transforms/formula/src/main/java/org/apache/hop/pipeline/transforms/formula/editor/FormulaEditor.java
index 3b4897af17..f50da18366 100644
---
a/plugins/transforms/formula/src/main/java/org/apache/hop/pipeline/transforms/formula/editor/FormulaEditor.java
+++
b/plugins/transforms/formula/src/main/java/org/apache/hop/pipeline/transforms/formula/editor/FormulaEditor.java
@@ -23,8 +23,11 @@ import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.pipeline.transforms.formula.FormulaMeta;
import org.apache.hop.pipeline.transforms.formula.function.FunctionDescription;
import org.apache.hop.pipeline.transforms.formula.function.FunctionLib;
+import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.core.gui.WindowProperty;
import org.apache.hop.ui.core.widget.StyledTextComp;
import org.apache.hop.ui.core.widget.TextComposite;
+import org.apache.hop.ui.pipeline.transform.BaseTransformDialog;
import org.eclipse.swt.SWT;
import org.eclipse.swt.browser.Browser;
import org.eclipse.swt.custom.SashForm;
@@ -49,6 +52,9 @@ public class FormulaEditor extends Dialog implements
KeyListener {
public static final Class<?> PKG = FormulaMeta.class;
public static final String FUNCTIONS_FILE = "functions.xml";
+ private static final int DEFAULT_WIDTH = 900;
+ private static final int DEFAULT_HEIGHT = 700;
+
private Shell shell;
private Tree tree;
private SashForm sashForm;
@@ -90,6 +96,7 @@ public class FormulaEditor extends Dialog implements
KeyListener {
formLayout.marginWidth = 5;
formLayout.marginHeight = 5;
shell.setLayout(formLayout);
+ shell.setText(BaseMessages.getString(PKG, "FormulaEditor.Shell.Title"));
// At the bottom we have a few buttons...
//
@@ -241,7 +248,7 @@ public class FormulaEditor extends Dialog implements
KeyListener {
fdMessage.bottom = new FormAttachment(0, 100);
message.setLayoutData(fdMessage);
- rightSash.setWeights(new int[] {10, 80});
+ rightSash.setWeights(new int[] {40, 60});
sashForm.setWeights(new int[] {15, 85});
@@ -266,7 +273,11 @@ public class FormulaEditor extends Dialog implements
KeyListener {
}
public String open() {
- shell.layout();
+ // The default size only applies the first time: setSize() restores the
geometry saved when the
+ // dialog was last closed, and no minimum is imposed on it so a smaller
size the user picked is
+ // honoured as well.
+ shell.setSize(DEFAULT_WIDTH, DEFAULT_HEIGHT);
+ BaseTransformDialog.setSize(shell, -1, -1);
shell.open();
while (!shell.isDisposed()) {
@@ -279,11 +290,21 @@ public class FormulaEditor extends Dialog implements
KeyListener {
public void ok() {
formula = expressionEditor.getText();
- shell.dispose();
+ dispose();
}
public void cancel() {
formula = null;
+ dispose();
+ }
+
+ /** Remember the geometry chosen by the user so the next opening restores
it. */
+ private void dispose() {
+ WindowProperty winprop = new WindowProperty(shell);
+ PropsUi props = PropsUi.getInstance();
+ props.setSessionScreen(winprop);
+ props.setScreen(winprop);
+
shell.dispose();
}
diff --git
a/plugins/transforms/formula/src/main/resources/org/apache/hop/pipeline/transforms/formula/messages/messages_en_US.properties
b/plugins/transforms/formula/src/main/resources/org/apache/hop/pipeline/transforms/formula/messages/messages_en_US.properties
index 85d47375b9..df3259336c 100644
---
a/plugins/transforms/formula/src/main/resources/org/apache/hop/pipeline/transforms/formula/messages/messages_en_US.properties
+++
b/plugins/transforms/formula/src/main/resources/org/apache/hop/pipeline/transforms/formula/messages/messages_en_US.properties
@@ -46,6 +46,7 @@ FormulaDialog.SelectCalculationType.Title = Select the
calculation type
FormulaDialog.Shell.Title=Formula
FormulaDialog.TransformName.Label=Formula name
FormulaDialog.ValueType.Column = Value type
+FormulaEditor.Shell.Title=Formula Expression Editor
FormulaMeta.CheckResult.ExpectedInputError = No input received from other
transforms\!
FormulaMeta.CheckResult.ExpectedInputOk = Transform is receiving info from
other steps.
FormulaMeta.CheckResult.FieldsReceived = Transform is connected to previous
one, receiving {0} fields
diff --git
a/plugins/transforms/formula/src/test/java/org/apache/hop/pipeline/transforms/formula/editor/FormulaEditorDialogTest.java
b/plugins/transforms/formula/src/test/java/org/apache/hop/pipeline/transforms/formula/editor/FormulaEditorDialogTest.java
new file mode 100644
index 0000000000..277c4a49e5
--- /dev/null
+++
b/plugins/transforms/formula/src/test/java/org/apache/hop/pipeline/transforms/formula/editor/FormulaEditorDialogTest.java
@@ -0,0 +1,138 @@
+/*
+ * 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.
+ */
+
+package org.apache.hop.pipeline.transforms.formula.editor;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.hop.core.exception.HopException;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.i18n.BaseMessages;
+import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.SWTError;
+import org.eclipse.swt.SWTException;
+import org.eclipse.swt.browser.Browser;
+import org.eclipse.swt.graphics.Point;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swtbot.swt.finder.SWTBot;
+import org.eclipse.swtbot.swt.finder.finders.UIThreadRunnable;
+import org.eclipse.swtbot.swt.finder.utils.SWTBotPreferences;
+import org.eclipse.swtbot.swt.finder.widgets.SWTBotShell;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Covers the geometry handling of the formula expression editor (issue
#8139): the shell used to be
+ * created without any size, so Windows sized every new instance from its
cascaded origin to the
+ * screen edge and each reopening was smaller than the previous one.
+ *
+ * <p>Tagged {@code uitest} so it is skipped when there is no display.
+ */
+@Tag("uitest")
+class FormulaEditorDialogTest extends SwtBotTestBase {
+
+ private static final String TITLE =
+ BaseMessages.getString(FormulaEditor.PKG, "FormulaEditor.Shell.Title");
+ private static final Point CHOSEN_SIZE = new Point(720, 540);
+
+ /**
+ * The editor reads its function library and starts a {@link Browser} before
it opens, which takes
+ * well over SWTBot's 5 s default on a cold CI machine.
+ */
+ private static final long DIALOG_TIMEOUT_MS = 30_000L;
+
+ private static long defaultTimeout;
+
+ @BeforeAll
+ static void slowDownSwtBot() {
+ defaultTimeout = SWTBotPreferences.TIMEOUT;
+ SWTBotPreferences.TIMEOUT = DIALOG_TIMEOUT_MS;
+ }
+
+ @AfterAll
+ static void restoreSwtBotTimeout() {
+ SWTBotPreferences.TIMEOUT = defaultTimeout;
+ }
+
+ @Test
+ void reopeningRestoresTheSizeTheUserChose() {
+ assumeTrue(
+ browserWidgetAvailable(),
+ "the SWT Browser widget is unavailable here (no WebKitGTK), so the
editor cannot be built");
+
+ openEditor(
+ bot -> {
+ SWTBotShell shell = bot.shell(TITLE).activate();
+ UIThreadRunnable.syncExec(() -> shell.widget.setSize(CHOSEN_SIZE.x,
CHOSEN_SIZE.y));
+ shell.bot().button(buttonLabel("System.Button.Cancel")).click();
+ });
+
+ AtomicReference<Point> sizeOnReopen = new AtomicReference<>();
+ openEditor(
+ bot -> {
+ SWTBotShell shell = bot.shell(TITLE).activate();
+ sizeOnReopen.set(UIThreadRunnable.syncExec(() ->
shell.widget.getSize()));
+ shell.bot().button(buttonLabel("System.Button.Cancel")).click();
+ });
+
+ assertEquals(
+ CHOSEN_SIZE,
+ sizeOnReopen.get(),
+ "the second opening must restore the geometry saved when the first one
was closed");
+ }
+
+ /**
+ * The formula editor embeds a {@link Browser}, which on Linux needs
WebKitGTK. Where that library
+ * is missing the widget throws while the dialog is being built and the
dialog never opens, so the
+ * test is skipped instead of failing on a shell that will never appear.
+ */
+ private static boolean browserWidgetAvailable() {
+ ensureDisplay();
+ Shell probe = new Shell(display, SWT.NONE);
+ try {
+ new Browser(probe, SWT.NONE);
+ return true;
+ } catch (SWTError | SWTException e) {
+ return false;
+ } finally {
+ probe.dispose();
+ }
+ }
+
+ private void openEditor(java.util.function.Consumer<SWTBot> interactions) {
+ withDialog(
+ parent -> {
+ try {
+ new FormulaEditor(
+ new Variables(),
+ parent,
+ SWT.APPLICATION_MODAL | SWT.SHEET,
+ "[field]",
+ new String[] {"field"})
+ .open();
+ } catch (HopException e) {
+ throw new IllegalStateException("could not open the formula
editor", e);
+ }
+ },
+ interactions);
+ }
+}
diff --git
a/plugins/transforms/janino/src/main/java/org/apache/hop/pipeline/transforms/janino/editor/FormulaEditor.java
b/plugins/transforms/janino/src/main/java/org/apache/hop/pipeline/transforms/janino/editor/FormulaEditor.java
index 38b1a99d3a..7aed459e2d 100644
---
a/plugins/transforms/janino/src/main/java/org/apache/hop/pipeline/transforms/janino/editor/FormulaEditor.java
+++
b/plugins/transforms/janino/src/main/java/org/apache/hop/pipeline/transforms/janino/editor/FormulaEditor.java
@@ -24,8 +24,11 @@ import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.pipeline.transforms.janino.JaninoMeta;
import org.apache.hop.pipeline.transforms.janino.function.FunctionDescription;
import org.apache.hop.pipeline.transforms.janino.function.FunctionLib;
+import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.core.gui.WindowProperty;
import org.apache.hop.ui.core.widget.StyledTextComp;
import org.apache.hop.ui.core.widget.TextComposite;
+import org.apache.hop.ui.pipeline.transform.BaseTransformDialog;
import org.eclipse.swt.SWT;
import org.eclipse.swt.browser.Browser;
import org.eclipse.swt.custom.SashForm;
@@ -51,6 +54,9 @@ import org.eclipse.swt.widgets.TreeItem;
public class FormulaEditor extends Dialog implements KeyListener {
public static final Class<?> PKG = JaninoMeta.class;
+ private static final int DEFAULT_WIDTH = 900;
+ private static final int DEFAULT_HEIGHT = 700;
+
private Shell shell;
private Tree tree;
private SashForm sashForm;
@@ -91,6 +97,7 @@ public class FormulaEditor extends Dialog implements
KeyListener {
formLayout.marginWidth = 5;
formLayout.marginHeight = 5;
shell.setLayout(formLayout);
+ shell.setText(BaseMessages.getString(PKG, "FormulaEditor.Shell.Title"));
// At the bottom we have a few buttons...
//
@@ -242,7 +249,7 @@ public class FormulaEditor extends Dialog implements
KeyListener {
fdMessage.bottom = new FormAttachment(0, 100);
message.setLayoutData(fdMessage);
- rightSash.setWeights(new int[] {10, 80});
+ rightSash.setWeights(new int[] {40, 60});
sashForm.setWeights(new int[] {15, 85});
red = new Color(shell.getDisplay(), 255, 0, 0);
@@ -266,7 +273,11 @@ public class FormulaEditor extends Dialog implements
KeyListener {
}
public String open() {
- shell.layout();
+ // The default size only applies the first time: setSize() restores the
geometry saved when the
+ // dialog was last closed, and no minimum is imposed on it so a smaller
size the user picked is
+ // honoured as well.
+ shell.setSize(DEFAULT_WIDTH, DEFAULT_HEIGHT);
+ BaseTransformDialog.setSize(shell, -1, -1);
shell.open();
// Detect X or ALT-F4 or something that kills this window...
@@ -288,11 +299,21 @@ public class FormulaEditor extends Dialog implements
KeyListener {
public void ok() {
formula = expressionEditor.getText();
- shell.dispose();
+ dispose();
}
public void cancel() {
formula = null;
+ dispose();
+ }
+
+ /** Remember the geometry chosen by the user so the next opening restores
it. */
+ private void dispose() {
+ WindowProperty winprop = new WindowProperty(shell);
+ PropsUi props = PropsUi.getInstance();
+ props.setSessionScreen(winprop);
+ props.setScreen(winprop);
+
shell.dispose();
}
diff --git
a/plugins/transforms/janino/src/main/resources/org/apache/hop/pipeline/transforms/janino/messages/messages_en_US.properties
b/plugins/transforms/janino/src/main/resources/org/apache/hop/pipeline/transforms/janino/messages/messages_en_US.properties
index 2358f3acca..6ad5a1ec09 100644
---
a/plugins/transforms/janino/src/main/resources/org/apache/hop/pipeline/transforms/janino/messages/messages_en_US.properties
+++
b/plugins/transforms/janino/src/main/resources/org/apache/hop/pipeline/transforms/janino/messages/messages_en_US.properties
@@ -17,6 +17,7 @@
#
#
+FormulaEditor.Shell.Title=Java Expression Editor
Janino.Description=Calculate the result of a Java Expression using Janino
Janino.Error.ValueTypeMismatch=A {0} type was specified for field [{1}], but a
[{2}] type was returned as a result of formula [{3}]
Janino.Injection.FIELD_FORMULA=Java expression
