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 8460b49c89 force recalculation of shell to place widgets correctly,
fixes #7689 (#7690)
8460b49c89 is described below
commit 8460b49c89780e17b2723210612a28e493b035a2
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Thu Jul 30 14:52:58 2026 +0200
force recalculation of shell to place widgets correctly, fixes #7689 (#7690)
---
.../hop/ui/core/gui/WindowPropertyLayoutTest.java | 136 +++++++++++++
.../vfs/HopVfsFileDialogToolbarLayoutTest.java | 218 +++++++++++++++++++++
.../org/apache/hop/ui/core/gui/WindowProperty.java | 43 ++++
3 files changed, 397 insertions(+)
diff --git
a/rcp/src/test/java/org/apache/hop/ui/core/gui/WindowPropertyLayoutTest.java
b/rcp/src/test/java/org/apache/hop/ui/core/gui/WindowPropertyLayoutTest.java
new file mode 100644
index 0000000000..04932d4f3b
--- /dev/null
+++ b/rcp/src/test/java/org/apache/hop/ui/core/gui/WindowPropertyLayoutTest.java
@@ -0,0 +1,136 @@
+/*
+ * 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.ui.core.gui;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.custom.SashForm;
+import org.eclipse.swt.layout.FormAttachment;
+import org.eclipse.swt.layout.FormData;
+import org.eclipse.swt.layout.FormLayout;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swt.widgets.ToolBar;
+import org.eclipse.swt.widgets.ToolItem;
+import org.eclipse.swt.widgets.Tree;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Restoring a remembered geometry with {@link WindowProperty#setShell(Shell)}
has to leave the
+ * widgets where their layout puts them.
+ *
+ * <p>This is the shape every Hop perspective with a toolbar over a tree uses
- metadata, git and
+ * execution information all build a {@link SashForm} pane holding a toolbar
above a tree, and they
+ * build it before the main shell is opened. {@code setShell} sets the shell's
*outer* bounds, so
+ * the window trim is only subtracted from the client area once the shell is
on screen; without a
+ * re-layout at that point every child of the pane stays shifted up by the
trim height and the
+ * toolbar is clipped against the top of the panel until the user resizes the
window.
+ */
+@Tag("uitest")
+class WindowPropertyLayoutTest extends SwtBotTestBase {
+
+ private static final String SHELL_NAME = "WindowPropertyLayoutTest";
+ private static final int REMEMBERED_WIDTH = 900;
+ private static final int REMEMBERED_HEIGHT = 600;
+
+ @Test
+ void toolbarInASashPaneIsLaidOutAfterRestoringTheGeometry() {
+ AtomicReference<ToolBar> toolBar = new AtomicReference<>();
+ AtomicReference<Tree> tree = new AtomicReference<>();
+
+ withScene(
+ shell -> {
+ shell.setText(SHELL_NAME);
+ buildPerspectiveLikeScene(shell, toolBar, tree);
+ // What HopGui and every remembering dialog do just before open().
+ new WindowProperty(SHELL_NAME, false, 100, 100, REMEMBERED_WIDTH,
REMEMBERED_HEIGHT)
+ .setShell(shell);
+ },
+ bot -> {
+ // Deliberately no resize and no explicit layout: this is what the
user sees on open.
+ bot.sleep(400);
+
+ org.eclipse.swt.graphics.Rectangle toolBarBounds =
syncBounds(toolBar.get());
+ org.eclipse.swt.graphics.Rectangle treeBounds =
syncBounds(tree.get());
+
+ assertEquals(
+ 0,
+ toolBarBounds.y,
+ "The toolbar should sit at the top of its panel, not above it: "
+ toolBarBounds);
+ assertTrue(
+ treeBounds.y >= toolBarBounds.y + toolBarBounds.height,
+ "The tree should start below the toolbar, but tree="
+ + treeBounds
+ + " toolbar="
+ + toolBarBounds);
+ });
+ }
+
+ /** Toolbar over a tree inside a SashForm pane: the layout the perspectives
use. */
+ private void buildPerspectiveLikeScene(
+ Shell shell, AtomicReference<ToolBar> toolBarRef, AtomicReference<Tree>
treeRef) {
+ shell.setLayout(new FormLayout());
+
+ SashForm sash = new SashForm(shell, SWT.HORIZONTAL);
+ FormData fdSash = new FormData();
+ fdSash.left = new FormAttachment(0, 0);
+ fdSash.top = new FormAttachment(0, 0);
+ fdSash.right = new FormAttachment(100, 0);
+ fdSash.bottom = new FormAttachment(100, 0);
+ sash.setLayoutData(fdSash);
+
+ Composite pane = new Composite(sash, SWT.BORDER);
+ pane.setLayout(new FormLayout());
+
+ ToolBar toolBar = new ToolBar(pane, SWT.WRAP | SWT.LEFT | SWT.HORIZONTAL);
+ for (int i = 0; i < 5; i++) {
+ new ToolItem(toolBar, SWT.PUSH).setText("i" + i);
+ }
+ FormData fdToolBar = new FormData();
+ fdToolBar.left = new FormAttachment(0, 0);
+ fdToolBar.top = new FormAttachment(0, 0);
+ fdToolBar.right = new FormAttachment(100, 0);
+ toolBar.setLayoutData(fdToolBar);
+ toolBar.pack();
+ toolBarRef.set(toolBar);
+
+ Tree tree = new Tree(pane, SWT.SINGLE | SWT.H_SCROLL | SWT.V_SCROLL);
+ FormData fdTree = new FormData();
+ fdTree.left = new FormAttachment(0, 0);
+ fdTree.top = new FormAttachment(toolBar, 0);
+ fdTree.right = new FormAttachment(100, 0);
+ fdTree.bottom = new FormAttachment(100, 0);
+ tree.setLayoutData(fdTree);
+ treeRef.set(tree);
+
+ // The editor side of the perspective.
+ new Composite(sash, SWT.NONE).setLayout(new FormLayout());
+ sash.setWeights(25, 75);
+ }
+
+ private org.eclipse.swt.graphics.Rectangle
syncBounds(org.eclipse.swt.widgets.Control control) {
+ AtomicReference<org.eclipse.swt.graphics.Rectangle> bounds = new
AtomicReference<>();
+ control.getDisplay().syncExec(() -> bounds.set(control.getBounds()));
+ return bounds.get();
+ }
+}
diff --git
a/rcp/src/test/java/org/apache/hop/ui/core/vfs/HopVfsFileDialogToolbarLayoutTest.java
b/rcp/src/test/java/org/apache/hop/ui/core/vfs/HopVfsFileDialogToolbarLayoutTest.java
new file mode 100644
index 0000000000..6e534c83d0
--- /dev/null
+++
b/rcp/src/test/java/org/apache/hop/ui/core/vfs/HopVfsFileDialogToolbarLayoutTest.java
@@ -0,0 +1,218 @@
+/*
+ * 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.ui.core.vfs;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.core.vfs.HopVfs;
+import org.apache.hop.i18n.BaseMessages;
+import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.core.dialog.BaseDialog;
+import org.apache.hop.ui.core.gui.WindowProperty;
+import org.apache.hop.ui.hopgui.HopGuiEnvironment;
+import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.eclipse.swt.graphics.Rectangle;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swt.widgets.ToolBar;
+import org.eclipse.swtbot.swt.finder.widgets.SWTBotShell;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * The toolbars of the Open file dialog have to sit where their layout puts
them the moment the
+ * dialog appears, without the user resizing the window first.
+ *
+ * <p>Restoring a remembered window geometry used to break that: the widgets
were laid out while the
+ * shell still had its pre-open size, so once the window trim (title bar) was
subtracted from the
+ * client area, everything inside a {@code SashForm} pane ended up shifted off
the top of the panel
+ * by the trim height - a toolbar clipped to a sliver above the file browser.
+ */
+@Tag("uitest")
+class HopVfsFileDialogToolbarLayoutTest extends SwtBotTestBase {
+
+ private static final int REMEMBERED_WIDTH = 1000;
+ private static final int REMEMBERED_HEIGHT = 600;
+
+ @TempDir private Path folder;
+
+ @BeforeAll
+ static void registerGuiPlugins() throws Exception {
+ // The toolbars are built from @GuiToolbarElement annotations, which only
reach the GuiRegistry
+ // once the GUI plugin types have been scanned.
+ HopGuiEnvironment.init();
+ }
+
+ @Test
+ void toolbarsAreLaidOutWhenTheDialogRemembersItsGeometry() throws Exception {
+ // Mimic a dialog that has been opened before:
BaseTransformDialog.setSize() then restores the
+ // geometry through WindowProperty instead of packing the shell.
+ PropsUi.getInstance()
+ .setSessionScreen(
+ new WindowProperty(
+ openFileTitle(), false, 100, 100, REMEMBERED_WIDTH,
REMEMBERED_HEIGHT));
+
+ List<String> problems = openDialogAndCheckToolbars();
+
+ assertTrue(problems.isEmpty(), "Toolbars are misplaced on open: " +
problems);
+ }
+
+ @Test
+ void toolbarsAreLaidOutWhenTheDialogHasNoRememberedGeometry() throws
Exception {
+ List<String> problems = openDialogAndCheckToolbars();
+
+ assertTrue(problems.isEmpty(), "Toolbars are misplaced on open: " +
problems);
+ }
+
+ /**
+ * Opens the real dialog and, without touching the window, reports every
toolbar that is not fully
+ * inside its parent's client area.
+ */
+ private List<String> openDialogAndCheckToolbars() throws IOException {
+ createPipelineFile("one.hpl");
+
+ AtomicReference<List<String>> problems = new AtomicReference<>();
+ withDialog(
+ parent -> presentDialog(parent),
+ bot -> {
+ SWTBotShell shell = bot.shell(openFileTitle());
+ Shell swtShell = shell.widget;
+ problems.set(collectMisplacedToolbars(swtShell));
+ shell.bot().button(buttonLabel("System.Button.Cancel")).click();
+ });
+ return problems.get();
+ }
+
+ private List<String> collectMisplacedToolbars(Shell shell) {
+ AtomicReference<List<String>> result = new AtomicReference<>();
+ shell.getDisplay().syncExec(() -> result.set(checkToolbars(shell)));
+ return result.get();
+ }
+
+ private List<String> checkToolbars(Shell shell) {
+ List<String> problems = new ArrayList<>();
+ List<ToolBar> toolBars = new ArrayList<>();
+ collectToolBars(shell, toolBars);
+ assertFalse(toolBars.isEmpty(), "The dialog should have toolbars to
check");
+
+ for (ToolBar toolBar : toolBars) {
+ Rectangle bounds = toolBar.getBounds();
+ Rectangle parentArea = toolBar.getParent().getClientArea();
+ if (bounds.y < 0 || bounds.y + bounds.height > parentArea.height) {
+ problems.add(
+ String.format(
+ "toolbar with %d item(s) at %s does not fit its parent's
client area %s",
+ toolBar.getItemCount(), bounds, parentArea));
+ }
+ // A toolbar sitting in the right place is useless if its items are
clipped out of it.
+ for (var item : toolBar.getItems()) {
+ Rectangle itemBounds = item.getBounds();
+ if (itemBounds.y < 0 || itemBounds.y + itemBounds.height >
bounds.height) {
+ problems.add(
+ String.format("item at %s is clipped by its toolbar %s",
itemBounds, bounds));
+ }
+ }
+ }
+ return problems;
+ }
+
+ /**
+ * Every toolbar row also has to keep its distance from the widget below it,
which is what the
+ * clipped-toolbar screenshot really shows.
+ */
+ @Test
+ void theFileBrowserStartsBelowItsToolbar() throws Exception {
+ PropsUi.getInstance()
+ .setSessionScreen(
+ new WindowProperty(
+ openFileTitle(), false, 100, 100, REMEMBERED_WIDTH,
REMEMBERED_HEIGHT));
+ createPipelineFile("one.hpl");
+
+ AtomicReference<Integer> toolbarTop = new AtomicReference<>();
+ withDialog(
+ parent -> presentDialog(parent),
+ bot -> {
+ SWTBotShell shell = bot.shell(openFileTitle());
+ Shell swtShell = shell.widget;
+ swtShell
+ .getDisplay()
+ .syncExec(
+ () -> {
+ List<ToolBar> toolBars = new ArrayList<>();
+ collectToolBars(swtShell, toolBars);
+ // The browser toolbar is the one sharing its parent with
the search field.
+ int top =
+ toolBars.stream()
+ .mapToInt(toolBar -> toolBar.getBounds().y)
+ .min()
+ .orElse(Integer.MIN_VALUE);
+ toolbarTop.set(top);
+ });
+ shell.bot().button(buttonLabel("System.Button.Cancel")).click();
+ });
+
+ assertEquals(0, toolbarTop.get(), "No toolbar should be pushed above the
top of its panel");
+ }
+
+ private void presentDialog(Shell parent) {
+ try {
+ BaseDialog.presentFileDialog(
+ false,
+ parent,
+ null,
+ new Variables(),
+ HopVfs.getFileObject(folder.toString()),
+ new String[] {"*.hpl"},
+ new String[] {"Pipelines"},
+ false);
+ } catch (Exception e) {
+ throw new IllegalStateException("Error opening the file dialog", e);
+ }
+ }
+
+ private void collectToolBars(Composite parent, List<ToolBar> into) {
+ for (Control child : parent.getChildren()) {
+ if (child instanceof ToolBar toolBar) {
+ into.add(toolBar);
+ }
+ if (child instanceof Composite composite) {
+ collectToolBars(composite, into);
+ }
+ }
+ }
+
+ private static String openFileTitle() {
+ return BaseMessages.getString(BaseDialog.class, "BaseDialog.OpenFile");
+ }
+
+ private void createPipelineFile(String name) throws IOException {
+ Files.writeString(folder.resolve(name), "<pipeline/>");
+ }
+}
diff --git a/ui/src/main/java/org/apache/hop/ui/core/gui/WindowProperty.java
b/ui/src/main/java/org/apache/hop/ui/core/gui/WindowProperty.java
index 68a478b37e..200d1b6e82 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/gui/WindowProperty.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/gui/WindowProperty.java
@@ -20,9 +20,12 @@ package org.apache.hop.ui.core.gui;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
+import org.eclipse.swt.SWT;
import org.eclipse.swt.graphics.Point;
import org.eclipse.swt.graphics.Rectangle;
import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Event;
+import org.eclipse.swt.widgets.Listener;
import org.eclipse.swt.widgets.Monitor;
import org.eclipse.swt.widgets.Shell;
@@ -197,6 +200,46 @@ public class WindowProperty {
shell.setBounds(resizedRect);
}
+
+ relayoutOnShow(shell);
+ }
+
+ /**
+ * Re-run the shell's layout once it is actually on screen.
+ *
+ * <p>Restoring a remembered geometry means the widgets are built - and laid
out - while the shell
+ * still has its pre-open size. {@link Shell#setBounds(Rectangle)} sets the
*outer* bounds, so the
+ * client area only loses the window trim (title bar) when the shell is
shown. Composites whose
+ * bounds are handed to them by a parent layout that does not re-run - a
{@link
+ * org.eclipse.swt.custom.SashForm} pane is the common case - then keep
child positions that are
+ * off by the trim height, which shows up as a toolbar clipped against the
top of the panel until
+ * the user resizes the window by hand.
+ *
+ * <p>A single recursive layout on the first show costs nothing noticeable
and puts every child
+ * where the freshly-sized shell says it belongs.
+ */
+ private void relayoutOnShow(Shell shell) {
+ shell.addListener(
+ SWT.Show,
+ new Listener() {
+ @Override
+ public void handleEvent(Event event) {
+ shell.removeListener(SWT.Show, this);
+ // Asynchronous: on the first show the platform is still settling
the window frame, so
+ // the client area is only final once the pending events have been
handled.
+ shell.getDisplay().asyncExec(() -> layoutIfAlive(shell));
+ }
+ });
+ // A shell that is already up gets its layout straight away: no Show event
is coming.
+ if (shell.isVisible()) {
+ layoutIfAlive(shell);
+ }
+ }
+
+ private static void layoutIfAlive(Shell shell) {
+ if (!shell.isDisposed()) {
+ shell.layout(true, true);
+ }
}
/**
