Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Commit

Permalink
Fix for issue #8142: "Ctrl + Space" should move to the next entry whe…
Browse files Browse the repository at this point in the history 
…n the code hint menu is open (#12251)

* 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.
  • Loading branch information
@bmax @petetnt
bmax authored and petetnt committed Aug 26, 2016
1 parent 8cce5ec commit 63962ce
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 39 deletions.
36 changes: 26 additions & 10 deletions src/editor/CodeHintList.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
};

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
}
};
Expand All @@ -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
*/
Expand Down
78 changes: 49 additions & 29 deletions src/editor/CodeHintManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down Expand Up @@ -376,6 +377,7 @@ define(function (require, exports, module) {
}
hintList.close();
hintList = null;
codeHintOpened = false;
keyDownEditor = null;
sessionProvider = null;
sessionEditor = null;
Expand Down Expand Up @@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -466,6 +475,7 @@ define(function (require, exports, module) {
* @param {Editor} editor
*/
_beginSession = function (editor) {

if (!codeHintsEnabled) {
return;
}
Expand Down Expand Up @@ -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),
Expand All @@ -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.
Expand Down Expand Up @@ -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 ||
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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);

Expand Down
82 changes: 82 additions & 0 deletions test/spec/CodeHint-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 63962ce

Please sign in to comment.