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 e049f5f655 fix naviagtion on canvas and some shortcuts, fixes #7848
(#7895)
e049f5f655 is described below
commit e049f5f6558c88aa0014788cb8ce9f1b512df622
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Thu Aug 13 09:05:50 2026 +0200
fix naviagtion on canvas and some shortcuts, fixes #7848 (#7895)
---
.../apache/hop/core/gui/plugin/GuiRegistry.java | 5 +-
.../hop/core/gui/plugin/key/KeyboardShortcut.java | 7 +-
.../org/apache/hop/ui/hopgui/HopWebEntryPoint.java | 21 +-
.../apache/hop/ui/hopgui/HopWebEntryPointTest.java | 30 +++
.../main/java/org/apache/hop/ui/core/PropsUi.java | 7 +
.../main/java/org/apache/hop/ui/hopgui/HopGui.java | 17 +-
.../org/apache/hop/ui/hopgui/HopGuiKeyHandler.java | 164 ++++++++++++-
.../hopgui/file/pipeline/HopGuiPipelineGraph.java | 49 ++++
.../ui/hopgui/file/shared/HopGuiAbstractGraph.java | 22 ++
.../hopgui/file/workflow/HopGuiWorkflowGraph.java | 46 ++++
.../perspective/execution/DragViewZoomBase.java | 88 ++++++-
.../apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java | 268 +++++++++++++++++++++
12 files changed, 691 insertions(+), 33 deletions(-)
diff --git a/core/src/main/java/org/apache/hop/core/gui/plugin/GuiRegistry.java
b/core/src/main/java/org/apache/hop/core/gui/plugin/GuiRegistry.java
index e35f464023..22b3f5504c 100644
--- a/core/src/main/java/org/apache/hop/core/gui/plugin/GuiRegistry.java
+++ b/core/src/main/java/org/apache/hop/core/gui/plugin/GuiRegistry.java
@@ -552,6 +552,7 @@ public class GuiRegistry {
List<KeyboardShortcut> shortcuts =
shortCutsMap.computeIfAbsent(guiPluginClassName, k -> new
ArrayList<>());
KeyboardShortcut keyboardShortCut = new KeyboardShortcut(shortcut, method);
+ keyboardShortCut.setParentClassName(guiPluginClassName);
shortcuts.add(keyboardShortCut);
}
@@ -559,7 +560,9 @@ public class GuiRegistry {
String parentClassName, Method parentMethod, GuiOsxKeyboardShortcut
shortcut) {
List<KeyboardShortcut> shortcuts =
shortCutsMap.computeIfAbsent(parentClassName, k -> new ArrayList<>());
- shortcuts.add(new KeyboardShortcut(shortcut, parentMethod));
+ KeyboardShortcut keyboardShortCut = new KeyboardShortcut(shortcut,
parentMethod);
+ keyboardShortCut.setParentClassName(parentClassName);
+ shortcuts.add(keyboardShortCut);
}
/**
diff --git
a/core/src/main/java/org/apache/hop/core/gui/plugin/key/KeyboardShortcut.java
b/core/src/main/java/org/apache/hop/core/gui/plugin/key/KeyboardShortcut.java
index e876b272dd..bff53911b9 100644
---
a/core/src/main/java/org/apache/hop/core/gui/plugin/key/KeyboardShortcut.java
+++
b/core/src/main/java/org/apache/hop/core/gui/plugin/key/KeyboardShortcut.java
@@ -31,7 +31,12 @@ public class KeyboardShortcut {
/** If true, shortcut works from anywhere (e.g. activate perspective, toggle
terminal). */
private boolean global;
- /** Class that declares the shortcut (where it should be triggered). */
+ /**
+ * Class the shortcut is triggered on. It defaults to the class declaring
the annotated method but
+ * is set to the GUI plugin class when the shortcut is registered: a
shortcut method can be
+ * inherited from a base class (the canvas navigation keys are declared on
DragViewZoomBase and
+ * inherited by the pipeline and workflow graphs).
+ */
private String parentClassName;
private String parentMethodName;
diff --git a/rap/src/main/java/org/apache/hop/ui/hopgui/HopWebEntryPoint.java
b/rap/src/main/java/org/apache/hop/ui/hopgui/HopWebEntryPoint.java
index 352b48f105..499ace78b9 100644
--- a/rap/src/main/java/org/apache/hop/ui/hopgui/HopWebEntryPoint.java
+++ b/rap/src/main/java/org/apache/hop/ui/hopgui/HopWebEntryPoint.java
@@ -59,11 +59,12 @@ public class HopWebEntryPoint extends AbstractEntryPoint {
Set.of("CTRL+C", "CTRL+V", "CTRL+X");
/**
- * Bare navigation keys used for caret movement and selection in text fields
(and tree widgets).
- * They may stay in {@code ACTIVE_KEYS} so canvas pan shortcuts still reach
the server when focus
- * is on the graph, but must not be in {@code CANCEL_KEYS} or the browser
never moves the caret
- * (see issue #7833). Modifier combinations (CTRL+ARROW_*, ALT+ARROW_*, …)
are separate RAP keys
- * and remain cancelled when registered as application shortcuts.
+ * Navigation keys used for caret movement and selection in text fields (and
tree widgets), both
+ * bare and with SHIFT held down to extend the selection. They may stay in
{@code ACTIVE_KEYS} so
+ * the canvas navigation shortcuts still reach the server when focus is on
the graph, but must not
+ * be in {@code CANCEL_KEYS} or the browser never moves the caret (see issue
#7833). Modifier
+ * combinations with CTRL or ALT are separate RAP keys and remain cancelled
when registered as
+ * application shortcuts.
*/
private static final Set<String> NATIVE_TEXT_NAVIGATION_KEYS =
Set.of(
@@ -74,7 +75,15 @@ public class HopWebEntryPoint extends AbstractEntryPoint {
"HOME",
"END",
"PAGE_UP",
- "PAGE_DOWN");
+ "PAGE_DOWN",
+ "SHIFT+ARROW_UP",
+ "SHIFT+ARROW_DOWN",
+ "SHIFT+ARROW_LEFT",
+ "SHIFT+ARROW_RIGHT",
+ "SHIFT+HOME",
+ "SHIFT+END",
+ "SHIFT+PAGE_UP",
+ "SHIFT+PAGE_DOWN");
/** Audit group/type/name for Hop Web theme preference (per-user in audit
folder). */
public static final String AUDIT_GROUP_HOP_WEB = "hop-web";
diff --git
a/rap/src/test/java/org/apache/hop/ui/hopgui/HopWebEntryPointTest.java
b/rap/src/test/java/org/apache/hop/ui/hopgui/HopWebEntryPointTest.java
index 60c65c1c7e..022dbef3ab 100644
--- a/rap/src/test/java/org/apache/hop/ui/hopgui/HopWebEntryPointTest.java
+++ b/rap/src/test/java/org/apache/hop/ui/hopgui/HopWebEntryPointTest.java
@@ -24,6 +24,7 @@ import static org.mockito.Mockito.when;
import java.util.Arrays;
import org.apache.hop.core.gui.plugin.key.KeyboardShortcut;
+import org.eclipse.swt.SWT;
import org.junit.jupiter.api.Test;
class HopWebEntryPointTest {
@@ -60,6 +61,35 @@ class HopWebEntryPointTest {
assertArrayEquals(new String[] {"CTRL+ARROW_LEFT", "CTRL+S"},
cancelledShortcuts);
}
+ @Test
+ void preservesShiftNavigationKeysAsActiveOnly() {
+ // SHIFT+arrow moves the selection on the canvas but still extends the
selection in text
+ // fields, so the browser must keep handling it.
+ String[] activeShortcuts = {
+ "SHIFT+ARROW_LEFT",
+ "SHIFT+ARROW_RIGHT",
+ "SHIFT+ARROW_UP",
+ "SHIFT+ARROW_DOWN",
+ "SHIFT+HOME",
+ "SHIFT+END",
+ "CTRL+SHIFT+ARROW_UP",
+ "CTRL+S"
+ };
+
+ String[] cancelledShortcuts =
HopWebEntryPoint.buildCancelledKeyboardShortcuts(activeShortcuts);
+
+ assertArrayEquals(new String[] {"CTRL+SHIFT+ARROW_UP", "CTRL+S"},
cancelledShortcuts);
+ }
+
+ @Test
+ void mapsShiftArrowShortcutToRapFormat() {
+ KeyboardShortcut shortcut = mock(KeyboardShortcut.class);
+ when(shortcut.getKeyCode()).thenReturn(SWT.ARROW_LEFT);
+ when(shortcut.isShift()).thenReturn(true);
+
+ assertEquals("SHIFT+ARROW_LEFT", new
HopWebEntryPoint().convertToRapFormat(shortcut));
+ }
+
@Test
void removesDuplicateCancelledShortcuts() {
String[] cancelledShortcuts =
diff --git a/ui/src/main/java/org/apache/hop/ui/core/PropsUi.java
b/ui/src/main/java/org/apache/hop/ui/core/PropsUi.java
index cd1119896b..62ad5d857a 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/PropsUi.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/PropsUi.java
@@ -34,6 +34,7 @@ import org.apache.hop.ui.core.gui.GuiResource;
import org.apache.hop.ui.core.gui.WindowProperty;
import org.apache.hop.ui.core.widget.OsHelper;
import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.HopGuiKeyHandler;
import org.apache.hop.ui.hopgui.TextSizeUtilFacade;
import org.apache.hop.ui.util.EnvironmentUtils;
import org.eclipse.swt.SWT;
@@ -774,6 +775,12 @@ public class PropsUi extends Props {
}
public static void setLook(final Widget widget, int style) {
+ // Handle the keyboard shortcuts of widgets that are created after their
shell was set up, e.g.
+ // when a metadata editor rebuilds a section. In Hop Web the key listener
has to be there when
+ // the widget is rendered or RAP never sends its key events to the server.
+ //
+ HopGuiKeyHandler.getInstance().attachTo(widget);
+
if (EnvironmentUtils.getInstance().isWeb()) {
setLookOnWeb(widget, style);
return;
diff --git a/ui/src/main/java/org/apache/hop/ui/hopgui/HopGui.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/HopGui.java
index 8ba94c6e95..a5654476df 100644
--- a/ui/src/main/java/org/apache/hop/ui/hopgui/HopGui.java
+++ b/ui/src/main/java/org/apache/hop/ui/hopgui/HopGui.java
@@ -1969,7 +1969,20 @@ public class HopGui
return;
}
- if (control.getData("HOP_TERMINAL_WIDGET") == Boolean.TRUE) {
+ // Widgets that this shell creates later on (a metadata editor rebuilding
a section, ...) are
+ // covered when they get the focus.
+ //
+ keyHandler.addHandledShell(display, control.getShell());
+
+ addKeyboardShortcutListeners(control, keyHandler);
+ }
+
+ private void addKeyboardShortcutListeners(Control control, HopGuiKeyHandler
keyHandler) {
+ if (control == null || control.isDisposed()) {
+ return;
+ }
+
+ if (control.getData(HopGuiKeyHandler.HOP_TERMINAL_WIDGET) == Boolean.TRUE)
{
return;
}
@@ -1980,7 +1993,7 @@ public class HopGui
//
if (control instanceof Composite compositeControl) {
for (Control child : compositeControl.getChildren()) {
- replaceKeyboardShortcutListeners(child, keyHandler);
+ addKeyboardShortcutListeners(child, keyHandler);
}
}
}
diff --git a/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java
index 9d2d467c75..8b6030100d 100644
--- a/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java
+++ b/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java
@@ -25,6 +25,7 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import org.apache.hop.core.Const;
import org.apache.hop.core.gui.plugin.GuiRegistry;
import org.apache.hop.core.gui.plugin.key.KeyboardShortcut;
@@ -35,13 +36,27 @@ import org.eclipse.swt.SWTException;
import org.eclipse.swt.custom.CCombo;
import org.eclipse.swt.events.KeyAdapter;
import org.eclipse.swt.events.KeyEvent;
+import org.eclipse.swt.events.KeyListener;
import org.eclipse.swt.widgets.Combo;
+import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swt.widgets.Table;
+import org.eclipse.swt.widgets.TableItem;
import org.eclipse.swt.widgets.Text;
+import org.eclipse.swt.widgets.Tree;
+import org.eclipse.swt.widgets.TreeItem;
import org.eclipse.swt.widgets.Widget;
public class HopGuiKeyHandler extends KeyAdapter {
+ /** Data key marking the terminal widget, which handles all keys itself. */
+ public static final String HOP_TERMINAL_WIDGET = "HOP_TERMINAL_WIDGET";
+
+ /** Widget classes that pass their key listeners on to a widget inside them.
*/
+ private static final Map<Class<?>, Boolean> DELEGATING_KEY_LISTENERS = new
ConcurrentHashMap<>();
+
private static HopGuiKeyHandler singleton;
public Set<Object> parentObjects;
@@ -49,6 +64,15 @@ public class HopGuiKeyHandler extends KeyAdapter {
/** Parent -> Control (e.g. shell) so we try that parent when focus is in
its window. */
private final Map<Object, Control> parentToControl = new HashMap<>();
+ /** Shells this handler covers, used to also cover the widgets they create
later on. */
+ private final Set<Shell> handledShells = new HashSet<>();
+
+ /**
+ * Displays with a focus filter. This handler is a singleton for the whole
process while Hop Web
+ * has a display per session, so the filter is installed once per display.
+ */
+ private final Set<Display> filteredDisplays = new HashSet<>();
+
private HopGuiKeyHandler() {
this.parentObjects = new HashSet<>();
}
@@ -77,6 +101,96 @@ public class HopGuiKeyHandler extends KeyAdapter {
parentToControl.remove(parentObject);
}
+ /**
+ * Start handling the keys of a shell, including those of the widgets it
creates later on.
+ *
+ * <p>The handler is a key listener that is attached to every widget of a
shell. Widgets created
+ * after that, when a dialog or metadata editor rebuilds part of its content
(e.g. the OAuth 2
+ * fields of a REST connection when the authentication type changes), have
no key listener of
+ * their own, so keyboard shortcuts like Ctrl+S do nothing while such a
widget has the focus.
+ * {@link #attachTo(Widget)} handles those, called from {@code
PropsUi.setLook()} when the widget
+ * is created and, on the desktop, when a widget receives the focus.
+ *
+ * @param display the display to listen to for focus changes
+ * @param shell the shell whose widgets are handled
+ */
+ public void addHandledShell(Display display, Shell shell) {
+ if (shell == null || shell.isDisposed() || !handledShells.add(shell)) {
+ return;
+ }
+ shell.addDisposeListener(e -> handledShells.remove(shell));
+
+ // Safety net for widgets that are created without PropsUi.setLook(). Hop
Web has no use for it:
+ // RAP does not fire focus events for a focus change made in the browser.
+ //
+ if (display != null && !display.isDisposed() &&
filteredDisplays.add(display)) {
+ display.addFilter(SWT.FocusIn, event -> attachTo(event.widget));
+ display.addListener(SWT.Dispose, e -> filteredDisplays.remove(display));
+ }
+ }
+
+ /**
+ * Attach this handler to a widget of a shell we handle, so its keyboard
shortcuts work.
+ *
+ * <p>This has to happen when the widget is created: RAP only sends the key
events of a widget to
+ * the server when that widget has a key listener at the time it is
rendered, so attaching later
+ * is too late for Hop Web.
+ *
+ * @param widget the widget to handle the keys of
+ * @return true if the handler was attached to the widget
+ */
+ public boolean attachTo(Widget widget) {
+ if (!(widget instanceof Control control) || control.isDisposed()) {
+ return false;
+ }
+ try {
+ if (!handledShells.contains(control.getShell())
+ || isInTerminalWidget(control)
+ || delegatesKeyListeners(control)) {
+ return false;
+ }
+ // Removing first avoids handling the key twice for widgets that already
have the handler
+ control.removeKeyListener(this);
+ control.addKeyListener(this);
+ return true;
+ } catch (SWTException e) {
+ return false;
+ }
+ }
+
+ /**
+ * Composite widgets like TextVar and ComboVar pass the key listeners they
get on to the widget
+ * inside them. They style themselves before creating that widget, so it is
not always there yet,
+ * and it gets the handler through its own {@code PropsUi.setLook()} call
anyway.
+ */
+ private static boolean delegatesKeyListeners(Control control) {
+ if (!(control instanceof Composite)) {
+ return false;
+ }
+ return DELEGATING_KEY_LISTENERS.computeIfAbsent(
+ control.getClass(),
+ widgetClass -> {
+ try {
+ Method method = widgetClass.getMethod("addKeyListener",
KeyListener.class);
+ return !Control.class.equals(method.getDeclaringClass());
+ } catch (NoSuchMethodException e) {
+ return Boolean.FALSE;
+ }
+ });
+ }
+
+ /** The terminal widget and everything in it handles all keys itself. */
+ private static boolean isInTerminalWidget(Control control) {
+ Control current = control;
+ while (current != null) {
+ if (current.getData(HOP_TERMINAL_WIDGET) == Boolean.TRUE) {
+ return true;
+ }
+ current = current.getParent();
+ }
+ return false;
+ }
+
@Override
public void keyPressed(KeyEvent event) {
if (!event.doit) {
@@ -92,6 +206,13 @@ public class HopGuiKeyHandler extends KeyAdapter {
return;
}
+ // Same for tables, trees and lists: they use the arrow keys to move
through their rows. Those
+ // widgets also live inside the pipeline and workflow graph (log, preview
and result tabs) where
+ // the arrow keys navigate the canvas.
+ if (isRowNavigationWidget(event.widget) && isCaretNavigationKey(event)) {
+ return;
+ }
+
List<Object> orderedParents = getParentObjectsInContextOrder(event.widget);
for (Object parentObject : orderedParents) {
List<KeyboardShortcut> shortcuts =
@@ -283,6 +404,17 @@ public class HopGuiKeyHandler extends KeyAdapter {
|| isStyledText(widget);
}
+ /**
+ * Returns true if the widget is a Table, Tree or List: they navigate rows
with the arrow keys.
+ */
+ private static boolean isRowNavigationWidget(Widget widget) {
+ return widget instanceof Table
+ || widget instanceof Tree
+ || widget instanceof TableItem
+ || widget instanceof TreeItem
+ || widget instanceof org.eclipse.swt.widgets.List;
+ }
+
/**
* Keys that text-like widgets must handle themselves:
copy/cut/paste/select-all,
* delete/backspace, and caret / selection navigation (arrows, home/end,
page up/down) without
@@ -298,20 +430,26 @@ public class HopGuiKeyHandler extends KeyAdapter {
if (event.keyCode == SWT.DEL || event.character == SWT.BS) {
return true;
}
- // Caret movement / selection: leave for the widget unless an app modifier
is held
- boolean hasAppModifier = (event.stateMask & (SWT.CONTROL | SWT.COMMAND |
SWT.ALT)) != 0;
- if (!hasAppModifier) {
- int keyCode = event.keyCode & SWT.KEY_MASK;
- return keyCode == SWT.ARROW_LEFT
- || keyCode == SWT.ARROW_RIGHT
- || keyCode == SWT.ARROW_UP
- || keyCode == SWT.ARROW_DOWN
- || keyCode == SWT.HOME
- || keyCode == SWT.END
- || keyCode == SWT.PAGE_UP
- || keyCode == SWT.PAGE_DOWN;
+ return isCaretNavigationKey(event);
+ }
+
+ /**
+ * Caret and row movement keys: the arrows, home/end and page up/down
without CTRL/CMD/ALT. SHIFT
+ * alone is allowed so extending a selection stays in the widget as well.
+ */
+ private static boolean isCaretNavigationKey(KeyEvent event) {
+ if ((event.stateMask & (SWT.CONTROL | SWT.COMMAND | SWT.ALT)) != 0) {
+ return false;
}
- return false;
+ int keyCode = event.keyCode & SWT.KEY_MASK;
+ return keyCode == SWT.ARROW_LEFT
+ || keyCode == SWT.ARROW_RIGHT
+ || keyCode == SWT.ARROW_UP
+ || keyCode == SWT.ARROW_DOWN
+ || keyCode == SWT.HOME
+ || keyCode == SWT.END
+ || keyCode == SWT.PAGE_UP
+ || keyCode == SWT.PAGE_DOWN;
}
/**
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
index 1f6e04f19d..b7e1d85029 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
@@ -2477,6 +2477,55 @@ public class HopGuiPipelineGraph extends
HopGuiAbstractGraph
}
}
+ /** Move the selected transforms and notes with the arrow keys, as a single
undo action. */
+ @Override
+ protected boolean nudgeSelectedElements(int dx, int dy) {
+ List<TransformMeta> transforms = pipelineMeta.getSelectedTransforms();
+ List<NotePadMeta> notes = pipelineMeta.getSelectedNotes();
+ if (Utils.isEmpty(transforms) && Utils.isEmpty(notes)) {
+ return false;
+ }
+
+ Point[] transformsBefore = captureLocations(transforms);
+ Point[] notesBefore = captureNoteLocations(notes);
+
+ moveSelected(dx, dy);
+
+ Point[] transformsAfter = captureLocations(transforms);
+ Point[] notesAfter = captureNoteLocations(notes);
+ if (Arrays.equals(transformsBefore, transformsAfter)
+ && Arrays.equals(notesBefore, notesAfter)) {
+ // Nothing moved: the selection is up against the top or left side of
the canvas.
+ return true;
+ }
+
+ // Record notes first, then transforms, linked into a single undo action
(nextAlso).
+ boolean also = false;
+ if (!Utils.isEmpty(notes)) {
+ also = !Utils.isEmpty(transforms);
+ hopGui.undoDelegate.addUndoPosition(
+ pipelineMeta,
+ notes.toArray(new NotePadMeta[0]),
+ pipelineMeta.getNoteIndexes(notes),
+ notesBefore,
+ notesAfter,
+ also);
+ }
+ if (!Utils.isEmpty(transforms)) {
+ hopGui.undoDelegate.addUndoPosition(
+ pipelineMeta,
+ transforms.toArray(new TransformMeta[0]),
+ pipelineMeta.getTransformIndexes(transforms),
+ transformsBefore,
+ transformsAfter,
+ also);
+ }
+
+ pipelineMeta.setChanged();
+ updateGui();
+ return true;
+ }
+
private void addCandidateAsHop(int mouseX, int mouseY) {
if (candidate == null) {
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/shared/HopGuiAbstractGraph.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/shared/HopGuiAbstractGraph.java
index 68f7ac10e8..1e973d7ef8 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/shared/HopGuiAbstractGraph.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/shared/HopGuiAbstractGraph.java
@@ -153,6 +153,28 @@ public abstract class HopGuiAbstractGraph extends
DragViewZoomBase
toolTip.setVisible(true);
}
+ /**
+ * On the pipeline and workflow canvas the arrow keys move the selected
transforms, actions and
+ * notes. Only when nothing is selected they pan the view.
+ */
+ @Override
+ protected boolean moveSelectionWithArrowKey(int dx, int dy, boolean
largeStep) {
+ int step = Math.max(1, PropsUi.getInstance().getCanvasGridSize());
+ if (largeStep) {
+ step *= LARGE_STEP_FACTOR;
+ }
+ return nudgeSelectedElements(dx * step, dy * step);
+ }
+
+ /**
+ * Move the selected elements on the canvas over the given distance, as one
undo action.
+ *
+ * @param dx the horizontal distance in graph coordinates
+ * @param dy the vertical distance in graph coordinates
+ * @return true if elements were selected and moved, false if there was
nothing to move
+ */
+ protected abstract boolean nudgeSelectedElements(int dx, int dy);
+
public abstract SnapAllignDistribute createSnapAlignDistribute();
@Override
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/HopGuiWorkflowGraph.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/HopGuiWorkflowGraph.java
index 3d0d7517ad..713ce64efa 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/HopGuiWorkflowGraph.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/HopGuiWorkflowGraph.java
@@ -3613,6 +3613,52 @@ public class HopGuiWorkflowGraph extends
HopGuiAbstractGraph
}
}
+ /** Move the selected actions and notes with the arrow keys, as a single
undo action. */
+ @Override
+ protected boolean nudgeSelectedElements(int dx, int dy) {
+ List<ActionMeta> actions = workflowMeta.getSelectedActions();
+ List<NotePadMeta> notes = workflowMeta.getSelectedNotes();
+ if (Utils.isEmpty(actions) && Utils.isEmpty(notes)) {
+ return false;
+ }
+
+ Point[] actionsBefore = captureLocations(actions);
+ Point[] notesBefore = captureNoteLocations(notes);
+
+ moveSelected(dx, dy);
+
+ Point[] actionsAfter = captureLocations(actions);
+ Point[] notesAfter = captureNoteLocations(notes);
+ if (Arrays.equals(actionsBefore, actionsAfter) &&
Arrays.equals(notesBefore, notesAfter)) {
+ // Nothing moved: the selection is up against the top or left side of
the canvas.
+ return true;
+ }
+
+ // Record notes first, then actions, linked into a single undo action
(nextAlso).
+ boolean also = false;
+ if (!Utils.isEmpty(notes)) {
+ also = !Utils.isEmpty(actions);
+ addUndoPosition(
+ notes.toArray(new NotePadMeta[0]),
+ workflowMeta.getNoteIndexes(notes),
+ notesBefore,
+ notesAfter,
+ also);
+ }
+ if (!Utils.isEmpty(actions)) {
+ addUndoPosition(
+ actions.toArray(new ActionMeta[0]),
+ workflowMeta.getActionIndexes(actions),
+ actionsBefore,
+ actionsAfter,
+ also);
+ }
+
+ workflowMeta.setChanged();
+ updateGui();
+ return true;
+ }
+
private void modalMessageDialog(String title, String message, int swtFlags) {
MessageBox messageBox = new MessageBox(hopShell(), swtFlags);
messageBox.setMessage(message);
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/DragViewZoomBase.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/DragViewZoomBase.java
index 086b192646..633bc9665c 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/DragViewZoomBase.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/DragViewZoomBase.java
@@ -30,6 +30,15 @@ import org.eclipse.swt.widgets.Canvas;
import org.eclipse.swt.widgets.Composite;
public abstract class DragViewZoomBase extends Composite {
+
+ /** The distance the view pans for one arrow key press, in graph coordinates
at 100% zoom. */
+ private static final double PAN_STEP = 20;
+
+ /**
+ * How much larger a step is when SHIFT is held down, both for panning and
for moving elements.
+ */
+ protected static final int LARGE_STEP_FACTOR = 5;
+
protected Canvas canvas;
protected DPoint offset;
protected Point maximum;
@@ -98,35 +107,94 @@ public abstract class DragViewZoomBase extends Composite {
@GuiKeyboardShortcut(key = SWT.ARROW_LEFT)
@GuiOsxKeyboardShortcut(key = SWT.ARROW_LEFT)
public void viewLeft() {
- offset.x += 15 * magnification * PropsUi.getNativeZoomFactor();
- validateOffset();
- redraw();
+ arrowKey(-1, 0, false);
}
@GuiKeyboardShortcut(key = SWT.ARROW_RIGHT)
@GuiOsxKeyboardShortcut(key = SWT.ARROW_RIGHT)
public void viewRight() {
- offset.x -= 15 * magnification * PropsUi.getNativeZoomFactor();
- validateOffset();
- redraw();
+ arrowKey(1, 0, false);
}
@GuiKeyboardShortcut(key = SWT.ARROW_UP)
@GuiOsxKeyboardShortcut(key = SWT.ARROW_UP)
public void viewUp() {
- offset.y += 15 * magnification * PropsUi.getNativeZoomFactor();
- validateOffset();
- redraw();
+ arrowKey(0, -1, false);
}
@GuiKeyboardShortcut(key = SWT.ARROW_DOWN)
@GuiOsxKeyboardShortcut(key = SWT.ARROW_DOWN)
public void viewDown() {
- offset.y -= 15 * magnification * PropsUi.getNativeZoomFactor();
+ arrowKey(0, 1, false);
+ }
+
+ @GuiKeyboardShortcut(shift = true, key = SWT.ARROW_LEFT)
+ @GuiOsxKeyboardShortcut(shift = true, key = SWT.ARROW_LEFT)
+ public void viewLeftFast() {
+ arrowKey(-1, 0, true);
+ }
+
+ @GuiKeyboardShortcut(shift = true, key = SWT.ARROW_RIGHT)
+ @GuiOsxKeyboardShortcut(shift = true, key = SWT.ARROW_RIGHT)
+ public void viewRightFast() {
+ arrowKey(1, 0, true);
+ }
+
+ @GuiKeyboardShortcut(shift = true, key = SWT.ARROW_UP)
+ @GuiOsxKeyboardShortcut(shift = true, key = SWT.ARROW_UP)
+ public void viewUpFast() {
+ arrowKey(0, -1, true);
+ }
+
+ @GuiKeyboardShortcut(shift = true, key = SWT.ARROW_DOWN)
+ @GuiOsxKeyboardShortcut(shift = true, key = SWT.ARROW_DOWN)
+ public void viewDownFast() {
+ arrowKey(0, 1, true);
+ }
+
+ /**
+ * Handle an arrow key: move the selected elements when there are any, pan
the view otherwise.
+ *
+ * @param dx -1, 0 or 1: the horizontal direction
+ * @param dy -1, 0 or 1: the vertical direction
+ * @param largeStep true when SHIFT is held down: take a larger step
+ */
+ protected void arrowKey(int dx, int dy, boolean largeStep) {
+ if (moveSelectionWithArrowKey(dx, dy, largeStep)) {
+ return;
+ }
+ double step = calculatePanStep(largeStep);
+ offset.x -= dx * step;
+ offset.y -= dy * step;
validateOffset();
redraw();
}
+ /**
+ * Move the elements selected on the canvas with the arrow keys. Only the
editors (pipeline and
+ * workflow graph) support this, the execution viewers always pan the view.
+ *
+ * @param dx -1, 0 or 1: the horizontal direction
+ * @param dy -1, 0 or 1: the vertical direction
+ * @param largeStep true when SHIFT is held down: take a larger step
+ * @return true if the arrow key moved a selection, false to pan the view
instead
+ */
+ protected boolean moveSelectionWithArrowKey(int dx, int dy, boolean
largeStep) {
+ return false;
+ }
+
+ /**
+ * The distance to pan for a single arrow key press, in graph coordinates.
It compensates for the
+ * magnification so that the view always moves the same visible distance.
+ *
+ * @param largeStep true to take a larger step (SHIFT)
+ * @return the distance to pan in graph coordinates
+ */
+ protected double calculatePanStep(boolean largeStep) {
+ double step = PAN_STEP / Math.max(0.1f, magnification);
+ return largeStep ? step * LARGE_STEP_FACTOR : step;
+ }
+
@GuiKeyboardShortcut(control = true, key = '+')
@GuiOsxKeyboardShortcut(command = true, key = '+')
public void zoomIn() {
diff --git
a/ui/src/test/java/org/apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java
b/ui/src/test/java/org/apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java
new file mode 100644
index 0000000000..14e34093e3
--- /dev/null
+++ b/ui/src/test/java/org/apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java
@@ -0,0 +1,268 @@
+/*
+ * 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.hopgui;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hop.core.gui.plugin.GuiRegistry;
+import org.apache.hop.core.gui.plugin.key.GuiKeyboardShortcut;
+import org.apache.hop.core.gui.plugin.key.GuiOsxKeyboardShortcut;
+import org.apache.hop.core.gui.plugin.key.KeyboardShortcut;
+import org.apache.hop.ui.core.widget.TextVar;
+import org.apache.hop.ui.hopgui.file.pipeline.HopGuiPipelineGraph;
+import org.apache.hop.ui.hopgui.file.workflow.HopGuiWorkflowGraph;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.events.KeyEvent;
+import org.eclipse.swt.widgets.Canvas;
+import org.eclipse.swt.widgets.Event;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swt.widgets.Table;
+import org.eclipse.swt.widgets.Text;
+import org.eclipse.swt.widgets.Tree;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests keyboard shortcut dispatching, in particular for shortcuts that a
{@code @GuiPlugin} class
+ * inherits from a super class. The canvas navigation keys (arrows, HOME,
zoom) of the pipeline and
+ * workflow graphs are declared on {@code DragViewZoomBase} and inherited, so
they only work when
+ * inherited shortcuts are dispatched.
+ */
+class HopGuiKeyHandlerTest {
+
+ /** Stands in for DragViewZoomBase: declares the canvas navigation shortcut.
*/
+ public static class NavigationBase {
+ public int left;
+ public int leftFast;
+
+ @GuiKeyboardShortcut(key = SWT.ARROW_LEFT)
+ @GuiOsxKeyboardShortcut(key = SWT.ARROW_LEFT)
+ public void viewLeft() {
+ left++;
+ }
+
+ @GuiKeyboardShortcut(shift = true, key = SWT.ARROW_LEFT)
+ @GuiOsxKeyboardShortcut(shift = true, key = SWT.ARROW_LEFT)
+ public void viewLeftFast() {
+ leftFast++;
+ }
+ }
+
+ /** Stands in for HopGuiPipelineGraph: the @GuiPlugin class that inherits
the shortcuts. */
+ public static class NavigationGraph extends NavigationBase {}
+
+ @Test
+ void shortcutInheritedFromSuperClassIsDispatched() {
+ NavigationGraph graph = new NavigationGraph();
+ registerShortcutsLikeHopGuiEnvironment(NavigationGraph.class);
+
+ HopGuiKeyHandler keyHandler = HopGuiKeyHandler.getInstance();
+ keyHandler.addParentObjectToHandle(graph);
+ try {
+ keyHandler.keyPressed(canvasKey(SWT.ARROW_LEFT, SWT.NONE));
+ assertEquals(1, graph.left, "Bare arrow key must reach the inherited
canvas pan method");
+
+ keyHandler.keyPressed(canvasKey(SWT.ARROW_LEFT, SWT.SHIFT));
+ assertEquals(1, graph.leftFast, "Shift+arrow must reach the inherited
large step method");
+ assertEquals(1, graph.left, "Shift+arrow must not also trigger the small
step");
+ } finally {
+ keyHandler.removeParentObjectToHandle(graph);
+ }
+ }
+
+ @Test
+ void arrowKeysAreLeftToTextWidgets() {
+ NavigationGraph graph = new NavigationGraph();
+ registerShortcutsLikeHopGuiEnvironment(NavigationGraph.class);
+
+ HopGuiKeyHandler keyHandler = HopGuiKeyHandler.getInstance();
+ keyHandler.addParentObjectToHandle(graph);
+ try {
+ KeyEvent event = keyEvent(mock(Text.class), SWT.ARROW_LEFT, SWT.NONE);
+ keyHandler.keyPressed(event);
+ assertEquals(0, graph.left, "Arrow keys must stay in text widgets for
caret movement");
+ } finally {
+ keyHandler.removeParentObjectToHandle(graph);
+ }
+ }
+
+ @Test
+ void arrowKeysAreLeftToTablesAndTrees() {
+ NavigationGraph graph = new NavigationGraph();
+ registerShortcutsLikeHopGuiEnvironment(NavigationGraph.class);
+
+ HopGuiKeyHandler keyHandler = HopGuiKeyHandler.getInstance();
+ keyHandler.addParentObjectToHandle(graph);
+ try {
+ // Tables and trees live inside the graph as well: log, preview and
result tabs
+ keyHandler.keyPressed(keyEvent(mock(Table.class), SWT.ARROW_LEFT,
SWT.NONE));
+ keyHandler.keyPressed(keyEvent(mock(Tree.class), SWT.ARROW_LEFT,
SWT.NONE));
+ keyHandler.keyPressed(keyEvent(mock(Table.class), SWT.ARROW_LEFT,
SWT.SHIFT));
+ assertEquals(0, graph.left, "Arrow keys must stay in tables and trees to
move through rows");
+ assertEquals(0, graph.leftFast, "Shift+arrow must stay in tables to
extend the selection");
+ } finally {
+ keyHandler.removeParentObjectToHandle(graph);
+ }
+ }
+
+ @Test
+ void attachesToWidgetsCreatedAfterTheShellWasHandled() {
+ // A metadata editor that rebuilds a section (e.g. the OAuth 2 fields of a
REST connection)
+ // creates widgets that never got the key listener, so Ctrl+S did nothing
while they had focus.
+ HopGuiKeyHandler keyHandler = HopGuiKeyHandler.getInstance();
+ Shell shell = mock(Shell.class);
+ Text lateCreatedWidget = mock(Text.class);
+ when(lateCreatedWidget.getShell()).thenReturn(shell);
+
+ assertFalse(
+ keyHandler.attachTo(lateCreatedWidget),
+ "Widgets of a shell we don't handle must be left alone");
+
+ keyHandler.addHandledShell(null, shell);
+
+ assertTrue(keyHandler.attachTo(lateCreatedWidget));
+ verify(lateCreatedWidget).addKeyListener(keyHandler);
+ }
+
+ @Test
+ void doesNotAttachToWidgetsThatPassKeyListenersOn() {
+ // TextVar styles itself before creating the Text inside it and passes key
listeners on to that
+ // Text, so attaching while it is being built threw a
NullPointerException. The Text inside it
+ // is styled separately, which is where it gets the handler.
+ HopGuiKeyHandler keyHandler = HopGuiKeyHandler.getInstance();
+ Shell shell = mock(Shell.class);
+ TextVar textVar = mock(TextVar.class);
+ when(textVar.getShell()).thenReturn(shell);
+ keyHandler.addHandledShell(null, shell);
+
+ assertFalse(keyHandler.attachTo(textVar));
+ verify(textVar, never()).addKeyListener(keyHandler);
+ }
+
+ @Test
+ void doesNotAttachToTheTerminalWidget() {
+ HopGuiKeyHandler keyHandler = HopGuiKeyHandler.getInstance();
+ Shell shell = mock(Shell.class);
+ Text terminalWidget = mock(Text.class);
+ when(terminalWidget.getShell()).thenReturn(shell);
+
when(terminalWidget.getData(HopGuiKeyHandler.HOP_TERMINAL_WIDGET)).thenReturn(Boolean.TRUE);
+ keyHandler.addHandledShell(null, shell);
+
+ assertFalse(keyHandler.attachTo(terminalWidget));
+ verify(terminalWidget, never()).addKeyListener(keyHandler);
+ }
+
+ @Test
+ void pipelineGraphBindsArrowKeys() {
+ assertGraphBindsArrowKeys(HopGuiPipelineGraph.class);
+ }
+
+ @Test
+ void workflowGraphBindsArrowKeys() {
+ assertGraphBindsArrowKeys(HopGuiWorkflowGraph.class);
+ }
+
+ private void assertGraphBindsArrowKeys(Class<?> graphClass) {
+ registerShortcutsLikeHopGuiEnvironment(graphClass);
+ List<KeyboardShortcut> shortcuts =
+ GuiRegistry.getInstance().getKeyboardShortcuts(graphClass.getName());
+ assertNotNull(shortcuts, "No keyboard shortcuts registered for " +
graphClass.getName());
+
+ for (int keyCode : new int[] {SWT.ARROW_LEFT, SWT.ARROW_RIGHT,
SWT.ARROW_UP, SWT.ARROW_DOWN}) {
+ for (boolean shift : new boolean[] {false, true}) {
+ KeyboardShortcut shortcut = findArrowShortcut(shortcuts, keyCode,
shift);
+ assertNotNull(
+ shortcut, "No " + (shift ? "shift+" : "") + "arrow key shortcut on
" + graphClass);
+ // This is what HopGuiKeyHandler checks before invoking the method
+ assertEquals(
+ graphClass.getName(),
+ shortcut.getParentClassName(),
+ "Shortcut must be linked to the graph class, not to the class
declaring the method");
+ try {
+ assertNotNull(graphClass.getMethod(shortcut.getParentMethodName()));
+ } catch (NoSuchMethodException e) {
+ throw new AssertionError("Method not callable on " + graphClass, e);
+ }
+ }
+ }
+ }
+
+ private KeyboardShortcut findArrowShortcut(
+ List<KeyboardShortcut> shortcuts, int keyCode, boolean shift) {
+ for (KeyboardShortcut shortcut : shortcuts) {
+ if (shortcut.getKeyCode() == keyCode
+ && shortcut.isShift() == shift
+ && !shortcut.isAlt()
+ && !shortcut.isControl()
+ && !shortcut.isCommand()) {
+ return shortcut;
+ }
+ }
+ return null;
+ }
+
+ /** Register the shortcuts of a class the same way HopGuiEnvironment does at
startup. */
+ private void registerShortcutsLikeHopGuiEnvironment(Class<?> guiPluginClass)
{
+ GuiRegistry registry = GuiRegistry.getInstance();
+ if (registry.getKeyboardShortcuts(guiPluginClass.getName()) != null) {
+ return; // The registry is a singleton: only register once per test run
+ }
+ for (Method method : findDeclaredMethods(guiPluginClass)) {
+ GuiKeyboardShortcut shortcut =
method.getAnnotation(GuiKeyboardShortcut.class);
+ if (shortcut != null) {
+ registry.addKeyboardShortcut(guiPluginClass.getName(), method,
shortcut);
+ }
+ GuiOsxKeyboardShortcut osxShortcut =
method.getAnnotation(GuiOsxKeyboardShortcut.class);
+ if (osxShortcut != null) {
+ registry.addKeyboardShortcut(guiPluginClass.getName(), method,
osxShortcut);
+ }
+ }
+ }
+
+ private List<Method> findDeclaredMethods(Class<?> parentClass) {
+ List<Method> methods = new ArrayList<>();
+ Class<?> current = parentClass;
+ while (current != null) {
+ methods.addAll(List.of(current.getDeclaredMethods()));
+ current = current.getSuperclass();
+ }
+ return methods;
+ }
+
+ private KeyEvent canvasKey(int keyCode, int stateMask) {
+ return keyEvent(mock(Canvas.class), keyCode, stateMask);
+ }
+
+ private KeyEvent keyEvent(org.eclipse.swt.widgets.Widget widget, int
keyCode, int stateMask) {
+ Event event = new Event();
+ event.widget = widget;
+ event.keyCode = keyCode;
+ event.stateMask = stateMask;
+ event.doit = true;
+ return new KeyEvent(event);
+ }
+}
