Merge pull request #1958 from blink1073/fix-notebook-shortcuts

Fix notebook focus bugs

packages/notebook/src/widget.ts

@@ -875,6 +875,9 @@ class Notebook extends StaticNotebook {
     case 'mousemove':
       this._evtMousemove(event as MouseEvent);
       break;
+    case 'keydown':
+      this._ensureFocus(true);
+      break;
     case 'dblclick':
       this._evtDblClick(event as MouseEvent);
       break;
@@ -908,6 +911,7 @@ class Notebook extends StaticNotebook {
     super.onAfterAttach(msg);
     let node = this.node;
     node.addEventListener('mousedown', this);
+    node.addEventListener('keydown', this);
     node.addEventListener('dblclick', this);
     node.addEventListener('focus', this, true);
     node.addEventListener('blur', this, true);
@@ -923,6 +927,7 @@ class Notebook extends StaticNotebook {
   protected onBeforeDetach(msg: Message): void {
     let node = this.node;
     node.removeEventListener('mousedown', this);
+    node.removeEventListener('keydown', this);
     node.removeEventListener('dblclick', this);
     node.removeEventListener('focus', this, true);
     node.removeEventListener('blur', this, true);
@@ -1383,14 +1388,11 @@ class Notebook extends StaticNotebook {
    */
   private _evtBlur(event: MouseEvent): void {
     let relatedTarget = event.relatedTarget as HTMLElement;
+    // Bail if focus is leaving the notebook.
     if (!this.node.contains(relatedTarget)) {
       return;
     }
-    // If the root node is not blurring and we are in command mode,
-    // focus ourselves.
-    if (this.mode === 'command' && event.target !== this.node) {
-      this.node.focus();
-    }
+    this.mode = 'command';
   }
 
   /**

packages/shortcuts-extension/src/index.ts

@@ -47,12 +47,12 @@ const SHORTCUTS = [
   },
   {
     command: 'completer:invoke-console',
-    selector: '.jp-ConsolePanel .jp-mod-completer-enabled',
+    selector: '.jp-CodeConsole-prompt .jp-mod-completer-enabled',
     keys: ['Tab']
   },
   {
     command: 'completer:invoke-notebook',
-    selector: '.jp-Notebook .jp-mod-completer-enabled',
+    selector: '.jp-Notebook.jp-mod-editMode .jp-mod-completer-enabled',
     keys: ['Tab']
   },
   {
@@ -132,47 +132,62 @@ const SHORTCUTS = [
   },
   {
     command: 'notebook-cells:run-and-advance',
-    selector: '.jp-Notebook',
+    selector: '.jp-Notebook:focus',
     keys: ['Shift Enter']
   },
   {
     command: 'notebook-cells:run-and-insert',
-    selector: '.jp-Notebook',
+    selector: '.jp-Notebook:focus',
     keys: ['Alt Enter']
   },
   {
     command: 'notebook-cells:run',
-    selector: '.jp-Notebook',
+    selector: '.jp-Notebook:focus',
+    keys: ['Ctrl Enter']
+  },
+  {
+    command: 'notebook-cells:run-and-advance',
+    selector: '.jp-Notebook.jp-mod-editMode',
+    keys: ['Shift Enter']
+  },
+  {
+    command: 'notebook-cells:run-and-insert',
+    selector: '.jp-Notebook.jp-mod-editMode',
+    keys: ['Alt Enter']
+  },
+  {
+    command: 'notebook-cells:run',
+    selector: '.jp-Notebook.jp-mod-editMode',
     keys: ['Ctrl Enter']
   },
   {
     command: 'notebook:interrupt-kernel',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['I', 'I']
   },
   {
     command: 'notebook:restart-kernel',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['0', '0']
   },
   {
     command: 'notebook-cells:to-code',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['Y']
   },
   {
     command: 'notebook-cells:to-markdown',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['M']
   },
   {
     command: 'notebook-cells:to-raw',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['R']
   },
   {
     command: 'notebook-cells:delete',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['D', 'D'],
   },
   {
@@ -182,122 +197,122 @@ const SHORTCUTS = [
   },
   {
     command: 'notebook-cells:merge',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['Shift M'],
   },
   {
     command: 'notebook-cells:select-above',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['ArrowUp'],
   },
   {
     command: 'notebook-cells:select-above',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['K'],
   },
   {
     command: 'notebook-cells:select-below',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['ArrowDown'],
   },
   {
     command: 'notebook-cells:select-below',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['J'],
   },
   {
     command: 'notebook-cells:extend-above',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['Shift ArrowUp'],
   },
   {
     command: 'notebook-cells:extend-above',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['Shift K'],
   },
   {
     command: 'notebook-cells:extend-below',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['Shift ArrowDown'],
   },
   {
     command: 'notebook-cells:extend-below',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['Shift J'],
   },
   {
     command: 'notebook-cells:undo',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['Z'],
   },
   {
     command: 'notebook-cells:redo',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['Shift Z'],
   },
   {
     command: 'notebook-cells:cut',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['X']
   },
   {
     command: 'notebook-cells:copy',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['C']
   },
   {
     command: 'notebook-cells:paste',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['V']
   },
   {
     command: 'notebook-cells:insert-above',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['A']
   },
   {
     command: 'notebook-cells:insert-below',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['B']
   },
   {
     command: 'notebook-cells:toggle-line-numbers',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['L']
   },
   {
     command: 'notebook-cells:markdown-header1',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['1']
   },
   {
     command: 'notebook-cells:markdown-header2',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['2']
   },
   {
     command: 'notebook-cells:markdown-header3',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['3']
   },
   {
     command: 'notebook-cells:markdown-header4',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['4']
   },
   {
     command: 'notebook-cells:markdown-header5',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['5']
   },
   {
     command: 'notebook-cells:markdown-header6',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['6']
   },
   {
     command: 'notebook:edit-mode',
-    selector: '.jp-Notebook.jp-mod-commandMode:focus',
+    selector: '.jp-Notebook:focus',
     keys: ['Enter']
   },
   {
@@ -312,12 +327,12 @@ const SHORTCUTS = [
   },
   {
     command: 'tooltip:launch-notebook',
-    selector: '.jp-Notebook',
+    selector: '.jp-Notebook.jp-mod-editMode',
     keys: ['Shift Tab']
   },
   {
     command: 'tooltip:launch-console',
-    selector: '.jp-ConsolePanel',
+    selector: '.jp-CodeConsole-prompt',
     keys: ['Shift Tab']
   }
 ];

test/src/notebook/widget.spec.ts

@@ -12,7 +12,7 @@ import {
 } from '@phosphor/widgets';
 
 import {
-  simulate
+  generate, simulate
 } from 'simulate-event';
 
 import {
@@ -955,14 +955,13 @@ describe('notebook/widget', () => {
           expect(widget.activeCell.editor.hasFocus()).to.be(true);
         });
 
-        it('should give focus back to the notebook', () => {
+        it('should set command mode', () => {
           simulate(widget.node, 'focus');
-          let other = document.createElement('div');
-          simulate(widget.node, 'blur', { relatedTarget: other });
-          expect(widget.mode).to.be('command');
-          MessageLoop.sendMessage(widget, Widget.Msg.ActivateRequest);
+          widget.mode = 'edit';
+          let evt = generate('blur');
+          (evt as any).relatedTarget = widget.activeCell.node;
+          widget.node.dispatchEvent(evt);
           expect(widget.mode).to.be('command');
-          expect(widget.activeCell.editor.hasFocus()).to.be(false);
         });
 
       });