Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Prevent menus from stealing focus and enables indent/undent from menus #672

Merged
merged 11 commits into from Apr 19, 2012
View
@@ -172,8 +172,6 @@ define(function (require, exports, module) {
{"Shift-F3": Commands.EDIT_FIND_PREVIOUS, "platform": "win"},
{"Ctrl-Alt-F": Commands.EDIT_REPLACE, "platform": "mac"},
{"Ctrl-H": Commands.EDIT_REPLACE, "platform": "win"},
- {"Ctrl-Tab": Commands.EDIT_INDENT},
- {"Ctrl-Shift-Tab": Commands.EDIT_UNINDENT},
// VIEW
{"Ctrl-Shift-H": Commands.VIEW_HIDE_SIDEBAR},
View
@@ -204,6 +204,9 @@ define(function (require, exports, module) {
}
}
+ // Translate some keys to their common names }
+ if (key === "\t") { key = "Tab"; }
+
return _buildKeyDescriptor(hasCtrl, hasAlt, hasShift, key);
}
View
@@ -41,7 +41,6 @@ define(function (require, exports, module) {
"menu-edit-find-next": Commands.EDIT_FIND_NEXT,
"menu-edit-find-previous": Commands.EDIT_FIND_PREVIOUS,
"menu-edit-replace": Commands.EDIT_REPLACE,
-
// View
"menu-view-hide-sidebar": Commands.VIEW_HIDE_SIDEBAR,
@@ -69,10 +68,8 @@ define(function (require, exports, module) {
function createExecFunc(commandStr) {
return function () {
- // TODO TY: should flash menu here
+ // TODO TY: should flash menu here on Mac
//console.log(commandStr);
-
- EditorManager.focusEditor();
CommandManager.execute(commandStr);
};
}
@@ -106,11 +103,22 @@ define(function (require, exports, module) {
shortcut = keyCmd.replace(/-/g, "+");
}
- $("#" + menuID).append("<span class='menu-shortcut'>" + shortcut + "</span>");
+ var $menu = $("#" + menuID);
+ // Some commands have multiple key commands. Only add the first one.
+ if ($menu.find(".menu-shortcut").length === 0) {
+ $menu.append("<span class='menu-shortcut'>" + shortcut + "</span>");
@redmunds

redmunds Apr 19, 2012

Contributor

Seems like the code can exit the loop after the first shortcut string is added. Or at least, only build the "shortcut" string once you've determined that it's the first one.

@peterflynn

peterflynn Apr 19, 2012

Member

I think that's the idea -- if there's more than one shortcut bound to the same menu item, we don't want to list all the shortcuts in the menu UI... just the first one we come across. There's no break or anything, so it won't actually exit the loop; it just won't add another shortcut label to the UI if one is already present.

@peterflynn

peterflynn Apr 19, 2012

Member

Although, Ty, this does raise one issue: if there's more than one shortcut, the one that wins is completely arbitrary, not the first one listed in the code in brackets.js (because we're iterating a map here, not an ordered list).

Maybe because of this, for now we should print a warning to the console whenever a 2nd shortcut for the same menu is encountered? I think with the removal of the Tab bindings above, we no longer have any commands that would hit this caseat the moment...

+ }
}
}
}
+ // Prevent clicks on the top-level menu bar from taking focus
+ // Note, bootstrap handles this already for the menu drop downs
+ $("#main-toolbar .dropdown").mousedown(function (e) {
+ e.preventDefault();
+ });
+
+
// Other debug menu items
// $("#menu-debug-wordwrap").click(function() {
View
@@ -47,8 +47,6 @@ define(function (require, exports, module) {
TextRange = require("document/TextRange").TextRange,
ViewUtils = require("utils/ViewUtils");
-
-
/**
* @private
@@ -191,7 +189,7 @@ define(function (require, exports, module) {
function _handleSelectAll() {
var editor = EditorManager.getFocusedEditor();
if (editor) {
- editor._codeMirror.execCommand("selectAll");
+ editor._selectAllVisible();
}
}
@@ -294,6 +292,7 @@ define(function (require, exports, module) {
// Editor supplies some standard keyboard behavior extensions of its own
var codeMirrorKeyMap = {
"Tab" : _handleTabKey,
+
"Left" : function (instance) {
if (!_handleSoftTabNavigation(instance, -1, "moveH")) {
CodeMirror.commands.goCharLeft(instance);
@@ -971,10 +970,6 @@ define(function (require, exports, module) {
CommandManager.register(Commands.EDIT_FIND_PREVIOUS, _findPrevious);
CommandManager.register(Commands.EDIT_SELECT_ALL, _handleSelectAll);
- // TODO: code mirror handles
- // CommandManager.register(Commands.EDIT_INDENT, _handleTabKey);
- // CommandManager.register(Commands.EDIT_UNINDENT, _handleTabKey);
-
// Define public API
exports.Editor = Editor;
});
View
@@ -98,9 +98,6 @@
<li><a href="#" id="menu-edit-find-previous">Find Previous</a></li>
<li><hr class="divider"></li>
<li><a href="#" id="menu-edit-replace">Replace</a></li>
- <li><hr class="divider"></li>
- <li><a href="#" id="menu-edit-indent">Indent</a></li>
- <li><a href="#" id="menu-edit-unindent">Unindent</a></li>
</ul>
</li>