Issue #3191077 by mherchel, ranjith_kumar_k_u, proeung, Gauravmahlawat, andrewmacpherson, alexpott: Olivero narrow/mobile menu constrains tabbing in one direction only

core/tests/Drupal/Nightwatch/Tests/oliveroMobileMenuTest.js

@@ -3,6 +3,33 @@ const headerNavSelector = '#header-nav';
 const linkSubMenuId = 'home-submenu-1';
 const buttonSubMenuId = 'button-submenu-2';
 
+/**
+ * Sends arbitrary number of tab keys, and then checks that the last focused
+ * element is within the given parent selector.
+ *
+ * @param {object} browser - Nightwatch Browser object
+ * @param {string} parentSelector - Selector to which to test focused element against.
+ * @param {number} tabCount - Amount of tab presses to send to browser
+ * @param {boolean} [tabBackwards] - Hold down the SHIFT key when sending tabs
+ */
+const focusTrapCheck = (browser, parentSelector, tabCount, tabBackwards) => {
+  if (tabBackwards === true) browser.keys(browser.Keys.SHIFT);
+  for (let i = 0; i < tabCount; i++) {
+    browser.keys(browser.Keys.TAB).pause(50);
+  }
+  browser.execute(
+    // eslint-disable-next-line func-names, prefer-arrow-callback, no-shadow
+    function (parentSelector) {
+      // Verify focused element is still within the focus trap.
+      return document.activeElement.matches(parentSelector);
+    },
+    [parentSelector],
+    (result) => {
+      browser.assert.ok(result.value);
+    },
+  );
+};
+
 module.exports = {
   '@tags': ['core', 'olivero'],
   before(browser) {
@@ -55,26 +82,16 @@ module.exports = {
   },
   'Verify mobile menu focus trap': (browser) => {
     browser.drupalRelativeURL('/').click(mobileNavButtonSelector);
-    // Send the tab key 17 times.
-    // @todo test shift+tab functionality when
-    // https://www.drupal.org/project/drupal/issues/3191077 is committed.
-    for (let i = 0; i < 17; i++) {
-      browser.keys(browser.Keys.TAB).pause(50);
-    }
-
-    // Ensure that focus trap keeps focused element within the navigation.
-    browser.execute(
-      // eslint-disable-next-line func-names, prefer-arrow-callback, no-shadow
-      function (mobileNavButtonSelector, headerNavSelector) {
-        // Verify focused element is still within the focus trap.
-        return document.activeElement.matches(
-          `${headerNavSelector} *, ${mobileNavButtonSelector}`,
-        );
-      },
-      [mobileNavButtonSelector, headerNavSelector],
-      (result) => {
-        browser.assert.ok(result.value);
-      },
+    focusTrapCheck(
+      browser,
+      `${headerNavSelector} *, ${mobileNavButtonSelector}`,
+      17,
+    );
+    focusTrapCheck(
+      browser,
+      `${headerNavSelector} *, ${mobileNavButtonSelector}`,
+      19,
+      true,
     );
   },
 };

core/themes/olivero/js/navigation.es6.js

@@ -1,4 +1,4 @@
-((Drupal, once) => {
+((Drupal, once, tabbable) => {
   /**
    * Checks if navWrapper contains "is-active" class.
    * @param {object} navWrapper
@@ -64,22 +64,30 @@
       toggleNav(props, false);
     });
 
-    // Focus trap.
-    props.navWrapper.addEventListener('keydown', (e) => {
-      if (e.key === 'Tab') {
+    // Focus trap. This is added to the header element because the navButton
+    // element is not a child element of the navWrapper element, and the keydown
+    // event would not fire if focus is on the navButton element.
+    props.header.addEventListener('keydown', (e) => {
+      if (e.key === 'Tab' && isNavOpen(props.navWrapper)) {
+        const tabbableNavElements = tabbable.tabbable(props.navWrapper);
+        tabbableNavElements.unshift(props.navButton);
+        const firstTabbableEl = tabbableNavElements[0];
+        const lastTabbableEl =
+          tabbableNavElements[tabbableNavElements.length - 1];
+
         if (e.shiftKey) {
           if (
-            document.activeElement === props.firstFocusableEl &&
+            document.activeElement === firstTabbableEl &&
             !props.olivero.isDesktopNav()
           ) {
-            props.navButton.focus();
+            lastTabbableEl.focus();
             e.preventDefault();
           }
         } else if (
-          document.activeElement === props.lastFocusableEl &&
+          document.activeElement === lastTabbableEl &&
           !props.olivero.isDesktopNav()
         ) {
-          props.navButton.focus();
+          firstTabbableEl.focus();
           e.preventDefault();
         }
       }
@@ -104,36 +112,31 @@
    */
   Drupal.behaviors.oliveroNavigation = {
     attach(context) {
-      const navWrapperId = 'header-nav';
-      const navWrapper = once(
+      const headerId = 'header';
+      const header = once(
         'olivero-navigation',
-        `#${navWrapperId}`,
+        `#${headerId}`,
         context,
       ).shift();
+      const navWrapperId = 'header-nav';
 
-      if (navWrapper) {
+      if (header) {
+        const navWrapper = header.querySelector('#header-nav');
         const { olivero } = Drupal;
         const navButton = context.querySelector('.mobile-nav-button');
         const body = context.querySelector('body');
         const overlay = context.querySelector('.overlay');
-        const focusableNavElements = navWrapper.querySelectorAll(
-          'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])',
-        );
-        const firstFocusableEl = focusableNavElements[0];
-        const lastFocusableEl =
-          focusableNavElements[focusableNavElements.length - 1];
 
         init({
           olivero,
+          header,
           navWrapperId,
           navWrapper,
           navButton,
           body,
           overlay,
-          firstFocusableEl,
-          lastFocusableEl,
         });
       }
     },
   };
-})(Drupal, once);
+})(Drupal, once, tabbable);

core/themes/olivero/js/navigation.js

@@ -5,7 +5,7 @@
 * @preserve
 **/
 
-(function (Drupal, once) {
+(function (Drupal, once, tabbable) {
   function isNavOpen(navWrapper) {
     return navWrapper.classList.contains('is-active');
   }
@@ -46,15 +46,20 @@
     props.overlay.addEventListener('touchstart', function () {
       toggleNav(props, false);
     });
-    props.navWrapper.addEventListener('keydown', function (e) {
-      if (e.key === 'Tab') {
+    props.header.addEventListener('keydown', function (e) {
+      if (e.key === 'Tab' && isNavOpen(props.navWrapper)) {
+        var tabbableNavElements = tabbable.tabbable(props.navWrapper);
+        tabbableNavElements.unshift(props.navButton);
+        var firstTabbableEl = tabbableNavElements[0];
+        var lastTabbableEl = tabbableNavElements[tabbableNavElements.length - 1];
+
         if (e.shiftKey) {
-          if (document.activeElement === props.firstFocusableEl && !props.olivero.isDesktopNav()) {
-            props.navButton.focus();
+          if (document.activeElement === firstTabbableEl && !props.olivero.isDesktopNav()) {
+            lastTabbableEl.focus();
             e.preventDefault();
           }
-        } else if (document.activeElement === props.lastFocusableEl && !props.olivero.isDesktopNav()) {
-          props.navButton.focus();
+        } else if (document.activeElement === lastTabbableEl && !props.olivero.isDesktopNav()) {
+          firstTabbableEl.focus();
           e.preventDefault();
         }
       }
@@ -72,28 +77,26 @@
 
   Drupal.behaviors.oliveroNavigation = {
     attach: function attach(context) {
+      var headerId = 'header';
+      var header = once('olivero-navigation', "#".concat(headerId), context).shift();
       var navWrapperId = 'header-nav';
-      var navWrapper = once('olivero-navigation', "#".concat(navWrapperId), context).shift();
 
-      if (navWrapper) {
+      if (header) {
+        var navWrapper = header.querySelector('#header-nav');
         var olivero = Drupal.olivero;
         var navButton = context.querySelector('.mobile-nav-button');
         var body = context.querySelector('body');
         var overlay = context.querySelector('.overlay');
-        var focusableNavElements = navWrapper.querySelectorAll('button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])');
-        var firstFocusableEl = focusableNavElements[0];
-        var lastFocusableEl = focusableNavElements[focusableNavElements.length - 1];
         init({
           olivero: olivero,
+          header: header,
           navWrapperId: navWrapperId,
           navWrapper: navWrapper,
           navButton: navButton,
           body: body,
-          overlay: overlay,
-          firstFocusableEl: firstFocusableEl,
-          lastFocusableEl: lastFocusableEl
+          overlay: overlay
         });
       }
     }
   };
-})(Drupal, once);
+})(Drupal, once, tabbable);

core/themes/olivero/olivero.libraries.yml

@@ -65,6 +65,7 @@ global-styling:
     - core/drupal.nodelist.foreach
     - core/drupal
     - core/once
+    - core/tabbable
 
 book:
   css: