Skip to content

Commit

Permalink
Pop up attribute value code hints immediately after committing attrib…
Browse files Browse the repository at this point in the history 
…ute name hint.

As part of this, made it so CodeHintList is no longer a singleton within
CodeHintManager--each new popup is a new CodeHintList. This is so we don't
have issues with order of operations, where something tries to close the
previous CodeHintList after a new one has popped up. Now a CodeHintList
will only try to close itself (and it's okay if it tries to do it more than
once).

Also included a partial fix for adobe#1381.
  • Loading branch information
@njx
njx committed Aug 17, 2012
1 parent 584bbca commit 7390867
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 30 deletions.
54 changes: 30 additions & 24 deletions src/editor/CodeHintManager.js
View file
Expand Up @@ -37,7 +37,8 @@ define(function (require, exports, module) {
ViewUtils = require("utils/ViewUtils");


var hintList, // initialized by htmlContentLoadComplete handler
var hintProviders = [],
hintList, // initialized by htmlContentLoadComplete handler
shouldShowHintsOnKeyUp = false;


Expand All @@ -49,7 +50,6 @@ define(function (require, exports, module) {
* to include: extensibility API, HTML attributes hints, JavaScript hints, CSS hints
*/
function CodeHintList() {
this.hintProviders = [];
this.currentProvider = null;
this.query = {queryStr: null};
this.displayList = [];
Expand All @@ -67,9 +67,6 @@ define(function (require, exports, module) {

this.$hintMenu.append($toggle)
.append("<ul class='dropdown-menu'></ul>");

$("#codehint-menu-bar > ul").append(this.$hintMenu);

}

/**
Expand All @@ -79,6 +76,7 @@ define(function (require, exports, module) {
*/
CodeHintList.prototype._handleItemClick = function (completion) {
this.currentProvider.handleSelect(completion, this.editor, this.editor.getCursorPos());
this.close();
};

/**
Expand All @@ -93,7 +91,10 @@ define(function (require, exports, module) {
);

var $item = $("<li><a href='#'><span class='codehint-item'>" + displayName + "</span></a></li>")
.on("click", function () {
.on("click", function (e) {
// Don't let the click propagate upward (otherwise it will hit the close handler in
// bootstrap-dropdown).
e.stopPropagation();
self._handleItemClick(name);
});

Expand Down Expand Up @@ -202,9 +203,6 @@ define(function (require, exports, module) {
// Enter/return key
// Trigger a click handler to commmit the selected item
$(this.$hintMenu.find("li")[this.selectedIndex]).triggerHandler("click");

// Close the list
this.close();
}
}

Expand Down Expand Up @@ -235,7 +233,7 @@ define(function (require, exports, module) {
CodeHintList.prototype.isOpen = function () {
// We don't get a notification when the dropdown closes. The best
// we can do is keep an "opened" flag and check to see if we
// still have the "open" class applied.
// still have the "open" class applied.
if (this.opened && !this.$hintMenu.hasClass("open")) {
this.opened = false;
}
Expand All @@ -254,7 +252,7 @@ define(function (require, exports, module) {
Menus.closeAll();

this.currentProvider = null;
$.each(this.hintProviders, function (index, item) {
$.each(hintProviders, function (index, item) {
var query = item.getQueryInfo(self.editor, self.editor.getCursorPos());
if (query.queryStr !== null) {
self.query = query;
Expand All @@ -269,6 +267,9 @@ define(function (require, exports, module) {
this.updateList();

if (this.displayList.length) {
// Need to add the menu to the DOM before trying to calculate its ideal location.
$("#codehint-menu-bar > ul").append(this.$hintMenu);

var hintPos = this.calcHintListLocation();

this.$hintMenu.addClass("open")
Expand All @@ -285,11 +286,19 @@ define(function (require, exports, module) {
* Closes the hint list
*/
CodeHintList.prototype.close = function () {
// TODO: Due to #1381, this won't get called if the user clicks out of the code hint menu.
// That's (sort of) okay right now since it doesn't really matter if a single old invisible
// code hint list is lying around (it'll get closed the next time the user pops up a code
// hint). Once #1381 is fixed this issue should go away.
this.$hintMenu.removeClass("open");
this.opened = false;
this.currentProvider = null;

PopUpManager.removePopUp(this.$hintMenu);
this.$hintMenu.remove();
if (hintList === this) {
hintList = null;
}
};

/**
Expand Down Expand Up @@ -343,13 +352,16 @@ define(function (require, exports, module) {

return itemsPerPage;
};


/**
* Show the code hint list near the current cursor position for the specified editor
* @param {Editor}
*/
function _showHint(editor) {
function showHint(editor) {
if (hintList) {
hintList.close();
}
hintList = new CodeHintList();
hintList.open(editor);
}

Expand All @@ -363,11 +375,11 @@ define(function (require, exports, module) {

// Check for Control+Space
if (event.type === "keydown" && event.keyCode === 32 && event.ctrlKey) {
_showHint(editor);
showHint(editor);
event.preventDefault();
} else if (event.type === "keypress") {
// Check if any provider wants to show hints on this key.
$.each(hintList.hintProviders, function (index, item) {
$.each(hintProviders, function (index, item) {
if (item.shouldShowHintsOnKey(String.fromCharCode(event.charCode))) {
provider = item;
return false;
Expand All @@ -378,7 +390,7 @@ define(function (require, exports, module) {
} else if (event.type === "keyup") {
if (shouldShowHintsOnKeyUp) {
shouldShowHintsOnKeyUp = false;
_showHint(editor);
showHint(editor);
}
}

Expand Down Expand Up @@ -413,7 +425,7 @@ define(function (require, exports, module) {
* - shouldShowHintsOnKey - inspects the char code and returns true if it wants to show code hints on that key.
*/
function registerHintProvider(providerInfo) {
hintList.hintProviders.push(providerInfo);
hintProviders.push(providerInfo);
}

/**
Expand All @@ -422,16 +434,10 @@ define(function (require, exports, module) {
function _getCodeHintList() {
return hintList;
}

// Initialize variables and listeners that depend on the HTML DOM
$(brackets).on("htmlContentLoadComplete", function () {
// HintList is a singleton for now. Todo: Figure out broader strategy for hint list across editors
// and different types of hint list when other types of hinting is added.
hintList = new CodeHintList();
});

// Define public API
exports.handleKeyEvent = handleKeyEvent;
exports.showHint = showHint;
exports._getCodeHintList = _getCodeHintList;
exports.registerHintProvider = registerHintProvider;
});
14 changes: 11 additions & 3 deletions src/extensions/default/HTMLCodeHints/main.js
View file
Expand Up @@ -150,7 +150,7 @@ define(function (require, exports, module) {
end = {line: -1, ch: -1},
tagInfo = HTMLUtils.getTagInfo(editor, cursor),
charCount = 0,
adjustCursor = false,
insertedName = false,
replaceExistingOne = tagInfo.attr.valueAssigned;

if (tagInfo.position.tokenType === HTMLUtils.ATTR_NAME) {
Expand All @@ -168,7 +168,7 @@ define(function (require, exports, module) {
if (!replaceExistingOne && attributes && attributes[completion] &&
attributes[completion].type !== "flag") {
completion += "=\"\"";
adjustCursor = true;
insertedName = true;
}

if (start.ch !== end.ch) {
Expand All @@ -177,8 +177,12 @@ define(function (require, exports, module) {
editor.document.replaceRange(completion, start);
}

if (adjustCursor) {
if (insertedName) {
editor.setCursorPos(start.line, start.ch + completion.length - 1);

// Since we're now inside the double-quotes we just inserted,
// mmediately pop up the attribute value hint.
CodeHintManager.showHint(editor);
}
};

Expand Down Expand Up @@ -271,4 +275,8 @@ define(function (require, exports, module) {
var attrHints = new AttrHints();
CodeHintManager.registerHintProvider(tagHints);
CodeHintManager.registerHintProvider(attrHints);

// For unit testing
exports.tagHintProvider = tagHints;
exports.attrHintProvider = attrHints;
});
21 changes: 19 additions & 2 deletions src/extensions/default/HTMLCodeHints/unittests.js
View file
Expand Up @@ -23,7 +23,7 @@


/*jslint vars: true, plusplus: true, devel: true, browser: true, nomen: true, indent: 4, maxerr: 50 */
/*global define, describe, it, expect, beforeEach, afterEach, waitsFor, runs, $, brackets, waitsForDone */
/*global define, describe, it, expect, beforeEach, afterEach, waitsFor, waits, runs, $, brackets, waitsForDone */

define(function (require, exports, module) {
"use strict";
Expand All @@ -33,7 +33,7 @@ define(function (require, exports, module) {
Editor = brackets.getModule("editor/Editor").Editor;

// Modules from testWindow
var HTMLCodeHints;
var HTMLCodeHints, CodeHintManager;

describe("HTML Attribute Hinting", function () {

Expand Down Expand Up @@ -62,6 +62,7 @@ define(function (require, exports, module) {
// Get access to the extension's module, so we can unit-test its APIs directly
var extensionRequire = testWindow.brackets.getModule("utils/ExtensionLoader").getRequireContextForExtension("HTMLCodeHints");
HTMLCodeHints = extensionRequire("main");
CodeHintManager = testWindow.brackets.getModule("editor/CodeHintManager");
});

// Once entire spec has finished, then close the window
Expand Down Expand Up @@ -363,6 +364,22 @@ define(function (require, exports, module) {
expectCursorAt({ line: 4, ch: 24 }); // cursor between the two "s
});

it("should pop up attribute value hints after attribute name has been inserted", function () {
runs(function () {
testEditor.setCursorPos({ line: 4, ch: 17 }); // cursor between space and >
});
waits(2000);
runs(function () {
selectHint(HTMLCodeHints.attrHintProvider, "class");
});
waits(2000);
runs(function () {
expect(testDocument.getLine(4)).toBe(" <h3 id = 'bar' class=\"\">Subheading</h3>");
expect(CodeHintManager._getCodeHintList()).toBeTruthy();
expect(CodeHintManager._getCodeHintList().isOpen()).toBe(true);
});
});

it("should NOT insert =\"\" after valueless attribute", function () {
testDocument.replaceRange(" <input \n", { line: 7, ch: 0 }); // insert new line

Expand Down
2 changes: 1 addition & 1 deletion test/spec/CodeHint-test.js
View file
Expand Up @@ -173,7 +173,7 @@ define(function (require, exports, module) {

// verify list is no longer open
var codeHintList = CodeHintManager._getCodeHintList();
expect(codeHintList.isOpen()).toBe(false);
expect(codeHintList).toBeFalsy();
});
});
});
Expand Down

0 comments on commit 7390867

Please sign in to comment.