Improve navigation through items using up/down shortcuts

1. Revert to native macOS navigation through items with up/down arrows.
2. Revert to native macOS navigation through items with Ctrl+N/Ctrl+P.
3. Simulate native macOS navigation through items with Ctrl+J/Ctrl+K.

As always, there are quirks and edge-cases with NSMenu that had to be
worked around, notably:

1. Header menu item should be hidden even though it is still shown and
   can be manipulated with. Though not in tests.
2. Separator should be force shown whenever it's the first item.

Fixes #442

Maccy/FilterMenuItemView.swift

@@ -237,27 +237,26 @@ class FilterMenuItemView: NSView, NSTextFieldDelegate {
         removeLastWordInSearchField()
         return true
       }
-    case Key.j, Key.n:
-      if modifierFlags.contains(.control) {
-        customMenu?.selectNext(alt: false)
+    case Key.j:
+      if modifierFlags == .control {
+        customMenu?.selectNext()
+        return true
+      }
+    case Key.k:
+      if modifierFlags == .control {
+        customMenu?.selectPrevious()
         return true
       }
     case Key.p:
       if modifierFlags.contains(.option) {
         customMenu?.pinOrUnpin()
         queryField.stringValue = "" // clear search field just in case
         return true
-      } else if modifierFlags.contains(.control) {
-        customMenu?.selectPrevious(alt: false)
-        return true
-      }
-    case Key.k:
-      if modifierFlags.contains(.control) {
-        customMenu?.selectPrevious(alt: false)
-        return true
+      } else {
+        return false
       }
-    case Key.return, Key.keypadEnter, Key.upArrow, Key.downArrow:
-      processSelectionKey(menu: customMenu, key: key, modifierFlags: modifierFlags)
+    case Key.return, Key.keypadEnter:
+      customMenu?.select()
       return true
     case GlobalHotKey.key:
       if modifierFlags == GlobalHotKey.modifierFlags {
@@ -327,26 +326,6 @@ class FilterMenuItemView: NSView, NSTextFieldDelegate {
     }
   }
 
-  private func processSelectionKey(menu: Menu?, key: Key, modifierFlags: NSEvent.ModifierFlags) {
-    switch key {
-    case .return, .keypadEnter:
-      menu?.select()
-    case .upArrow:
-      if modifierFlags.contains(.command) {
-        menu?.selectFirst()
-      } else {
-        menu?.selectPrevious(alt: modifierFlags.contains(.option))
-      }
-    case .downArrow:
-      if modifierFlags.contains(.command) {
-        menu?.selectLast()
-      } else {
-        menu?.selectNext(alt: modifierFlags.contains(.option))
-      }
-    default: ()
-    }
-  }
-
   private func focusQueryField() {
     queryField.becomeFirstResponder()
     // Making text field a first responder selects all the text by default.

Maccy/Keys.swift

@@ -6,8 +6,10 @@ class Keys {
     Key.pageUp,
     Key.pageDown,
     Key.end,
+    Key.downArrow,
     Key.leftArrow,
     Key.rightArrow,
+    Key.upArrow,
     Key.escape,
     Key.tab,
     Key.f1,

Maccy/Maccy.swift

@@ -160,6 +160,9 @@ class Maccy: NSObject {
     headerItem.title = "Maccy"
     headerItem.view = headerItemView
     headerItem.isEnabled = false
+    // HACK: Enable location of text field in UI tests, but make sure
+    //       it's excldued from up/down arrow selection in real usage.
+    headerItem.isHidden = !ProcessInfo.processInfo.arguments.contains("ui-testing")
 
     menu.addItem(headerItem)
   }

Maccy/Menu.swift

@@ -175,6 +175,7 @@ class Menu: NSMenu, NSMenuDelegate {
 
     setKeyEquivalents(historyMenuItems)
     highlight(filter.isEmpty ? firstUnpinnedHistoryMenuItem : historyMenuItems.first)
+    forceSeparatorShown()
   }
 
   func select() {
@@ -184,26 +185,18 @@ class Menu: NSMenu, NSMenuDelegate {
     }
   }
 
-  func selectPrevious(alt: Bool) {
-    if !highlightNext(items.reversed(), alt: alt) {
-      selectLast(alt: alt) // start from the end after reaching the first item
+  func selectPrevious() {
+    if !highlightNext(items.reversed()) {
+      highlight(highlightableItems(items).last) // start from the end after reaching the first item
     }
   }
 
-  func selectNext(alt: Bool) {
-    if !highlightNext(items, alt: alt) {
-      selectFirst(alt: alt) // start from the beginning after reaching the last item
+  func selectNext() {
+    if !highlightNext(items) {
+      highlight(highlightableItems(items).first) // start from the beginning after reaching the last item
     }
   }
 
-  func selectFirst(alt: Bool = false) {
-    highlight(highlightableItems(items, alt: alt).first)
-  }
-
-  func selectLast(alt: Bool = false) {
-    highlight(highlightableItems(items, alt: alt).last)
-  }
-
   func delete() {
     guard let itemToRemove = highlightedItem else {
       return
@@ -279,8 +272,8 @@ class Menu: NSMenu, NSMenuDelegate {
     }
   }
 
-  private func highlightNext(_ items: [NSMenuItem], alt: Bool) -> Bool {
-    let highlightableItems = self.highlightableItems(items, alt: alt)
+  private func highlightNext(_ items: [NSMenuItem]) -> Bool {
+    let highlightableItems = self.highlightableItems(items)
     let currentHighlightedItem = highlightedItem ?? highlightableItems.first
     var itemsIterator = highlightableItems.makeIterator()
     while let item = itemsIterator.next() {
@@ -294,8 +287,8 @@ class Menu: NSMenu, NSMenuDelegate {
     return false
   }
 
-  private func highlightableItems(_ items: [NSMenuItem], alt: Bool = false) -> [NSMenuItem] {
-    return items.filter { !$0.isSeparatorItem && $0.isEnabled && $0.isAlternate == alt }
+  private func highlightableItems(_ items: [NSMenuItem]) -> [NSMenuItem] {
+    return items.filter { !$0.isSeparatorItem && $0.isEnabled && !$0.isHidden }
   }
 
   private func highlight(_ itemToHighlight: NSMenuItem?) {
@@ -420,6 +413,22 @@ class Menu: NSMenu, NSMenuDelegate {
       }
     }
   }
+
+  // When separator is the first visible item in the menu - it is automatically hidden.
+  // This can happen when for example search results are empty.
+  // Later, when separator is no longer the first item - it is still not automatically shown.
+  private func forceSeparatorShown() {
+    guard !UserDefaults.standard.hideFooter else {
+      return
+    }
+
+    guard let separatorItem = items.first(where: { $0.isSeparatorItem }) else {
+      return
+    }
+
+    separatorItem.isHidden = true
+    separatorItem.isHidden = false
+  }
 }
 // swiftlint:enable type_body_length
 // swiftlint:enable file_length

Maccy/MenuFooter.swift

@@ -11,7 +11,7 @@ enum MenuFooter: Int, CaseIterable {
   var menuItem: NSMenuItem {
     let item = self == .separator ? NSMenuItem.separator() : NSMenuItem()
     item.isAlternate = isAlternate && !UserDefaults.standard.hideFooter
-    item.isHidden = UserDefaults.standard.hideFooter
+    item.isHidden = isAlternate || UserDefaults.standard.hideFooter
     item.keyEquivalent = keyEquivalent
     item.keyEquivalentModifierMask = keyEquivalentModifierMask
     item.tag = rawValue

MaccyUITests/MaccyUITests.swift

@@ -1,4 +1,3 @@
-// swiftlint:disable file_length
 import Carbon
 import XCTest
 
@@ -127,64 +126,12 @@ class MaccyUITests: XCTestCase {
     XCTAssertEqual(pasteboard.string(forType: .fileURL), file2.absoluteString)
   }
 
-  func testDownArrow() {
-    popUpWithHotkey()
-    app.typeKey(.downArrow, modifierFlags: [])
-    XCTAssertTrue(app.menuItems[copy2].firstMatch.isSelected)
-  }
-
-  func testCyclingWithDownArrow() {
-    popUpWithHotkey()
-    app.typeKey(.upArrow, modifierFlags: [])
-    app.typeKey(.downArrow, modifierFlags: [])
-    XCTAssertTrue(app.menuItems[copy1].firstMatch.isSelected)
-  }
-
-  func testCommandDownArrow() {
-    popUpWithHotkey()
-    app.typeKey(.downArrow, modifierFlags: [.command])
-    XCTAssertTrue(app.menuItems["Quit"].firstMatch.isSelected)
-  }
-
-  func testControlN() {
-    popUpWithHotkey()
-    app.typeKey("n", modifierFlags: [.control])
-    XCTAssertTrue(app.menuItems[copy2].firstMatch.isSelected)
-  }
-
   func testControlJ() {
     popUpWithHotkey()
     app.typeKey("j", modifierFlags: [.control])
     XCTAssertTrue(app.menuItems[copy2].firstMatch.isSelected)
   }
 
-  func testUpArrow() {
-    popUpWithHotkey()
-    app.typeKey(.downArrow, modifierFlags: [])
-    app.typeKey(.upArrow, modifierFlags: [])
-    XCTAssertTrue(app.menuItems[copy1].firstMatch.isSelected)
-  }
-
-  func testCyclingWithUpArrow() {
-    popUpWithHotkey()
-    app.typeKey(.upArrow, modifierFlags: [])
-    XCTAssertTrue(app.menuItems["Quit"].firstMatch.isSelected)
-  }
-
-  func testCommandUpArrow() {
-    popUpWithHotkey()
-    app.typeKey(.upArrow, modifierFlags: []) // "Quit"
-    app.typeKey(.upArrow, modifierFlags: [.command])
-    XCTAssertTrue(app.menuItems[copy1].firstMatch.isSelected)
-  }
-
-  func testControlP() {
-    popUpWithHotkey()
-    app.typeKey(.downArrow, modifierFlags: [])
-    app.typeKey("p", modifierFlags: [.control])
-    XCTAssertTrue(app.menuItems[copy1].firstMatch.isSelected)
-  }
-
   func testControlK() {
     popUpWithHotkey()
     app.typeKey(.downArrow, modifierFlags: [])
@@ -449,4 +396,3 @@ class MaccyUITests: XCTestCase {
   }
 }
 // swiftlint:enable type_body_length
-// swiftlint:enable file_length