Merge pull request #187 from marthacryan/remove-tabindex

Make focus consistent with active element in menus
jupyterlab · Jul 26, 2021 · ad3aeab · ad3aeab
2 parents cd1de16 + 2cebd35
commit ad3aeab
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 5 deletions.
diff --git a/packages/widgets/src/menu.ts b/packages/widgets/src/menu.ts
@@ -220,6 +220,11 @@ class Menu extends Widget {
     // Update the active index.
     this._activeIndex = value;
 
+    // Make active element in focus
+    if (this._activeIndex >= 0 && this.contentNode.childNodes[this._activeIndex]) {
+      (this.contentNode.childNodes[this._activeIndex] as HTMLElement).focus();
+    }
+
     // schedule an update of the items.
     this.update();
   }
@@ -541,7 +546,14 @@ class Menu extends Widget {
       let item = items[i];
       let active = i === activeIndex;
       let collapsed = collapsedFlags[i];
-      content[i] = renderer.renderItem({ item, active, collapsed });
+      content[i] = renderer.renderItem({
+        item,
+        active,
+        collapsed,
+        onfocus: () => {
+          this.activeIndex = i;
+        }
+      });
     }
     VirtualDOM.render(content, this.contentNode);
   }
@@ -1109,6 +1121,11 @@ namespace Menu {
      * Whether the item should be collapsed.
      */
     readonly collapsed: boolean;
+
+    /**
+     * Handler for when element is in focus.
+     */
+    readonly onfocus?: () => void;
   }
 
   /**
@@ -1151,7 +1168,14 @@ namespace Menu {
       let dataset = this.createItemDataset(data);
       let aria = this.createItemARIA(data);
       return (
-        h.li({ className, dataset, ...aria },
+        h.li(
+          {
+            className,
+            dataset,
+            tabindex: '0',
+            onfocus: data.onfocus,
+            ...aria
+          },
           this.renderIcon(data),
           this.renderLabel(data),
           this.renderShortcut(data),
@@ -1335,8 +1359,14 @@ namespace Menu {
         break;
       case 'submenu':
         aria['aria-haspopup'] = 'true';
+        if (!data.item.isEnabled) {
+          aria['aria-disabled'] = 'true';
+        }
         break;
       default:
+        if (!data.item.isEnabled) {
+          aria['aria-disabled'] = 'true';
+        }
         aria.role = 'menuitem';
       }
       return aria;
@@ -1425,7 +1455,7 @@ namespace Private {
     /* </DEPRECATED> */
     node.appendChild(content);
     content.setAttribute('role', 'menu');
-    node.tabIndex = -1;
+    node.tabIndex = 0;
     return node;
   }
 

diff --git a/packages/widgets/src/menubar.ts b/packages/widgets/src/menubar.ts
@@ -144,6 +144,11 @@ class MenuBar extends Widget {
     // Update the active index.
     this._activeIndex = value;
 
+    // Update focus to new active index
+    if (this._activeIndex >= 0 && this.contentNode.childNodes[this._activeIndex]) {
+      (this.contentNode.childNodes[this._activeIndex] as HTMLElement).focus();
+    }
+
     // Schedule an update of the items.
     this.update();
   }
@@ -404,7 +409,13 @@ class MenuBar extends Widget {
     for (let i = 0, n = menus.length; i < n; ++i) {
       let title = menus[i].title;
       let active = i === activeIndex;
-      content[i] = renderer.renderItem({ title, active });
+      content[i] = renderer.renderItem({
+        title,
+        active,
+        onfocus: () => {
+          this.activeIndex = i;
+        }
+      });
     }
     VirtualDOM.render(content, this.contentNode);
   }
@@ -742,6 +753,8 @@ namespace MenuBar {
      * Whether the item is the active item.
      */
     readonly active: boolean;
+
+    readonly onfocus?: (event: FocusEvent) => void;
   }
 
   /**
@@ -784,7 +797,7 @@ namespace MenuBar {
       let dataset = this.createItemDataset(data);
       let aria = this.createItemARIA(data);
       return (
-        h.li({ className, dataset, ...aria },
+        h.li({ className, dataset, tabindex: '0', onfocus: data.onfocus, ...aria },
           this.renderIcon(data),
           this.renderLabel(data)
         )
@@ -950,6 +963,7 @@ namespace Private {
     node.appendChild(content);
     content.setAttribute('role', 'menubar');
     node.tabIndex = 0;
+    content.tabIndex = 0;
     return node;
   }