fix(menu): support menu item list change in deep decendents

adobe · May 27, 2020 · b2b47f3 · b2b47f3
1 parent e98b273
commit b2b47f3
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 9 deletions.
diff --git a/packages/menu-group/src/menu-group.ts b/packages/menu-group/src/menu-group.ts
@@ -17,6 +17,9 @@ import menuGroupStyles from './menu-group.css.js';
 /**
  * Spectrum Menu Group Component
  * @element sp-menu-group
+ *
+ * @slot header - headline of the menu group
+ * @slot - menu items to be listed in the group
  */
 export class MenuGroup extends LitElement {
     public static get styles(): CSSResultArray {

diff --git a/packages/menu-item/src/menu-item.ts b/packages/menu-item/src/menu-item.ts
@@ -54,6 +54,13 @@ export class MenuItem extends ActionButton {
         }
     }
 
+    /**
+     * Hide this getter from web-component-analyzer until
+     * https://github.com/runem/web-component-analyzer/issues/131
+     * has been addressed.
+     *
+     * @private
+     */
     public get itemText(): string {
         return (this.textContent || /* istanbul ignore next */ '').trim();
     }

diff --git a/packages/menu/src/menu.ts b/packages/menu/src/menu.ts
@@ -38,6 +38,13 @@ export class Menu extends LitElement {
     public focusedItemIndex = 0;
     public focusInItemIndex = 0;
 
+    /**
+     * Hide this getter from web-component-analyzer until
+     * https://github.com/runem/web-component-analyzer/issues/131
+     * has been addressed.
+     *
+     * @private
+     */
     public get childRole(): string {
         return this.getAttribute('role') === 'menu' ? 'menuitem' : 'option';
     }
@@ -68,11 +75,13 @@ export class Menu extends LitElement {
     private onClick(event: Event): void {
         const path = event.composedPath();
         const target = path.find((el) => {
+            /* istanbul ignore if */
             if (!(el instanceof Element)) {
                 return false;
             }
             return el.getAttribute('role') === this.childRole;
         }) as MenuItem;
+        /* istanbul ignore if */
         if (!target) {
             return;
         }
@@ -125,6 +134,10 @@ export class Menu extends LitElement {
             'focusout',
             () => {
                 requestAnimationFrame(() => {
+                    /* istanbul ignore if */
+                    if (this.menuItems.length === 0) {
+                        return;
+                    }
                     if (this.querySelector('[selected]')) {
                         const itemToBlur = this.menuItems[
                             this.focusInItemIndex
@@ -149,11 +162,12 @@ export class Menu extends LitElement {
             index -= 1;
             item = this.menuItems[index] as MenuItem;
         }
+        index = Math.max(index, 0);
         this.focusedItemIndex = index;
         this.focusInItemIndex = index;
     }
 
-    public handleSlotchange(): void {
+    private prepItems = (): void => {
         this.menuItems = [
             ...this.querySelectorAll(`[role="${this.childRole}"]`),
         ];
@@ -163,11 +177,11 @@ export class Menu extends LitElement {
         this.updateSelectedItemIndex();
         const focusInItem = this.menuItems[this.focusInItemIndex] as MenuItem;
         focusInItem.tabIndex = 0;
-    }
+    };
 
     public render(): TemplateResult {
         return html`
-            <slot @slotchange=${this.handleSlotchange}></slot>
+            <slot></slot>
         `;
     }
 
@@ -184,7 +198,22 @@ export class Menu extends LitElement {
             this.dispatchEvent(queryRoleEvent);
             this.setAttribute('role', queryRoleEvent.detail.role || 'menu');
         }
+        if (!this.observer) {
+            this.observer = new MutationObserver(this.prepItems);
+        }
+        this.observer.observe(this, { childList: true, subtree: true });
+        this.updateComplete.then(() => this.prepItems());
     }
+
+    public disconnectedCallback(): void {
+        /* istanbul ignore else */
+        if (this.observer) {
+            this.observer.disconnect();
+        }
+        super.disconnectedCallback();
+    }
+
+    private observer?: MutationObserver;
 }
 
 declare global {

diff --git a/packages/menu/test/menu.test.ts b/packages/menu/test/menu.test.ts
@@ -13,6 +13,7 @@ import '../';
 import { Menu } from '../';
 import '../../menu-item';
 import { MenuItem } from '../../menu-item';
+import '../../menu-group';
 import {
     fixture,
     elementUpdated,
@@ -39,18 +40,18 @@ describe('Menu', () => {
     it('renders empty', async () => {
         const el = await fixture<Menu>(
             html`
-                <sp-menu><a href="#">Test</a></sp-menu>
+                <sp-menu tabindex="0"><a href="#">Test</a></sp-menu>
             `
         );
 
         await elementUpdated(el);
 
         el.focus();
-        expect(document.activeElement === document.body).to.be.true;
+        expect(document.activeElement === document.body, 'self').to.be.true;
 
         const anchor = el.querySelector('a') as HTMLAnchorElement;
         anchor.focus();
-        expect(document.activeElement === anchor).to.be.true;
+        expect(document.activeElement === anchor, 'anchor').to.be.true;
     });
     it('renders w/ menu items', async () => {
         const el = await fixture<Menu>(
@@ -160,6 +161,54 @@ describe('Menu', () => {
         expect(document.activeElement === secondToLastItem).to.be.true;
     });
 
+    it('handle focus and late descendent additions', async () => {
+        const el = await fixture<Menu>(
+            html`
+                <sp-menu>
+                    <sp-menu-group>
+                        <span slot="header">Options</span>
+                        <sp-menu-item>
+                            Deselect
+                        </sp-menu-item>
+                    </sp-menu-group>
+                </sp-menu>
+            `
+        );
+
+        await elementUpdated(el);
+
+        const firstItem = el.querySelector(
+            'sp-menu-item:nth-of-type(1)'
+        ) as MenuItem;
+
+        el.focus();
+
+        expect(document.activeElement === firstItem).to.be.true;
+
+        firstItem.blur();
+
+        const group = el.querySelector('sp-menu-group') as HTMLElement;
+        const prependedItem = document.createElement('sp-menu-item');
+        prependedItem.innerHTML = 'Prepended Item';
+        const appendedItem = document.createElement('sp-menu-item');
+        prependedItem.innerHTML = 'Appended Item';
+        group.prepend(prependedItem);
+        group.append(appendedItem);
+
+        await elementUpdated(el);
+
+        expect(document.activeElement === firstItem).to.be.false;
+        expect(document.activeElement === prependedItem).to.be.false;
+
+        el.focus();
+
+        expect(document.activeElement === prependedItem).to.be.true;
+
+        el.dispatchEvent(arrowUpEvent);
+
+        expect(document.activeElement === appendedItem).to.be.true;
+    });
+
     it('cleans up when tabbing away', async () => {
         const el = await fixture<Menu>(
             html`
@@ -190,10 +239,10 @@ describe('Menu', () => {
         ) as MenuItem;
 
         el.focus();
-        expect(document.activeElement === firstItem).to.be.true;
+        expect(document.activeElement === firstItem, 'first').to.be.true;
         el.dispatchEvent(arrowDownEvent);
         el.dispatchEvent(arrowDownEvent);
-        expect(document.activeElement === thirdItem).to.be.true;
+        expect(document.activeElement === thirdItem, 'third').to.be.true;
         // imitate tabbing away
         el.dispatchEvent(tabEvent);
         el.dispatchEvent(
@@ -207,6 +256,6 @@ describe('Menu', () => {
         el.startListeningToKeyboard();
         // focus management should start again from the first item.
         el.dispatchEvent(arrowDownEvent);
-        expect(document.activeElement === secondItem).to.be.true;
+        expect(document.activeElement === secondItem, 'second').to.be.true;
     });
 });