Procházet zdrojové kódy

Focusin/out instead of focus/blur for notebook cells to allow events to bubble (#3540)

* Focusin/out instead of focus/blur for notebook cells to allow events to
bubble.

* Update tests.

* Handle focus issues where `event.target` is an orphaned node in Firefox.

* No need for capture in focusin/focusout.

* Fix typo in docstring.
Ian Rose před 7 roky
rodič
revize
837df44d21

+ 36 - 18
packages/notebook/src/widget.ts

@@ -1125,11 +1125,11 @@ class Notebook extends StaticNotebook {
     case 'dblclick':
       this._evtDblClick(event as MouseEvent);
       break;
-    case 'focus':
-      this._evtFocus(event as MouseEvent);
+    case 'focusin':
+      this._evtFocusIn(event as MouseEvent);
       break;
-    case 'blur':
-      this._evtBlur(event as MouseEvent);
+    case 'focusout':
+      this._evtFocusOut(event as MouseEvent);
       break;
     case 'p-dragenter':
       this._evtDragEnter(event as IDragEvent);
@@ -1157,8 +1157,8 @@ class Notebook extends StaticNotebook {
     node.addEventListener('mousedown', this);
     node.addEventListener('keydown', this);
     node.addEventListener('dblclick', this);
-    node.addEventListener('focus', this, true);
-    node.addEventListener('blur', this, true);
+    node.addEventListener('focusin', this);
+    node.addEventListener('focusout', this);
     node.addEventListener('p-dragenter', this);
     node.addEventListener('p-dragleave', this);
     node.addEventListener('p-dragover', this);
@@ -1173,8 +1173,8 @@ class Notebook extends StaticNotebook {
     node.removeEventListener('mousedown', this);
     node.removeEventListener('keydown', this);
     node.removeEventListener('dblclick', this);
-    node.removeEventListener('focus', this, true);
-    node.removeEventListener('blur', this, true);
+    node.removeEventListener('focusin', this);
+    node.removeEventListener('focusout', this);
     node.removeEventListener('p-dragenter', this);
     node.removeEventListener('p-dragleave', this);
     node.removeEventListener('p-dragover', this);
@@ -1359,13 +1359,16 @@ class Notebook extends StaticNotebook {
       return;
     }
 
-    // Try to find the cell associated with the event.
-    // `event.target` sometimes gives an orphaned node in Firefox 57.
+    // `event.target` sometimes gives an orphaned node in Firefox 57, which
+    // can have `null` anywhere in its parent tree. If we fail to find a
+    // cell using `event.target`, try again using a target reconstructed from
+    // the position of the click event.
     let target = event.target as HTMLElement;
-    if (!target.parentElement) {
+    let index = this._findCell(target);
+    if (index === -1) {
       target = document.elementFromPoint(event.clientX, event.clientY) as HTMLElement;
+      index = this._findCell(target);
     }
-    let index = this._findCell(target);
     let widget = this.widgets[index];
 
     // Switch to command mode if the click is not in an editor.
@@ -1696,7 +1699,7 @@ class Notebook extends StaticNotebook {
   /**
    * Handle `focus` events for the widget.
    */
-  private _evtFocus(event: MouseEvent): void {
+  private _evtFocusIn(event: MouseEvent): void {
     let target = event.target as HTMLElement;
     let i = this._findCell(target);
     if (i !== -1) {
@@ -1718,14 +1721,24 @@ class Notebook extends StaticNotebook {
   }
 
   /**
-   * Handle `blur` events for the notebook.
+   * Handle `focusout` events for the notebook.
    */
-  private _evtBlur(event: MouseEvent): void {
+  private _evtFocusOut(event: MouseEvent): void {
     let relatedTarget = event.relatedTarget as HTMLElement;
     // Bail if focus is leaving the notebook.
     if (!this.node.contains(relatedTarget)) {
       return;
     }
+    // Bail if the item gaining focus is another cell,
+    // and we should not be entering command mode.
+    const i = this._findCell(relatedTarget);
+    if (i !== -1) {
+      const widget = this.widgets[i];
+      if (widget.editorWidget.node.contains(relatedTarget)) {
+        return;
+      }
+    }
+    // Otherwise enter command mode.
     this.mode = 'command';
   }
 
@@ -1739,12 +1752,17 @@ class Notebook extends StaticNotebook {
     }
     this.deselectAll();
 
-    // `event.target` sometimes gives an orphaned node in Firefox 57.
+    // `event.target` sometimes gives an orphaned node in Firefox 57, which
+    // can have `null` anywhere in its parent tree. If we fail to find a
+    // cell using `event.target`, try again using a target reconstructed from
+    // the position of the click event.
     let target = event.target as HTMLElement;
-    if (!target.parentElement) {
+    let i = this._findCell(target);
+    if (i === -1) {
       target = document.elementFromPoint(event.clientX, event.clientY) as HTMLElement;
+      i = this._findCell(target);
     }
-    let i = this._findCell(target);
+
     if (i === -1) {
       return;
     }

+ 16 - 16
tests/test-notebook/src/widget.spec.ts

@@ -1206,35 +1206,35 @@ describe('notebook/widget', () => {
 
       });
 
-      context('focus', () => {
+      context('focusin', () => {
 
         it('should change to edit mode if a child cell takes focus', () => {
           let child = widget.widgets[0];
-          simulate(child.editorWidget.node, 'focus');
-          expect(widget.events).to.contain('focus');
+          simulate(child.editorWidget.node, 'focusin');
+          expect(widget.events).to.contain('focusin');
           expect(widget.mode).to.be('edit');
         });
 
         it('should change to command mode if the widget takes focus', () => {
           let child = widget.widgets[0];
-          simulate(child.editorWidget.node, 'focus');
-          expect(widget.events).to.contain('focus');
+          simulate(child.editorWidget.node, 'focusin');
+          expect(widget.events).to.contain('focusin');
           expect(widget.mode).to.be('edit');
           widget.events = [];
-          simulate(widget.node, 'focus');
-          expect(widget.events).to.contain('focus');
+          simulate(widget.node, 'focusin');
+          expect(widget.events).to.contain('focusin');
           expect(widget.mode).to.be('command');
         });
 
       });
 
-      context('blur', () => {
+      context('focusout', () => {
 
         it('should preserve the mode', () => {
-          simulate(widget.node, 'focus');
+          simulate(widget.node, 'focusin');
           widget.mode = 'edit';
           let other = document.createElement('div');
-          simulate(widget.node, 'blur', { relatedTarget: other });
+          simulate(widget.node, 'focusout', { relatedTarget: other });
           expect(widget.mode).to.be('edit');
           MessageLoop.sendMessage(widget, Widget.Msg.ActivateRequest);
           expect(widget.mode).to.be('edit');
@@ -1242,9 +1242,9 @@ describe('notebook/widget', () => {
         });
 
         it('should set command mode', () => {
-          simulate(widget.node, 'focus');
+          simulate(widget.node, 'focusin');
           widget.mode = 'edit';
-          let evt = generate('blur');
+          let evt = generate('focusout');
           (evt as any).relatedTarget = widget.activeCell.node;
           widget.node.dispatchEvent(evt);
           expect(widget.mode).to.be('command');
@@ -1267,8 +1267,8 @@ describe('notebook/widget', () => {
           expect(widget.events).to.contain('mousedown');
           simulate(widget.node, 'dblclick');
           expect(widget.events).to.contain('dblclick');
-          simulate(child.node, 'focus');
-          expect(widget.events).to.contain('focus');
+          simulate(child.node, 'focusin');
+          expect(widget.events).to.contain('focusin');
           widget.dispose();
           done();
         });
@@ -1305,8 +1305,8 @@ describe('notebook/widget', () => {
           expect(widget.events).to.not.contain('mousedown');
           simulate(widget.node, 'dblclick');
           expect(widget.events).to.not.contain('dblclick');
-          simulate(child.node, 'focus');
-          expect(widget.events).to.not.contain('focus');
+          simulate(child.node, 'focusin');
+          expect(widget.events).to.not.contain('focusin');
           widget.dispose();
           done();
         });