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 b7b8d6bb11 Issue #8114 : Focus Launch in the run dialog so Enter and
Space start the run (#8120)
b7b8d6bb11 is described below
commit b7b8d6bb113da4f808938aff75ebfcf4353250ba
Author: Matt Casters <[email protected]>
AuthorDate: Thu Aug 27 14:09:45 2026 +0200
Issue #8114 : Focus Launch in the run dialog so Enter and Space start the
run (#8120)
The run options dialog set a default button but did not keep keyboard
focus on Launch. Enter could still fire the default button; Space went
to Cancel, the run-configuration combo, or a Parameters cell editor
started by TableView.clearAll() before the shell was open.
Focus Launch after the shell is activated, and only auto-edit the first
cell from clearAll() when the grid is already visible.
---
.../dialog/ConfigurationDialogLaunchFocusTest.java | 92 +++++++++++++
.../hop/ui/core/widget/TableViewClearAllTest.java | 153 +++++++++++++++++++++
.../hop/ui/core/dialog/ConfigurationDialog.java | 30 +++-
.../org/apache/hop/ui/core/widget/TableView.java | 17 ++-
4 files changed, 289 insertions(+), 3 deletions(-)
diff --git
a/rcp/src/test/java/org/apache/hop/ui/core/dialog/ConfigurationDialogLaunchFocusTest.java
b/rcp/src/test/java/org/apache/hop/ui/core/dialog/ConfigurationDialogLaunchFocusTest.java
new file mode 100644
index 0000000000..7bae1a07b7
--- /dev/null
+++
b/rcp/src/test/java/org/apache/hop/ui/core/dialog/ConfigurationDialogLaunchFocusTest.java
@@ -0,0 +1,92 @@
+/*
+ * 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.dialog;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
+import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.layout.RowLayout;
+import org.eclipse.swt.widgets.Button;
+import org.eclipse.swt.widgets.Table;
+import org.eclipse.swt.widgets.TableItem;
+import org.eclipse.swtbot.swt.finder.SWTBot;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Space only launches when Launch has keyboard focus. SWT otherwise focuses
the first child
+ * (Cancel) or a table that queued {@code setFocus} from {@code clearAll()}.
+ */
+@Tag("uitest")
+class ConfigurationDialogLaunchFocusTest extends SwtBotTestBase {
+
+ @Test
+ void launchKeepsKeyboardFocusDespiteALaterTableFocus() {
+ AtomicReference<Button> launchRef = new AtomicReference<>();
+
+ withScene(
+ shell -> {
+ shell.setLayout(new RowLayout());
+ Button cancel = new Button(shell, SWT.PUSH);
+ cancel.setText("Cancel");
+ Button launch = new Button(shell, SWT.PUSH);
+ launch.setText("Launch");
+ Table table = new Table(shell, SWT.BORDER | SWT.FULL_SELECTION);
+ new TableItem(table, SWT.NONE);
+ shell.setDefaultButton(launch);
+ // Same timing as TableView.clearAll(): an asyncExec queued before
the shell is open.
+ shell.getDisplay().asyncExec(table::setFocus);
+ ConfigurationDialog.focusLaunchButtonWhenActivated(shell, launch);
+ launchRef.set(launch);
+ },
+ bot ->
+ assertTrue(
+ waitUntil(bot, () -> onUi(() ->
launchRef.get().isFocusControl())),
+ "Launch must keep keyboard focus so Space activates it"));
+ }
+
+ private static boolean waitUntil(SWTBot bot, Supplier<Boolean> condition) {
+ for (int attempt = 0; attempt < 40; attempt++) {
+ if (Boolean.TRUE.equals(condition.get())) {
+ return true;
+ }
+ bot.sleep(25);
+ }
+ return Boolean.TRUE.equals(condition.get());
+ }
+
+ private static <T> T onUi(Supplier<T> supplier) {
+ AtomicReference<T> result = new AtomicReference<>();
+ AtomicReference<RuntimeException> failure = new AtomicReference<>();
+ display.syncExec(
+ () -> {
+ try {
+ result.set(supplier.get());
+ } catch (RuntimeException e) {
+ failure.set(e);
+ }
+ });
+ if (failure.get() != null) {
+ throw failure.get();
+ }
+ return result.get();
+ }
+}
diff --git
a/rcp/src/test/java/org/apache/hop/ui/core/widget/TableViewClearAllTest.java
b/rcp/src/test/java/org/apache/hop/ui/core/widget/TableViewClearAllTest.java
new file mode 100644
index 0000000000..8b0dd6ab73
--- /dev/null
+++ b/rcp/src/test/java/org/apache/hop/ui/core/widget/TableViewClearAllTest.java
@@ -0,0 +1,153 @@
+/*
+ * 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.widget;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.layout.FillLayout;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swt.widgets.Text;
+import org.eclipse.swtbot.swt.finder.SWTBot;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * {@link TableView#clearAll(boolean)} used to always start editing the first
cell. That stole focus
+ * from the run-dialog Launch button when Parameters were filled before the
shell was open.
+ */
+@Tag("uitest")
+class TableViewClearAllTest extends SwtBotTestBase {
+
+ @Test
+ void clearAllBeforeTheShellIsOpenDoesNotStartAnEditor() {
+ AtomicReference<TableView> viewRef = new AtomicReference<>();
+
+ withScene(
+ parent -> {
+ Shell dialog = new Shell(parent, SWT.DIALOG_TRIM);
+ dialog.setLayout(new FillLayout());
+ TableView view = newTableView(dialog);
+ view.clearAll(false);
+ viewRef.set(view);
+ },
+ bot -> {
+ drainUi();
+ bot.sleep(50);
+ assertNull(
+ onUi(() -> findText(viewRef.get().table)),
+ "clearAll on a hidden shell must not open a cell editor");
+ });
+ }
+
+ @Test
+ void clearAllOnAVisibleGridStillStartsEditing() {
+ AtomicReference<TableView> viewRef = new AtomicReference<>();
+
+ withScene(
+ shell -> {
+ shell.setLayout(new FillLayout());
+ viewRef.set(newTableView(shell));
+ },
+ bot -> {
+ onUi(
+ () -> {
+ viewRef.get().clearAll(false);
+ return null;
+ });
+ assertNotNull(
+ waitForEditor(bot, viewRef.get()),
+ "clearAll on a visible grid should still start editing");
+ });
+ }
+
+ private static TableView newTableView(Composite parent) {
+ ColumnInfo[] columns = {
+ new ColumnInfo("Name", ColumnInfo.COLUMN_TYPE_TEXT, false, false),
+ new ColumnInfo("Value", ColumnInfo.COLUMN_TYPE_TEXT, false, false),
+ };
+ return new TableView(
+ new Variables(),
+ parent,
+ SWT.BORDER | SWT.FULL_SELECTION,
+ columns,
+ 1,
+ null,
+ PropsUi.getInstance());
+ }
+
+ private static Text waitForEditor(SWTBot bot, TableView view) {
+ for (int attempt = 0; attempt < 40; attempt++) {
+ Text found = onUi(() -> findText(view.table));
+ if (found != null) {
+ return found;
+ }
+ bot.sleep(25);
+ }
+ return onUi(() -> findText(view.table));
+ }
+
+ private static Text findText(Composite parent) {
+ for (Control child : parent.getChildren()) {
+ if (child instanceof Text text && !text.isDisposed()) {
+ return text;
+ }
+ if (child instanceof Composite composite) {
+ Text found = findText(composite);
+ if (found != null) {
+ return found;
+ }
+ }
+ }
+ return null;
+ }
+
+ private static void drainUi() {
+ display.syncExec(
+ () -> {
+ while (display.readAndDispatch()) {
+ // flush clearAll asyncExec
+ }
+ });
+ }
+
+ private static <T> T onUi(Supplier<T> supplier) {
+ AtomicReference<T> result = new AtomicReference<>();
+ AtomicReference<RuntimeException> failure = new AtomicReference<>();
+ display.syncExec(
+ () -> {
+ try {
+ result.set(supplier.get());
+ } catch (RuntimeException e) {
+ failure.set(e);
+ }
+ });
+ if (failure.get() != null) {
+ throw failure.get();
+ }
+ return result.get();
+ }
+}
diff --git
a/ui/src/main/java/org/apache/hop/ui/core/dialog/ConfigurationDialog.java
b/ui/src/main/java/org/apache/hop/ui/core/dialog/ConfigurationDialog.java
index ebfa6e2fe4..c81a9a8ae7 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/dialog/ConfigurationDialog.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/dialog/ConfigurationDialog.java
@@ -51,6 +51,7 @@ import org.eclipse.swt.widgets.Control;
import org.eclipse.swt.widgets.Dialog;
import org.eclipse.swt.widgets.Group;
import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.Listener;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.TableItem;
@@ -365,12 +366,39 @@ public abstract class ConfigurationDialog extends Dialog {
}
protected void openDialog() {
- // Set the focus on the OK button
+ // setDefaultButton makes Enter launch, but Space only activates the
focused control.
shell.setDefaultButton(wOk);
+ focusLaunchButtonWhenActivated(shell, wOk);
BaseDialog.defaultShellHandling(shell, c -> ok(), c -> cancel());
}
+ /**
+ * Give {@code launch} keyboard focus after the shell is activated. A single
asyncExec queued
+ * before {@code shell.open()} loses to SWT's initial focus (first child)
and to a TableView cell
+ * editor queued from {@code clearAll()}. Space then types into that control
instead of Launch.
+ */
+ static void focusLaunchButtonWhenActivated(Shell shell, Button launch) {
+ if (shell == null || shell.isDisposed() || launch == null) {
+ return;
+ }
+ Runnable focusLaunch =
+ () -> {
+ if (!launch.isDisposed()) {
+ launch.setFocus();
+ }
+ };
+ Listener[] holder = new Listener[1];
+ holder[0] =
+ event -> {
+ shell.removeListener(SWT.Activate, holder[0]);
+ shell.getDisplay().asyncExec(focusLaunch);
+ };
+ shell.addListener(SWT.Activate, holder[0]);
+ // Hop Web may not fire Activate the same way; still try after the current
event burst.
+ shell.getDisplay().asyncExec(focusLaunch);
+ }
+
protected abstract void optionsSectionControls();
/**
diff --git a/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java
b/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java
index 3e69167ab5..05cfadb5e3 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java
@@ -2516,8 +2516,21 @@ public class TableView extends Composite {
if (id == SWT.YES) {
table.removeAll();
new TableItem(table, SWT.NONE);
- if (!readonly) {
- composite.getDisplay().asyncExec(() -> edit(0, 1));
+ // Only start editing when the user cleared an already-visible grid.
Programmatic refill
+ // before a dialog is opened (run options Parameters/Variables) must not
grab focus.
+ Shell parentShell = composite.getShell();
+ if (!readonly
+ && parentShell != null
+ && !parentShell.isDisposed()
+ && parentShell.isVisible()) {
+ composite
+ .getDisplay()
+ .asyncExec(
+ () -> {
+ if (!table.isDisposed() && table.getItemCount() > 0) {
+ edit(0, 1);
+ }
+ });
}
this.setModified(); // timh
}
