fix(left-nav): fix focus trapping (#11842)

### Related Ticket(s) https://jsw.ibm.com/browse/ADCMS-5191 ### Description Fixes the focus trapping on the left-nav component ### Changelog **Changed** - Fixes the focus trapping on the left-nav component ### Testing Instructions - Open the masthead story - Reduce viewport width to get the mobile version of the nav - Open the left-nav with the keyboard - Ensure focus moves to the first item in the left-nav on open - Ensure focus is trapped in the left-nav, cycling between the close button & last link/button element in both directions.
carbon-design-system · Jun 10, 2024 · 5f824ec · 5f824ec
1 parent 2c8944c
commit 5f824ec
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 112 deletions.
diff --git a/packages/carbon-web-components/src/components/ui-shell/side-nav.ts b/packages/carbon-web-components/src/components/ui-shell/side-nav.ts
@@ -10,7 +10,6 @@
 import { LitElement, html } from 'lit';
 import { property } from 'lit/decorators.js';
 import { classMap } from 'lit/directives/class-map.js';
-import on from '../../globals/mixins/on';
 import { prefix } from '../../globals/settings';
 import HostListenerMixin from '../../globals/mixins/host-listener';
 import HostListener from '../../globals/decorators/host-listener';
@@ -43,18 +42,6 @@ class CDSSideNav extends HostListenerMixin(LitElement) {
    */
   private _hTransition: Handle | null = null;
 
-  /**
-   * A promise that is resolved when the transition is complete.
-   */
-  private _transitionPromise = Promise.resolve();
-
-  /**
-   * A promise that is resolved when the transition upon proprety update is complete.
-   */
-  private get _updateAndTransitionPromise() {
-    return this.updateComplete.then(() => this._transitionPromise);
-  }
-
   /**
    * Cleans the handle for `transitionend` event listener.
    */
@@ -69,19 +56,8 @@ class CDSSideNav extends HostListenerMixin(LitElement) {
    */
   @HostListener('parentRoot:eventButtonToggle')
   // @ts-ignore: The decorator refers to this method but TS thinks this method is not referred to
-  private _handleButtonToggle = async (event: CustomEvent) => {
+  protected _handleButtonToggle = async (event: CustomEvent) => {
     this.expanded = event.detail.active;
-    if (this.expanded) {
-      await this._updateAndTransitionPromise;
-      // Checks if the side nav is not collapsed during the animation
-      if (this.expanded) {
-        (
-          this.querySelector(
-            (this.constructor as typeof CDSSideNav).selectorNavItems
-          ) as HTMLElement
-        )?.focus();
-      }
-    }
   };
 
   /**
@@ -138,19 +114,6 @@ class CDSSideNav extends HostListenerMixin(LitElement) {
     super.disconnectedCallback();
   }
 
-  shouldUpdate(changedProperties) {
-    if (changedProperties.has('expanded')) {
-      this._transitionPromise = new Promise((resolve) => {
-        this._cleanHTransition();
-        this._hTransition = on(this, 'transitionend', () => {
-          this._cleanHTransition();
-          resolve();
-        });
-      });
-    }
-    return true;
-  }
-
   updated(changedProperties) {
     const doc = this.getRootNode() as Document;
     if (changedProperties.has('collapseMode')) {
@@ -179,6 +142,11 @@ class CDSSideNav extends HostListenerMixin(LitElement) {
         forEach(headerItems, (item) => {
           item.setAttribute('tabindex', '-1');
         });
+        (
+          this.querySelector(
+            (this.constructor as typeof CDSSideNav).selectorNavItems
+          ) as HTMLElement
+        )?.focus();
       } else {
         forEach(headerItems, (item) => {
           item.removeAttribute('tabindex');

diff --git a/packages/web-components/src/components/masthead/left-nav-menu-item.ts b/packages/web-components/src/components/masthead/left-nav-menu-item.ts
@@ -60,7 +60,6 @@ class C4DLeftNavMenuItem extends CDSSideNavMenuItem {
     return href
       ? html`
           <a
-            tabindex="-1"
             part="link"
             class="${linkClasses}"
             href="${href}"

diff --git a/packages/web-components/src/components/masthead/left-nav-menu-section.ts b/packages/web-components/src/components/masthead/left-nav-menu-section.ts
@@ -15,7 +15,6 @@ import ChevronLeft16 from '../../internal/vendor/@carbon/web-components/icons/ch
 import FocusMixin from '../../internal/vendor/@carbon/web-components/globals/mixins/focus.js';
 import { selectorTabbable } from '../../internal/vendor/@carbon/web-components/globals/settings.js';
 import settings from '../../internal/vendor/@carbon/ibmdotcom-utilities/utilities/settings/settings';
-import { forEach } from '../../globals/internal/collection-helpers';
 import styles from './masthead.scss';
 import C4DLeftNav from './left-nav';
 import { carbonElement as customElement } from '../../internal/vendor/@carbon/web-components/globals/decorators/carbon-element.js';
@@ -177,36 +176,12 @@ class C4DLeftNavMenuSection extends HostListenerMixin(FocusMixin(LitElement)) {
   }
 
   async updated(changedProperties) {
-    // make sure leftNavMenuSection updates before setting the tabIndex's per item
     await this._requestLeftNavMenuSectionUpdate();
 
     if (changedProperties.has('expanded')) {
-      const { selectorNavMenu, selectorNavItem } = this
-        .constructor as typeof C4DLeftNavMenuSection;
       const { expanded, isSubmenu } = this;
 
       if (expanded) {
-        if (isSubmenu) {
-          const backBtn = this.shadowRoot?.querySelector('button');
-          if (backBtn) {
-            backBtn.tabIndex = 0;
-          }
-        }
-        forEach(this.querySelectorAll(selectorNavMenu), (elem) => {
-          const item = (elem as HTMLElement).shadowRoot?.querySelector(
-            'button'
-          );
-          if (item) {
-            item.tabIndex = 0;
-          }
-        });
-        forEach(this.querySelectorAll(selectorNavItem), (elem) => {
-          const item = (elem as HTMLElement).shadowRoot?.querySelector('a');
-          if (item) {
-            item.tabIndex = 0;
-          }
-        });
-
         // set focus to first element of menu panel to allow for tabbing through the menu
         let tabbable;
         if (isSubmenu) {
@@ -228,27 +203,6 @@ class C4DLeftNavMenuSection extends HostListenerMixin(FocusMixin(LitElement)) {
             { once: true }
           );
         }
-      } else {
-        forEach(this.querySelectorAll(selectorNavMenu), (elem) => {
-          const item = (elem as HTMLElement).shadowRoot?.querySelector(
-            'button'
-          );
-          if (item) {
-            item.tabIndex = -1;
-          }
-        });
-        forEach(this.querySelectorAll(selectorNavItem), (elem) => {
-          const item = (elem as HTMLElement).shadowRoot?.querySelector('a');
-          if (item) {
-            item.tabIndex = -1;
-          }
-        });
-        if (isSubmenu) {
-          const backBtn = this.shadowRoot?.querySelector('button');
-          if (backBtn) {
-            backBtn.tabIndex = -1;
-          }
-        }
       }
     }
   }
@@ -276,7 +230,6 @@ class C4DLeftNavMenuSection extends HostListenerMixin(FocusMixin(LitElement)) {
                 role="none">
                 <button
                   class="${prefix}--side-nav__link"
-                  tabindex="-1"
                   @click="${handleClickBack}">
                   <span class="${prefix}--side-nav__link-text"
                     >${ChevronLeft16()}${backButtonText}</span

diff --git a/packages/web-components/src/components/masthead/left-nav-menu.ts b/packages/web-components/src/components/masthead/left-nav-menu.ts
@@ -113,7 +113,6 @@ class C4DLeftNavMenu extends FocusMixin(LitElement) {
         <button
           type="button"
           aria-haspopup="true"
-          tabindex="-1"
           aria-expanded="${expanded}"
           class="${buttonClasses}"
           @click=${handleClickExpando}

diff --git a/packages/web-components/src/components/masthead/left-nav.ts b/packages/web-components/src/components/masthead/left-nav.ts
@@ -62,11 +62,31 @@ class C4DLeftNav extends StableSelectorMixin(CDSSideNav) {
   private _importedSideNav = false;
 
   /**
-   * Handles `c4d-request-focus-wrap` event on the document.
+   * Handles `${prefix}-header-menu-button-toggle` event on the document.
+   */
+  @HostListener('parentRoot:eventButtonToggle')
+  // @ts-ignore: The decorator refers to this method but TS thinks this method is not referred to
+  protected _handleButtonToggle = async (event: CustomEvent) => {
+    if (!this._importedSideNav) {
+      await Promise.all([
+        import('./left-nav-name'),
+        import('./left-nav-menu'),
+        import('./left-nav-menu-section'),
+        import('./left-nav-menu-item'),
+        import('./left-nav-menu-category-heading'),
+        import('./left-nav-overlay'),
+      ]);
+      this._importedSideNav = true;
+    }
+    this.expanded = event.detail.active;
+  };
+
+  /**
+   * Handles `cds-request-focus-wrap` event on the document dispatched from focuswrap.
    *
    * @param event The event.
    */
-  @HostListener('document:c4d-request-focus-wrap')
+  @HostListener('document:cds-request-focus-wrap')
   // @ts-ignore: The decorator refers to this method but TS thinks this method is not referred to
   private _handleRequestMenuButtonFocusWrap = (event: CustomEvent) => {
     const { selectorButtonToggle } = this.constructor as typeof C4DLeftNav;
@@ -221,7 +241,7 @@ class C4DLeftNav extends StableSelectorMixin(CDSSideNav) {
     document.addEventListener('click', this._handleClickOut.bind(this));
   }
 
-  updated(changedProperties) {
+  async updated(changedProperties) {
     super.updated(changedProperties);
     const { usageMode } = this;
     if (
@@ -256,15 +276,6 @@ class C4DLeftNav extends StableSelectorMixin(CDSSideNav) {
           ${c4dPrefix}-masthead-composite`
         )
         ?.querySelector(`${c4dPrefix}-masthead`);
-      if (expanded && !this._importedSideNav) {
-        import('./left-nav-name');
-        import('./left-nav-menu');
-        import('./left-nav-menu-section');
-        import('./left-nav-menu-item');
-        import('./left-nav-menu-category-heading');
-        import('./left-nav-overlay');
-        this._importedSideNav = true;
-      }
       if (expanded && masthead) {
         this._hFocusWrap = focuswrap(this.shadowRoot!, [
           startSentinelNode,
@@ -312,7 +323,7 @@ class C4DLeftNav extends StableSelectorMixin(CDSSideNav) {
     }
   }
 
-  private _renderSentinel = (side: String) => {
+  private _renderSentinel = (side: string) => {
     return html`
       <button
         id="${side}-sentinel"

diff --git a/packages/web-components/src/components/masthead/megamenu-top-nav-menu.ts b/packages/web-components/src/components/masthead/megamenu-top-nav-menu.ts
@@ -179,16 +179,18 @@ class C4DMegaMenuTopNavMenu extends C4DTopNavMenu {
       if (this.expanded) {
         // Import needed subcomponents on first expansion
         if (!(this.parentElement as C4DTopNav)?.importedMegamenu) {
-          await import('./megamenu-left-navigation');
-          await import('./megamenu-category-link');
-          await import('./megamenu-category-link-group');
-          await import('./megamenu-category-group');
-          await import('./megamenu-category-group-copy');
-          await import('./megamenu-category-heading');
-          await import('./megamenu-link-with-icon');
-          await import('./megamenu-overlay');
-          await import('./megamenu-tab');
-          await import('./megamenu-tabs');
+          await Promise.all([
+            import('./megamenu-left-navigation'),
+            import('./megamenu-category-link'),
+            import('./megamenu-category-link-group'),
+            import('./megamenu-category-group'),
+            import('./megamenu-category-group-copy'),
+            import('./megamenu-category-heading'),
+            import('./megamenu-link-with-icon'),
+            import('./megamenu-overlay'),
+            import('./megamenu-tab'),
+            import('./megamenu-tabs'),
+          ]);
           (this.parentElement as C4DTopNav).importedMegamenu = true;
         }
 

diff --git a/packages/web-components/tests/snapshots/c4d-left-nav-menu.md b/packages/web-components/tests/snapshots/c4d-left-nav-menu.md
@@ -13,7 +13,6 @@
     data-attribute1="headerNav"
     data-attribute2="L0"
     data-attribute3=""
-    tabindex="-1"
     type="button"
   >
     <div class="cds--side-nav__submenu-content">
@@ -38,7 +37,6 @@
     data-attribute1="headerNav"
     data-attribute2="L0"
     data-attribute3="title-foo"
-    tabindex="-1"
     type="button"
   >
     <div class="cds--side-nav__submenu-content">