Logged-in keyboard navigation fixes

Author: nsharma123

packages/ia-topnav/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@internetarchive/ia-topnav",
-  "version": "1.3.22",
+  "version": "1.3.23-alpha1",
   "description": "Top nav for Internet Archive",
   "license": "AGPL-3.0-only",
   "main": "dist/index.js",

packages/ia-topnav/src/dropdown-menu.ts

@@ -1,4 +1,4 @@
-import { CSSResult, html, nothing, TemplateResult } from 'lit';
+import { CSSResult, html, nothing, PropertyValues, TemplateResult } from 'lit';
 import { property } from 'lit/decorators.js';
 
 import icons from './assets/img/icons';
@@ -9,6 +9,7 @@ import { IATopNavConfig, IATopNavLink } from './models';
 import dropdownMenuCSS from './styles/dropdown-menu';
 import TrackedElement from './tracked-element';
 import { ifDefined } from 'lit/directives/if-defined.js';
+import KeyboardNavigation from './lib/keyboard-navigation';
 
 export default class DropdownMenu extends TrackedElement {
   @property({ type: String }) baseHost = '';
@@ -18,10 +19,33 @@ export default class DropdownMenu extends TrackedElement {
   @property({ type: Boolean }) animated = false;
   @property({ type: Boolean }) open = false;
 
+  private previousKeydownListener?: // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  (this: HTMLElement, ev: KeyboardEvent) => any;
+
   static get styles(): CSSResult[] {
     return [dropdownMenuCSS];
   }
 
+  updated(props: PropertyValues) {
+    if (props.has('open') && this.open) {
+      const container = this.shadowRoot?.querySelector(
+        '.nav-container',
+      ) as HTMLElement;
+
+      if (container) {
+        const keyboardNavigation = new KeyboardNavigation(
+          container,
+          'usermenu',
+        );
+        this.addEventListener('keydown', keyboardNavigation.handleKeyDown);
+        if (this.previousKeydownListener) {
+          this.removeEventListener('keydown', this.previousKeydownListener);
+        }
+        this.previousKeydownListener = keyboardNavigation.handleKeyDown;
+      }
+    }
+  }
+
   get dropdownItems() {
     if (!this.menuItems) return nothing;
 
@@ -56,17 +80,19 @@ export default class DropdownMenu extends TrackedElement {
 
   dropdownLink(link: IATopNavLink): TemplateResult {
     const calloutText = this.config?.callouts?.[link.title];
+    const isMobileUpload = link.class === 'mobile-upload';
+    const isTabbable = this.open && !isMobileUpload;
+
     return html`<a
       href="${formatUrl(link.url, this.baseHost)}"
       class=${ifDefined(link.class)}
-      tabindex="${this.open ? '' : '-1'}"
+      tabindex="${isTabbable ? '' : '-1'}"
       @click=${this.trackClick}
       data-event-click-tracking="${this.config
         ?.eventCategory}|Nav${link.analyticsEvent}"
       aria-label=${calloutText ? `New feature: ${link.title}` : nothing}
     >
-      ${link.class === 'mobile-upload' ? icons.uploadUnpadded : nothing}
-      ${link.title}
+      ${isMobileUpload ? icons.uploadUnpadded : nothing} ${link.title}
       ${calloutText
         ? html`<span class="callout" aria-hidden="true">${calloutText}</span>`
         : nothing}

packages/ia-topnav/src/ia-topnav.ts

@@ -258,6 +258,9 @@ export class IATopNav extends LitElement {
         ?hideSearch=${this.hideSearch}
         tabindex="${this.signedOutTabIndex}"
         .menuItems=${this.signedOutMenuItems}
+        @focusToOtherMenuItem=${(e: CustomEvent) => {
+          this.currentTab = e.detail;
+        }}
       ></signed-out-dropdown>
     `;
   }

packages/ia-topnav/src/lib/keyboard-navigation.ts

@@ -13,31 +13,25 @@ export default class KeyboardNavigation {
     this.elementsContainer = elementsContainer;
     this.menuOption = menuOption;
     this.focusableElements = this.getFocusableElements();
-    this.focusedIndex = this.getInitialFocusedIndex();
+    this.focusedIndex = 0; // always start from first element
 
     if (menuOption !== 'search') {
       this.focusableElements[this.focusedIndex]?.focus();
     }
     this.handleKeyDown = this.handleKeyDown.bind(this);
   }
 
-  /**
-   * Returns the initial focused index based on the menu option.
-   * @returns {number} The initial focused index (0 for 'web', 1 for 'usermenu').
-   */
-  getInitialFocusedIndex(): number {
-    return this.menuOption === 'usermenu' ? 1 : 0;
-  }
-
   /**
    * Gets an array of focusable elements within the container.
    * @returns {HTMLElement[]} An array of focusable elements.
    */
   getFocusableElements(): HTMLElement[] {
-    const focusableTagSelectors =
-      'a[href], button, input, [tabindex]:not([tabindex="-1"])';
-    const isDisabledOrHidden = (el: Element) =>
-      !el.hasAttribute('disabled') && !el.getAttribute('aria-hidden');
+    const focusableTagSelectors = 'a[href], button, input, [tabindex]';
+
+    const isFocusable = (el: Element) =>
+      !el.hasAttribute('disabled') &&
+      el.getAttribute('aria-hidden') !== 'true' &&
+      el.getAttribute('tabindex') !== '-1';
 
     let elements;
     if (this.menuOption === 'web') {
@@ -69,9 +63,7 @@ export default class KeyboardNavigation {
       elements = this.elementsContainer.querySelectorAll(focusableTagSelectors);
     }
 
-    return Array.from(elements ?? []).filter(
-      isDisabledOrHidden,
-    ) as HTMLElement[];
+    return Array.from(elements ?? []).filter(isFocusable) as HTMLElement[];
   }
 
   /**

packages/ia-topnav/src/login-button.ts

@@ -3,6 +3,7 @@ import TrackedElement from './tracked-element';
 import icons from './assets/img/icons';
 import loginButtonCSS from './styles/login-button';
 import formatUrl from './lib/format-url';
+import { makeBooleanString } from './lib/make-boolean-string';
 import { customElement, property, state } from 'lit/decorators.js';
 import { IATopNavConfig } from './models';
 import { defaultTopNavConfig } from './data/menus';
@@ -31,7 +32,7 @@ export class LoginButton extends TrackedElement {
     return `${this.config?.eventCategory}|NavLoginIcon`;
   }
 
-  get menuOpened() {
+  get menuOpened(): boolean {
     return this.openMenu === 'login';
   }
 
@@ -57,13 +58,15 @@ export class LoginButton extends TrackedElement {
   render() {
     return html`
       <div class="logged-out-toolbar">
-        <a
-          class="${this.avatarClass}"
+        <button
+          class="logged-out-menu ${this.avatarClass}"
           @click=${this.toggleDropdown}
           data-event-click-tracking="${this.analyticsEvent}"
+          aria-label="Toggle login menu"
+          aria-expanded="${makeBooleanString(this.menuOpened)}"
         >
           ${icons.user}
-        </a>
+        </button>
         <span>
           <a href="${this.signupPath}">Sign up</a>
           |

packages/ia-topnav/src/primary-nav.ts

@@ -93,10 +93,20 @@ export class PrimaryNav extends TrackedElement {
             ?.classList.contains('images');
         });
 
+        let nextElement;
+        if (this.username) {
+          nextElement = this.shadowRoot?.querySelector('a.upload');
+        } else {
+          nextElement = this.shadowRoot
+            ?.querySelector('login-button')
+            ?.shadowRoot?.querySelector('span a');
+        }
+
+        const menuItemElement =
+          lastMediaButton[0]?.shadowRoot?.querySelector('a.menu-item');
+
         const focusElement =
-          this.currentTab.moveTo === 'next'
-            ? this.shadowRoot?.querySelector('a.upload')
-            : lastMediaButton[0]?.shadowRoot?.querySelector('a.menu-item');
+          this.currentTab.moveTo === 'next' ? nextElement : menuItemElement;
 
         if (focusElement) {
           (focusElement as HTMLElement).focus();

packages/ia-topnav/src/styles/login-button.ts

@@ -1,6 +1,14 @@
 import { css } from 'lit';
 
 export default css`
+  .logged-out-menu {
+    background: inherit;
+    border: none;
+  }
+  .logged-out-menu:focus-visible {
+    outline: none;
+    border: none;
+  }
   .dropdown-toggle {
     display: block;
     text-transform: uppercase;

packages/ia-topnav/src/styles/primary-nav.ts

@@ -304,7 +304,8 @@ export default css`
     }
 
     .upload span {
-      display: inline;
+      display: inline-block;
+      vertical-align: middle;
     }
   }
 `;

packages/ia-topnav/src/user-menu.ts

@@ -1,42 +1,18 @@
-import { CSSResult, html, PropertyValues } from 'lit';
+import { CSSResult, html } from 'lit';
 import DropdownMenu from './dropdown-menu';
 import userMenuCSS from './styles/user-menu';
 import dropdownStyles from './styles/dropdown-menu';
-import KeyboardNavigation from './lib/keyboard-navigation';
 import { customElement, property } from 'lit/decorators.js';
 
 @customElement('user-menu')
 export default class UserMenu extends DropdownMenu {
   @property({ type: String }) username = '';
   @property({ type: String }) screenName = '';
 
-  private previousKeydownListener?: // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  (this: HTMLElement, ev: KeyboardEvent) => any;
-
   static get styles(): CSSResult[] {
     return [dropdownStyles, userMenuCSS];
   }
 
-  updated(props: PropertyValues) {
-    if (props.has('open') && this.open) {
-      const container = this.shadowRoot?.querySelector(
-        '.nav-container',
-      ) as HTMLElement;
-
-      if (container) {
-        const keyboardNavigation = new KeyboardNavigation(
-          container,
-          'usermenu',
-        );
-        this.addEventListener('keydown', keyboardNavigation.handleKeyDown);
-        if (this.previousKeydownListener) {
-          this.removeEventListener('keydown', this.previousKeydownListener);
-        }
-        this.previousKeydownListener = keyboardNavigation.handleKeyDown;
-      }
-    }
-  }
-
   render() {
     return html`
       <div class="nav-container">