Add keyboard shortcuts to menu dropdowns (#5525)

* render keyboard shortcuts from KeyboardManager render keyboard shortcuts from KeyboardManager * float menu keybindings to the right * Clean up styling * Humanize keybindings for display * Add some missing menu item/action pairs * Move styles to stylesheet * remove click event add/removal in enable/disable_paste * change the CSS rule to avoid wrapping in the middle of the shortcut text (#2759) Co-authored-by: Aaron Myatt <[email protected]> Co-authored-by: Grant Nestor <[email protected]> Co-authored-by: Jarrad Whitaker <[email protected]> Co-authored-by: Pierre Monod-Broca <[email protected]>
jupyter · Jul 23, 2020 · 7978c0f · 7978c0f
1 parent a94d316
commit 7978c0f
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 21 deletions.
diff --git a/notebook/static/notebook/js/menubar.js b/notebook/static/notebook/js/menubar.js
@@ -7,10 +7,11 @@ define([
     'base/js/dialog',
     'base/js/utils',
     'base/js/i18n',
+    'notebook/js/quickhelp',
     './celltoolbar',
     './tour',
     'moment',
-], function($, IPython, dialog, utils, i18n, celltoolbar, tour, moment) {
+], function($, IPython, dialog, utils, i18n, quickhelp, celltoolbar, tour, moment) {
     "use strict";
 
     var MenuBar = function (selector, options) {
@@ -24,6 +25,7 @@ define([
          *  options: dictionary
          *      Dictionary of keyword arguments.
          *          notebook: Notebook instance
+         *          render keyboard shortcuts from KeyboardManager
          *          contents: ContentManager instance
          *          events: $(Events) instance
          *          save_widget: SaveWidget instance
@@ -37,7 +39,8 @@ define([
         this.base_url = options.base_url || utils.get_body_data("baseUrl");
         this.selector = selector;
         this.notebook = options.notebook;
-        this.actions = this.notebook.keyboard_manager.actions;
+        this.keyboard_manager = this.notebook.keyboard_manager;
+        this.actions = this.keyboard_manager.actions;
         this.contents = options.contents;
         this.events = options.events;
         this.save_widget = options.save_widget;
@@ -232,6 +235,9 @@ define([
             '#int_kernel': 'interrupt-kernel',
             '#cut_cell': 'cut-cell',
             '#copy_cell': 'copy-cell',
+            '#paste_cell_above': 'paste-cell-above',
+            '#paste_cell_below': 'paste-cell-below',
+            '#paste_cell_replace': 'paste-cell-replace',
             '#delete_cell': 'delete-cell',
             '#undelete_cell': 'undo-cell-deletion',
             '#split_cell': 'split-cell-at-cursor',
@@ -263,6 +269,7 @@ define([
             '#copy_cell_attachments': 'copy-cell-attachments',
             '#paste_cell_attachments': 'paste-cell-attachments',
             '#insert_image': 'insert-image',
+            '#keyboard_shortcuts' : 'show-keyboard-shortcuts',
             '#edit_keyboard_shortcuts' : 'edit-command-mode-keyboard-shortcuts',
         };
 
@@ -276,9 +283,22 @@ define([
             }
             // Immediately-Invoked Function Expression cause JS.
             (function(that, id_act, idx){
-                that.element.find(idx).click(function(event){
+                var el = that.element.find(idx);
+                el.click(function(event){
                     that.actions.call(id_act, event);
                 });
+
+                var keybinding = that.keyboard_manager.command_shortcuts.get_action_shortcut(id_act) || that.keyboard_manager.edit_shortcuts.get_action_shortcut(id_act);
+
+                if (keybinding) {
+                    var shortcut = quickhelp.humanize_sequence(keybinding);
+                    var link_element = el.children('a');
+                    var text = link_element.text();
+                    link_element.text('');
+                    link_element.addClass('menu-shortcut-container');
+                    link_element.append('<span class="action">' + text + '</span>');
+                    link_element.append('<span class="kb">' + shortcut + '</span>');
+                }
             })(that, id_act, idx);
         }
 
@@ -295,9 +315,6 @@ define([
         } else {
             this.element.find('#notebook_tour').addClass("disabled");
         }
-        this.element.find('#keyboard_shortcuts').click(function () {
-            that.quick_help.show_keyboard_shortcuts();
-        });
 
         this.update_restore_checkpoint(null);
 

diff --git a/notebook/static/notebook/js/notebook.js b/notebook/static/notebook/js/notebook.js
@@ -1640,17 +1640,11 @@ define([
         var that = this;
         if (!this.paste_enabled) {
             $('#paste_cell_replace').removeClass('disabled')
-                .on('click', function () {that.keyboard_manager.actions.call(
-                    'jupyter-notebook:paste-cell-replace');});
-            $('#paste_cell_replace > a').attr('aria-disabled', 'false'); 
+            $('#paste_cell_replace > a').attr('aria-disabled', 'false');
             $('#paste_cell_above').removeClass('disabled')
-                .on('click', function () {that.keyboard_manager.actions.call(
-                    'jupyter-notebook:paste-cell-above');});
-            $('#paste_cell_above > a').attr('aria-disabled', 'false'); 
+            $('#paste_cell_above > a').attr('aria-disabled', 'false');
             $('#paste_cell_below').removeClass('disabled')
-                .on('click', function () {that.keyboard_manager.actions.call(
-                    'jupyter-notebook:paste-cell-below');});
-            $('#paste_cell_below > a').attr('aria-disabled', 'false');         
+            $('#paste_cell_below > a').attr('aria-disabled', 'false');
             this.paste_enabled = true;
         }
     };
@@ -1660,12 +1654,12 @@ define([
      */
     Notebook.prototype.disable_paste = function () {
         if (this.paste_enabled) {
-            $('#paste_cell_replace').addClass('disabled').off('click');
-            $('#paste_cell_replace > a').attr('aria-disabled', 'true'); 
-            $('#paste_cell_above').addClass('disabled').off('click');
-            $('#paste_cell_above > a').attr('aria-disabled', 'true'); 
-            $('#paste_cell_below').addClass('disabled').off('click');
-            $('#paste_cell_below > a').attr('aria-disabled', 'true'); 
+            $('#paste_cell_replace').addClass('disabled');
+            $('#paste_cell_replace > a').attr('aria-disabled', 'true');
+            $('#paste_cell_above').addClass('disabled');
+            $('#paste_cell_above > a').attr('aria-disabled', 'true');
+            $('#paste_cell_below').addClass('disabled');
+            $('#paste_cell_below > a').attr('aria-disabled', 'true');
             this.paste_enabled = false;
         }
     };

diff --git a/notebook/static/notebook/less/menubar.less b/notebook/static/notebook/less/menubar.less
@@ -71,6 +71,13 @@ i.menu-icon {
     .pull-left();
 }
 
+ul.dropdown-menu li a.menu-shortcut-container {
+  display: flex;
+  flex-direction: row;
+  justify-content: space-between;
+  align-items: flex-end;
+}
+
 ul#help_menu li a{
     overflow: hidden;
     padding-right: 2.2em;
@@ -146,4 +153,17 @@ ul#help_menu li a{
     margin-left: 10px;
 }
 
+.action {
+
+}
+
+.kb {
+    color: darkgray;
+    margin-left: 10px;
+    text-transform: capitalize;
+    kbd {
+        white-space: nowrap;
+    }
+}
+
 //end submenu