fix: reset tabindex to -1 when initializing menu item (#7203)

vaadin · Mar 14, 2024 · 5937dcd · 5937dcd
1 parent c104ecc
commit 5937dcd
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 1 deletion.
diff --git a/packages/context-menu/src/vaadin-contextmenu-items-mixin.js b/packages/context-menu/src/vaadin-contextmenu-items-mixin.js
@@ -211,6 +211,7 @@ export const ItemsMixin = (superClass) =>
 
         if (component instanceof Item) {
           component.setAttribute('role', 'menuitem');
+          component.tabIndex = -1;
           component.classList.add('vaadin-menu-item');
         } else if (component.localName === 'hr') {
           component.setAttribute('role', 'separator');

diff --git a/packages/context-menu/test/items.test.js b/packages/context-menu/test/items.test.js
@@ -25,6 +25,12 @@ const menuOpenEvent = isTouch ? 'click' : 'mouseover';
 describe('items', () => {
   let rootMenu, subMenu, target;
 
+  const createComponent = (text) => {
+    const item = document.createElement('vaadin-context-menu-item');
+    item.textContent = text;
+    return item;
+  };
+
   const menuComponents = (menu = rootMenu) => {
     return Array.from(menu.$.overlay.content.firstElementChild.children);
   };
@@ -80,6 +86,14 @@ describe('items', () => {
           ],
         },
         { text: 'foo-1' },
+        {
+          text: 'foo-2',
+          children: [
+            { component: createComponent('foo-2-0') },
+            { component: createComponent('foo-2-1') },
+            { component: createComponent('foo-2-2') },
+          ],
+        },
       ];
       open();
       await nextRender(rootMenu);
@@ -383,6 +397,34 @@ describe('items', () => {
       expect(spy.calledOnce).to.be.true;
     });
 
+    it('should focus first item after re-opening when using components', async () => {
+      subMenu.close();
+      rootMenu.$.overlay.focus();
+
+      const rootItem = getMenuItems(rootMenu)[2];
+
+      // Open the sub-menu with item components
+      open(rootItem);
+      await nextRender();
+      const subMenu2 = getSubMenu(rootMenu);
+      const items = getMenuItems(subMenu2);
+
+      // Arrow Down to focus next item
+      items[0].focus();
+      arrowDownKeyDown(items[0]);
+      expect(items[1].hasAttribute('focus-ring')).to.be.true;
+
+      // Arrow Left to close the sub-menu
+      arrowLeftKeyDown(items[1]);
+      await nextFrame();
+      expect(rootItem.hasAttribute('focus-ring')).to.be.true;
+
+      // Re-open sub-menu and check focus
+      open(rootItem);
+      await nextRender();
+      expect(items[0].hasAttribute('focus-ring')).to.be.true;
+    });
+
     it('should have modeless sub menus', () => {
       const rootItemRect = menuComponents()[0].getBoundingClientRect();
       const element = document.elementFromPoint(rootItemRect.left, rootItemRect.top);

diff --git a/packages/menu-bar/test/sub-menu.test.js b/packages/menu-bar/test/sub-menu.test.js
@@ -27,6 +27,12 @@ const menuOpenEvent = isTouch ? 'click' : 'mouseover';
 describe('sub-menu', () => {
   let menu, buttons, subMenu, item;
 
+  const createComponent = (text) => {
+    const item = document.createElement('vaadin-context-menu-item');
+    item.textContent = text;
+    return item;
+  };
+
   beforeEach(async () => {
     menu = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>');
     menu.items = [
@@ -43,7 +49,14 @@ describe('sub-menu', () => {
       { text: 'Menu Item 2' },
       {
         text: 'Menu Item 3',
-        children: [{ text: 'Menu Item 3 1' }, { text: 'Menu Item 3 2' }],
+        children: [
+          {
+            component: createComponent('Menu Item 3 1'),
+          },
+          {
+            component: createComponent('Menu Item 3 2'),
+          },
+        ],
       },
     ];
     await nextRender(menu);
@@ -135,6 +148,22 @@ describe('sub-menu', () => {
     expect(spy.calledOnce).to.be.true;
   });
 
+  it('should focus first item after re-opening when using components', async () => {
+    arrowDown(buttons[2]);
+    await nextRender(subMenu);
+
+    const items = subMenu.$.overlay.querySelectorAll('vaadin-context-menu-item');
+    arrowDown(items[0]);
+    expect(items[1].hasAttribute('focus-ring')).to.be.true;
+
+    // Close and re-open
+    esc(items[1]);
+    arrowDown(buttons[2]);
+    await nextRender(subMenu);
+
+    expect(items[0].hasAttribute('focus-ring')).to.be.true;
+  });
+
   it('should close sub-menu on first item arrow up', async () => {
     arrowDown(buttons[0]);
     await oneEvent(subMenu, 'opened-changed');