Fix for issue #8142: "Ctrl + Space" should move to the next entry when the code hint menu is open (#12251)

bmax · petetnt · commit 63962ceec71d · 2016-08-26T10:23:13.000+03:00
* Can now move up with ctrl+space.

* Changes made suggested by PR

* Found a cleaner way to do the alreadyOpen Bool. Still having trouble with the javascript '.' character hints

* Remove unneeded var

* Fix issues with CodeHints starting with dots

* Fix wrong parameter

* Changes made suggested by PR

* Fix issues with CodeHints starting with dots

* Fix issues with CodeHints starting with dots

* Fix issues with CodeHints starting with dots

* Remove unneeded var

* All tests pass.
diff --git a/src/editor/CodeHintList.js b/src/editor/CodeHintList.js
@@ -304,14 +304,21 @@ define(function (require, exports, module) {
     };
 
     /**
-     * Check whether keyCode is one of the keys that we handle or not.
+     * Check whether Event is one of the keys that we handle or not.
      *
-     * @param {number} keyCode
+     * @param {KeyBoardEvent|keyBoardEvent.keyCode} keyEvent
      */
-    CodeHintList.prototype.isHandlingKeyCode = function (keyCode) {
+    CodeHintList.prototype.isHandlingKeyCode = function (keyCodeOrEvent) {
+        var keyCode = typeof keyCodeOrEvent === "object" ? keyCodeOrEvent.keyCode : keyCodeOrEvent;
+        var ctrlKey = typeof keyCodeOrEvent === "object" ? keyCodeOrEvent.ctrlKey : false;
+
+
         return (keyCode === KeyEvent.DOM_VK_UP || keyCode === KeyEvent.DOM_VK_DOWN ||
                 keyCode === KeyEvent.DOM_VK_PAGE_UP || keyCode === KeyEvent.DOM_VK_PAGE_DOWN ||
                 keyCode === KeyEvent.DOM_VK_RETURN ||
+                keyCode === KeyEvent.DOM_VK_CONTROL ||
+                keyCode === KeyEvent.DOM_VK_ESCAPE ||
+                (ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE) ||
                 (keyCode === KeyEvent.DOM_VK_TAB && this.insertHintOnTab));
     };
 
@@ -382,21 +389,23 @@ define(function (require, exports, module) {
         }
 
         // (page) up, (page) down, enter and tab key are handled by the list
-        if (event.type === "keydown" && this.isHandlingKeyCode(event.keyCode)) {
+        if (event.type === "keydown" && this.isHandlingKeyCode(event)) {
             keyCode = event.keyCode;
 
-            if (event.shiftKey &&
+            if (event.keyCode === KeyEvent.DOM_VK_ESCAPE || 
+                  (event.shiftKey &&
                     (event.keyCode === KeyEvent.DOM_VK_UP ||
                      event.keyCode === KeyEvent.DOM_VK_DOWN ||
                      event.keyCode === KeyEvent.DOM_VK_PAGE_UP ||
-                     event.keyCode === KeyEvent.DOM_VK_PAGE_DOWN)) {
+                     event.keyCode === KeyEvent.DOM_VK_PAGE_DOWN))) {
                 this.handleClose();
 
                 // Let the event bubble.
                 return false;
             } else if (keyCode === KeyEvent.DOM_VK_UP) {
                 _rotateSelection.call(this, -1);
-            } else if (keyCode === KeyEvent.DOM_VK_DOWN) {
+            } else if (keyCode === KeyEvent.DOM_VK_DOWN ||
+                    (event.ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE)) {
                 _rotateSelection.call(this, 1);
             } else if (keyCode === KeyEvent.DOM_VK_PAGE_UP) {
                 _rotateSelection.call(this, -_itemsPerPage());
@@ -478,8 +487,6 @@ define(function (require, exports, module) {
                 .css({"left": hintPos.left, "top": hintPos.top, "width": hintPos.width + "px"});
             this.opened = true;
 
-            PopUpManager.addPopUp(this.$hintMenu, this.handleClose, true);
-
             KeyBindingManager.addGlobalKeydownHook(this._keydownHook);
         }
     };
@@ -501,7 +508,16 @@ define(function (require, exports, module) {
                                 "width": hintPos.width + "px"});
         }
     };
-
+    /**
+     * Calls the move up keybind to move hint suggestion selector
+     *
+     * @param {KeyBoardEvent} keyEvent
+     */
+    CodeHintList.prototype.callMoveUp = function (event) {
+        delete event.type;
+        event.type = "keydown";
+        this._keydownHook(event);
+    };
     /**
      * Closes the hint list
      */
diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js
@@ -246,7 +246,8 @@ define(function (require, exports, module) {
         hintList         = null,
         deferredHints    = null,
         keyDownEditor    = null,
-        codeHintsEnabled = true;
+        codeHintsEnabled = true,
+        codeHintOpened   = false;
 
 
     PreferencesManager.definePreference("showCodeHints", "boolean", true, {
@@ -376,6 +377,7 @@ define(function (require, exports, module) {
         }
         hintList.close();
         hintList = null;
+        codeHintOpened = false;
         keyDownEditor = null;
         sessionProvider = null;
         sessionEditor = null;
@@ -407,19 +409,25 @@ define(function (require, exports, module) {
         }
         return false;
     }
-
     /**
      * From an active hinting session, get hints from the current provider and
      * render the hint list window.
      *
      * Assumes that it is called when a session is active (i.e. sessionProvider is not null).
      */
-    function _updateHintList() {
+    function _updateHintList(callMoveUpEvent) {
+
+        callMoveUpEvent = typeof callMoveUpEvent === "undefined" ? false : callMoveUpEvent;
+
         if (deferredHints) {
             deferredHints.reject();
             deferredHints = null;
         }
 
+        if (callMoveUpEvent) {
+            return hintList.callMoveUp(callMoveUpEvent);
+        }
+
         var response = sessionProvider.getHints(lastChar);
         lastChar = null;
 
@@ -430,6 +438,7 @@ define(function (require, exports, module) {
             // if the response is true, end the session and begin another
             if (response === true) {
                 var previousEditor = sessionEditor;
+
                 _endSession();
                 _beginSession(previousEditor);
             } else if (response.hasOwnProperty("hints")) { // a synchronous response
@@ -466,6 +475,7 @@ define(function (require, exports, module) {
      * @param {Editor} editor
      */
     _beginSession = function (editor) {
+
         if (!codeHintsEnabled) {
             return;
         }
@@ -497,7 +507,6 @@ define(function (require, exports, module) {
             }
 
             sessionEditor = editor;
-
             hintList = new CodeHintList(sessionEditor, insertHintOnTab, maxCodeHints);
             hintList.onSelect(function (hint) {
                 var restart = sessionProvider.insertHint(hint),
@@ -515,26 +524,6 @@ define(function (require, exports, module) {
         }
     };
 
-    /**
-     * Explicitly start a new session. If we have an existing session,
-     * then close the current one and restart a new one.
-     * @param {Editor} editor
-     */
-    function _startNewSession(editor) {
-        if (!editor) {
-            editor = EditorManager.getFocusedEditor();
-        }
-
-        if (editor) {
-            lastChar = null;
-            if (_inSession(editor)) {
-                _endSession();
-            }
-            // Begin a new explicit session
-            _beginSession(editor);
-        }
-    }
-
     /**
      * Handles keys related to displaying, searching, and navigating the hint list.
      * This gets called before handleChange.
@@ -571,7 +560,8 @@ define(function (require, exports, module) {
     function _handleKeyupEvent(jqEvent, editor, event) {
         keyDownEditor = editor;
         if (_inSession(editor)) {
-            if (event.keyCode === KeyEvent.DOM_VK_HOME || event.keyCode === KeyEvent.DOM_VK_END) {
+          if (event.keyCode === KeyEvent.DOM_VK_HOME || 
+                  event.keyCode === KeyEvent.DOM_VK_END) {
                 _endSession();
             } else if (event.keyCode === KeyEvent.DOM_VK_LEFT ||
                        event.keyCode === KeyEvent.DOM_VK_RIGHT ||
@@ -580,6 +570,8 @@ define(function (require, exports, module) {
                 // We do this in "keyup" because we want the cursor position to be updated before
                 // we redraw the list.
                 _updateHintList();
+            } else if (event.ctrlKey && event.keyCode === KeyEvent.DOM_VK_SPACE) {
+                _updateHintList(event);
             }
         }
     }
@@ -664,6 +656,30 @@ define(function (require, exports, module) {
         return (hintList && hintList.isOpen());
     }
 
+    /**
+     * Explicitly start a new session. If we have an existing session,
+     * then close the current one and restart a new one.
+     * @param {Editor} editor
+     */
+    function _startNewSession(editor) {
+        if (isOpen()) {
+            return;
+        }
+
+        if (!editor) {
+            editor = EditorManager.getFocusedEditor();
+        }
+        if (editor) {
+            lastChar = null;
+            if (_inSession(editor)) {
+                _endSession();
+            }
+
+            // Begin a new explicit session
+            _beginSession(editor);
+        }
+    }
+
     /**
      * Expose CodeHintList for unit testing
      */
@@ -693,12 +709,16 @@ define(function (require, exports, module) {
     activeEditorChangeHandler(null, EditorManager.getActiveEditor(), null);
 
     EditorManager.on("activeEditorChange", activeEditorChangeHandler);
-
-    // Dismiss code hints before executing any command since the command
+ 
+    // Dismiss code hints before executing any command other than showing code hints since the command
     // may make the current hinting session irrevalent after execution.
     // For example, when the user hits Ctrl+K to open Quick Doc, it is
-    // pointless to keep the hint list since the user wants to view the Quick Doc.
-    CommandManager.on("beforeExecuteCommand", _endSession);
+    // pointless to keep the hint list since the user wants to view the Quick Doc
+    CommandManager.on("beforeExecuteCommand", function (event, commandId) {
+        if (commandId !== Commands.SHOW_CODE_HINTS) {
+            _endSession();
+        }
+    });
 
     CommandManager.register(Strings.CMD_SHOW_CODE_HINTS, Commands.SHOW_CODE_HINTS, _startNewSession);
 
diff --git a/test/spec/CodeHint-test.js b/test/spec/CodeHint-test.js
@@ -278,6 +278,88 @@ define(function (require, exports, module) {
                 });
             });
 
+            it("should go to next hint with ctrl+space", function () {
+                var editor,
+                    pos = {line: 3, ch: 1},
+                    hintBefore,
+                    hintAfter;
+
+                // minimal markup with an open '<' before IP
+                // Note: line for pos is 0-based and editor lines numbers are 1-based
+                initCodeHintTest("test1.html", pos);
+
+                // simulate ctrl+space key to make sure it goes to next hint
+                runs(function () {
+                    var e = $.Event("keydown");
+                    e.keyCode = KeyEvent.DOM_VK_SPACE;
+                    e.ctrlKey = true;
+
+                    editor = EditorManager.getCurrentFullEditor();
+                    expect(editor).toBeTruthy();
+
+                    invokeCodeHints();
+                    var codeHintList = expectSomeHints();
+                    hintBefore = codeHintList.selectedIndex;
+
+                    // make sure hint list starts at 0
+                    expect(hintBefore).toEqual(0);
+
+                    // simulate ctrl+space keyhook
+                    CodeHintManager._getCodeHintList()._keydownHook(e);
+                    hintAfter = codeHintList.selectedIndex;
+
+                    // selectedIndex should be one more after doing ctrl+space key event.
+                    expect(hintBefore).toEqual(hintAfter-1);
+
+                    editor = null;
+                });
+            });
+
+            it("should loop to first hint when ctrl+space at last hint", function () {
+                var editor,
+                    pos = {line: 3, ch: 1},
+                    hintBefore,
+                    hintAfter;
+
+                // minimal markup with an open '<' before IP
+                // Note: line for pos is 0-based and editor lines numbers are 1-based
+                initCodeHintTest("test1.html", pos);
+
+                // simulate ctrl+space key to make sure it goes to next hint
+                runs(function () {
+                    var e = $.Event("keydown");
+                    e.keyCode = KeyEvent.DOM_VK_UP;
+
+                    editor = EditorManager.getCurrentFullEditor();
+                    expect(editor).toBeTruthy();
+
+                    invokeCodeHints();
+
+                    // simulate up keyhook to send it to last hint
+                    CodeHintManager._getCodeHintList()._keydownHook(e);
+
+                    var codeHintList = expectSomeHints();
+                    hintBefore = codeHintList.selectedIndex;
+                    var numberOfHints = codeHintList.$hintMenu.find("li").length-1;
+
+                    // should be at last hint
+                    expect(hintBefore).toEqual(numberOfHints);
+
+                    // call ctrl+space to loop it to first hint
+                    e.keyCode = KeyEvent.DOM_VK_SPACE;
+                    e.ctrlKey = true;
+
+                    // simulate ctrl+space keyhook to send it to first hint
+                    CodeHintManager._getCodeHintList()._keydownHook(e);
+                    hintAfter = codeHintList.selectedIndex;
+
+                    // should now be at hint 0
+                    expect(hintAfter).toEqual(0);
+
+                    editor = null;
+                });
+            });
+
             it("should not show code hints if there is a multiple selection", function () {
                 // minimal markup with an open '<' before IP
                 // Note: line for pos is 0-based and editor lines numbers are 1-based
