Changeset 18456 in josm

Timestamp:

2022-05-25T23:09:41+02:00 (2 years ago)

Author:

taylor.smock

Message:

Fix #22065: Mac users cannot deselect with ctrl

From the Mac OS HIG guidelines, "[as] much as possible,
avoid using the Control key as a modifier."

In this case, we were using ctrl to deselect elements (and
we disabled this on Mac), merge nodes, scale, and rotate.

All of those now use ⌘ (meta) instead of ctrl on Mac.

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/mapmode/MapMode.java (modified) (4 diffs)
• actions/mapmode/SelectAction.java (modified) (17 diffs)
• actions/mapmode/SelectLassoAction.java (modified) (1 diff)
• gui/SelectionManager.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/actions/mapmode/MapMode.java

@@ -21 +21 @@
 import org.openstreetmap.josm.tools.ImageProvider;
 import org.openstreetmap.josm.tools.Logging;
+import org.openstreetmap.josm.tools.PlatformHook;
+import org.openstreetmap.josm.tools.PlatformManager;
 import org.openstreetmap.josm.tools.Shortcut;
 
@@ -34 +36 @@
     protected boolean alt;
     protected boolean shift;
+    /**
+     * {@code true} if the meta key was pressed (the "Windows" key or the Mac "Command" key)
+     * @since xxx
+     */
+    protected boolean meta;
+    /**
+     * {@code true} if the platform specific menu key was pressed ("ctrl" on Linux/Windows, "cmd" on Mac)
+     * @see PlatformHook#getMenuShortcutKeyMaskEx()
+     * @since xxx
+     */
+    protected boolean platformMenuShortcutKeyMask;
 
     /**
@@ -155 +168 @@
 
     /**
-     * Update internal ctrl, alt, shift mask from given extended modifiers mask.
+     * Update internal ctrl, alt, shift, meta mask from given extended modifiers mask.
      * @param modifiers event extended modifiers mask
      * @since 12517
@@ -163 +176 @@
         alt = (modifiers & (InputEvent.ALT_DOWN_MASK | InputEvent.ALT_GRAPH_DOWN_MASK)) != 0;
         shift = (modifiers & InputEvent.SHIFT_DOWN_MASK) != 0;
+        meta = (modifiers & InputEvent.META_DOWN_MASK) != 0;
+        platformMenuShortcutKeyMask = (modifiers & PlatformManager.getPlatform().getMenuShortcutKeyMaskEx()) != 0;
     }
 

trunk/src/org/openstreetmap/josm/actions/mapmode/SelectAction.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
 import static org.openstreetmap.josm.tools.I18n.tr;
+import static org.openstreetmap.josm.tools.I18n.trc;
 import static org.openstreetmap.josm.tools.I18n.trn;
 
@@ -9 +10 @@
 import java.awt.Point;
 import java.awt.Rectangle;
+import java.awt.event.InputEvent;
 import java.awt.event.KeyEvent;
 import java.awt.event.MouseEvent;
@@ -288 +290 @@
 
         // CTRL toggles selection, but if while dragging CTRL means merge
-        final boolean isToggleMode = ctrl && !dragInProgress();
+        final boolean isToggleMode = platformMenuShortcutKeyMask && !dragInProgress();
         if (c.isPresent() && (isToggleMode || !c.get().isSelected())) {
             // only highlight primitives that will change the selection
@@ -318 +320 @@
                 // only consider merge if ctrl is pressed and there are nodes in
                 // the selection that could be merged
-                if (!ctrl || getLayerManager().getEditDataSet().getSelectedNodes().isEmpty()) {
+                if (!platformMenuShortcutKeyMask || getLayerManager().getEditDataSet().getSelectedNodes().isEmpty()) {
                     c = "move";
                     break;
@@ -333 +335 @@
             if (shift) {
                 c += "_add";
-            } else if (ctrl) {
+            } else if (platformMenuShortcutKeyMask) {
                 c += osm == null || osm.isSelected() ? "_rm" : "_add";
             }
@@ -346 +348 @@
             if (lassoMode) {
                 c = "lasso";
+            } else if (shift) {
+                c = "rect_add";
+            } else if (platformMenuShortcutKeyMask) {
+                c = "rect_rm";
             } else {
-                c = "rect" + (shift ? "_add" : (ctrl && !PlatformManager.isPlatformOsx() ? "_rm" : ""));
+                c = "rect";
             }
             break;
@@ -409 +415 @@
         // We don't want to change to draw tool if the user tries to (de)select
         // stuff but accidentally clicks in an empty area when selection is empty
-        cancelDrawMode = shift || ctrl;
+        cancelDrawMode = shift || platformMenuShortcutKeyMask;
         didMouseDrag = false;
         initialMoveThresholdExceeded = false;
@@ -514 +520 @@
             // If ctrl is pressed we are in merge mode. Look for a nearby node,
             // highlight it and adjust the cursor accordingly.
-            final boolean canMerge = ctrl && !getLayerManager().getEditDataSet().getSelectedNodes().isEmpty();
+            final boolean canMerge = platformMenuShortcutKeyMask && !getLayerManager().getEditDataSet().getSelectedNodes().isEmpty();
             final OsmPrimitive p = canMerge ? findNodeToMergeTo(e.getPoint()) : null;
             boolean needsRepaint = removeHighlighting();
@@ -670 +676 @@
     private void determineMapMode(boolean hasSelectionNearby) {
         if (getLayerManager().getEditDataSet() != null) {
-            if (shift && ctrl) {
+            if (shift && platformMenuShortcutKeyMask) {
                 mode = Mode.ROTATE;
-            } else if (alt && ctrl) {
+            } else if (alt && platformMenuShortcutKeyMask) {
                 mode = Mode.SCALE;
             } else if (hasSelectionNearby || dragInProgress()) {
@@ -861 +867 @@
             // if small number of elements were moved,
             updateKeyModifiers(e);
-            if (ctrl) mergePrims(e.getPoint());
+            if (platformMenuShortcutKeyMask) mergePrims(e.getPoint());
         }
     }
@@ -979 +985 @@
         // anything if about to drag the virtual node (i.e. !released) but continue if the
         // cursor is only released above a virtual node by accident (i.e. released). See #7018
-        if (ds == null || (shift && ctrl) || (ctrl && !released) || (virtualManager.hasVirtualWaysToBeConstructed() && !released))
+        if (ds == null || (shift && platformMenuShortcutKeyMask) || (platformMenuShortcutKeyMask && !released)
+                || (virtualManager.hasVirtualWaysToBeConstructed() && !released))
             return;
 
@@ -989 +996 @@
         }
 
-        if (ctrl) {
+        if (platformMenuShortcutKeyMask) {
             // Ctrl on an item toggles its selection status,
             // but Ctrl on an *area* just clears those items
@@ -1018 +1025 @@
     @Override
     public String getModeHelpText() {
+        // There needs to be a better way
+        final String menuKey;
+        switch (PlatformManager.getPlatform().getMenuShortcutKeyMaskEx()) {
+        case InputEvent.CTRL_DOWN_MASK:
+            menuKey = trc("SelectAction help", "Ctrl");
+            break;
+        case InputEvent.META_DOWN_MASK:
+            menuKey = trc("SelectAction help", "Meta");
+            break;
+        default:
+            throw new IllegalStateException("Unknown platform menu shortcut key for " + PlatformManager.getPlatform().getOSDescription());
+        }
+        final String type = this.lassoMode ? trc("SelectAction help", "lasso") : trc("SelectAction help", "rectangle");
         if (mouseDownButton == MouseEvent.BUTTON1 && mouseReleaseTime < mouseDownTime) {
             if (mode == Mode.SELECT)
-                return tr("Release the mouse button to select the objects in the rectangle.");
+                return tr("Release the mouse button to select the objects in the {0}.", type);
             else if (mode == Mode.MOVE && (System.currentTimeMillis() - mouseDownTime >= initialMoveDelay)) {
                 final DataSet ds = getLayerManager().getEditDataSet();
                 final boolean canMerge = ds != null && !ds.getSelectedNodes().isEmpty();
-                final String mergeHelp = canMerge ? (' ' + tr("Ctrl to merge with nearest node.")) : "";
+                final String mergeHelp = canMerge ? (' ' + tr("{0} to merge with nearest node.", menuKey)) : "";
                 return tr("Release the mouse button to stop moving.") + mergeHelp;
             } else if (mode == Mode.ROTATE)
@@ -1031 +1051 @@
                 return tr("Release the mouse button to stop scaling.");
         }
-        return tr("Move objects by dragging; Shift to add to selection (Ctrl to toggle); Shift-Ctrl to rotate selected; " +
-                  "Alt-Ctrl to scale selected; or change selection");
+        return tr("Move objects by dragging; Shift to add to selection ({0} to toggle); Shift-{0} to rotate selected; " +
+                "Alt-{0} to scale selected; or change selection", menuKey);
     }
 
@@ -1107 +1127 @@
                         // true nearest primitive on mousePressed right away
                         if (cycleList.size() == 2 && !waitForMouseUpParameter) {
-                            if (!(osm.equals(old) || osm.isNew() || ctrl)) {
+                            if (!(osm.equals(old) || osm.isNew() || platformMenuShortcutKeyMask)) {
                                 cyclePrims = false;
                                 osm = old;
@@ -1155 +1175 @@
                         foundInDS = nxt;
                         // first selected primitive in cycleList is found
-                        if (cyclePrims || ctrl) {
+                        if (cyclePrims || platformMenuShortcutKeyMask) {
                             ds.clearSelection(foundInDS); // deselect it
                             nxt = i.hasNext() ? i.next() : first;
@@ -1166 +1186 @@
 
             // if "no-alt-cycling" is enabled, Ctrl-Click arrives here.
-            if (ctrl) {
+            if (platformMenuShortcutKeyMask) {
                 // a member of cycleList was found in the current dataset selection
                 if (foundInDS != null) {

trunk/src/org/openstreetmap/josm/actions/mapmode/SelectLassoAction.java

@@ -55 +55 @@
         return MainApplication.getMap().mapModeSelect.layerIsSupported(l);
     }
+
+    @Override
+    public String getModeHelpText() {
+        return MainApplication.getMap().mapModeSelect.getModeHelpText();
+    }
 }

trunk/src/org/openstreetmap/josm/gui/SelectionManager.java

@@ -27 +27 @@
 import org.openstreetmap.josm.gui.layer.AbstractMapViewPaintable;
 import org.openstreetmap.josm.tools.ColorHelper;
+import org.openstreetmap.josm.tools.PlatformManager;
 
 /**
@@ -186 +187 @@
             SelectByInternalPointAction.performSelection(MainApplication.getMap().mapView.getEastNorth(e.getX(), e.getY()),
                     (e.getModifiersEx() & MouseEvent.SHIFT_DOWN_MASK) != 0,
-                    (e.getModifiersEx() & MouseEvent.CTRL_DOWN_MASK) != 0);
+                    (e.getModifiersEx() & PlatformManager.getPlatform().getMenuShortcutKeyMaskEx()) != 0);
         } else if (e.getButton() == MouseEvent.BUTTON1) {
             mousePosStart = mousePos = e.getPoint();