Merge pull request #5932 from ian-r-rose/context-menu-cleanup

Simplify application context menu cache, rename to be clearer.
jupyterlab · Feb 1, 2019 · a496aad · a496aad
2 parents c224c16 + 26712a2
commit a496aad
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 28 deletions.
diff --git a/packages/application-extension/src/index.tsx b/packages/application-extension/src/index.tsx
@@ -386,7 +386,7 @@ const sidebar: JupyterFrontEndPlugin<void> = {
         // application context menu click,
         // If we can't find it there, look for use the active
         // left/right widgets.
-        const contextNode: HTMLElement = app.contextMenuFirst(
+        const contextNode: HTMLElement = app.contextMenuHitTest(
           node => !!node.dataset.id
         );
         let id: string;

diff --git a/packages/application/src/frontend.ts b/packages/application/src/frontend.ts
@@ -106,15 +106,36 @@ export abstract class JupyterFrontEnd<
    *
    * @returns an HTMLElement or undefined, if none is found.
    */
-  contextMenuFirst(
+  contextMenuHitTest(
     test: (node: HTMLElement) => boolean
   ): HTMLElement | undefined {
-    for (let node of this._getContextMenuNodes()) {
+    if (
+      !this._contextMenuEvent ||
+      !(this._contextMenuEvent.target instanceof HTMLElement)
+    ) {
+      return undefined;
+    }
+    let node = this._contextMenuEvent.target as HTMLElement;
+    do {
       if (test(node)) {
         return node;
       }
+      node = node.parentNode as HTMLElement;
+    } while (node.parentNode && node !== node.parentNode);
+    return undefined;
+
+    // TODO: we should be able to use .composedPath() to simplify this function
+    // down to something like the below, but it seems like composedPath is
+    // sometimes returning an empty list.
+    /*
+    if (this._contextMenuEvent) {
+      this._contextMenuEvent
+        .composedPath()
+        .filter(x => x instanceof HTMLElement)
+        .find(test);
     }
     return undefined;
+    */
   }
 
   /**
@@ -125,30 +146,6 @@ export abstract class JupyterFrontEnd<
     super.evtContextMenu(event);
   }
 
-  /**
-   * Gets the hierarchy of html nodes that was under the cursor
-   * when the most recent contextmenu event was issued
-   */
-  private _getContextMenuNodes(): HTMLElement[] {
-    if (!this._contextMenuEvent) {
-      return [];
-    }
-
-    // This one-liner doesn't work, but should at some point in the future
-    // `return this._contextMenuEvent.composedPath() as HTMLElement[];`
-    // cf. (https://developer.mozilla.org/en-US/docs/Web/API/Event)
-
-    const nodes: HTMLElement[] = [this._contextMenuEvent.target as HTMLElement];
-    while (
-      'parentNode' in nodes[nodes.length - 1] &&
-      nodes[nodes.length - 1].parentNode &&
-      nodes[nodes.length - 1] !== nodes[nodes.length - 1].parentNode
-    ) {
-      nodes.push(nodes[nodes.length - 1].parentNode as HTMLElement);
-    }
-    return nodes;
-  }
-
   private _contextMenuEvent: MouseEvent;
 }
 

diff --git a/packages/docmanager-extension/src/index.ts b/packages/docmanager-extension/src/index.ts
@@ -623,7 +623,7 @@ function addLabCommands(
     const pathRe = /[Pp]ath:\s?(.*)\n?/;
     const test = (node: HTMLElement) =>
       node['title'] && !!node['title'].match(pathRe);
-    const node = app.contextMenuFirst(test);
+    const node = app.contextMenuHitTest(test);
 
     if (!node) {
       // Fall back to active doc widget if path cannot be obtained from event.