PNI - Search JS code improvements

source/js/buyers-guide/search/member-functions.js

@@ -156,43 +156,42 @@ export function setupGoBackToAll(instance) {
 
 /**
  * Attach event listeners to the Product Review nav links
- *
- * @param {*} instance
- *
- * @todo See if we can make this DRYer by making use of the toggleProductReviewView function?
  */
-export function setupReviewLinks(instance) {
+export function setupReviewLinks() {
   const navLinks = document.querySelectorAll(`.product-review-link`);
 
   if (!navLinks) return;
 
   for (const nav of navLinks) {
     nav.addEventListener("click", (evt) => {
-      const editorialContent = document.querySelector(".editorial-content");
+      evt.preventDefault();
+      evt.stopPropagation();
+
       const burger = document.querySelector(".burger");
-      if (editorialContent) {
-        evt.preventDefault();
-        evt.stopPropagation();
-        nav.classList.add("active");
-        location.hash = "product-review";
-        editorialContent.classList.add("tw-hidden");
-        if (burger && burger.classList.contains("menu-open")) {
-          document.querySelector(".burger").click();
-        }
+      const hash = nav.hash;
+
+      location.hash = hash;
+      showProductReviewView();
+
+      // on mobile, clicking the .product-review-link link should also close the menu screen
+      if (burger && burger.classList.contains("menu-open")) {
+        document.querySelector(".burger").click();
       }
     });
   }
 }
 
 /**
- * Toggle the Product Review view
+ * Show the Product Review view
  */
-export function toggleProductReviewView() {
+export function showProductReviewView() {
   const editorialContent = document.querySelector(".editorial-content");
   const navLinks = document.querySelectorAll(`.product-review-link`);
+
   if (editorialContent) {
     editorialContent.classList.add("tw-hidden");
   }
+
   for (const nav of navLinks) {
     nav.classList.add("active");
   }

source/js/buyers-guide/search/pni-sort-dropdown.js

@@ -164,28 +164,55 @@ export class PNISortDropdown {
   /**
    * Focus on the next list item based on the given direction.
    *
-   * @param {number} direction - They key code of the direction to move the focus (e.g., key code for up arrow / down arrow)
-   *
-   * @todo Consider refactoring this method to make it more readable and maintainable.
+   * @param {number} direction The key code of the direction to move the focus (e.g., key code for up arrow / down arrow)
    */
   focusNextListItem(direction) {
     const activeElementId = document.activeElement.id;
-    const currentActiveElementIndex = this.listItemIds.indexOf(activeElementId);
+    const currentIndex = this.listItemIds.indexOf(activeElementId);
+
+    // return early if active element is not in the list
+    if (currentIndex < 0) return;
+
+    // handle focus based on direction
     if (direction === DOWN_ARROW_KEY_CODE) {
-      const currentActiveElementIsNotLastItem =
-        currentActiveElementIndex < this.listItemIds.length - 1;
-      if (currentActiveElementIsNotLastItem) {
-        const nextListItemId = this.listItemIds[currentActiveElementIndex + 1];
-        document.querySelector(`#${nextListItemId}`).focus();
-      }
+      this.focusNextItem(currentIndex);
     } else if (direction === UP_ARROW_KEY_CODE) {
-      const currentActiveElementIsNotFirstItem = currentActiveElementIndex > 0;
-      if (currentActiveElementIsNotFirstItem) {
-        const nextListItemId = this.listItemIds[currentActiveElementIndex - 1];
-        document.querySelector(`#${nextListItemId}`).focus();
-      } else {
-        this.dropdownButton.focus();
-      }
+      this.focusPreviousItem(currentIndex);
+    }
+  }
+
+  /**
+   * Focus the next list item if it's not the last one.
+   *
+   * @param {number} currentIndex The index of the current active element in the list
+   */
+  focusNextItem(currentIndex) {
+    const isNotLastItem = currentIndex < this.listItemIds.length - 1;
+    if (isNotLastItem) {
+      this.focusItem(currentIndex + 1);
     }
   }
+
+  /**
+   * Focus the previous list item if it's not the first, or focus the dropdown button.
+   *
+   * @param {number} currentIndex The index of the current active element in the list
+   */
+  focusPreviousItem(currentIndex) {
+    if (currentIndex > 0) {
+      this.focusItem(currentIndex - 1);
+    } else {
+      this.dropdownButton.focus();
+    }
+  }
+
+  /**
+   * Focus the list item at the specified index.
+   *
+   * @param {number} index The index of the item to focus
+   */
+  focusItem(index) {
+    const nextListItemId = this.listItemIds[index];
+    document.querySelector(`#${nextListItemId}`).focus();
+  }
 }

source/js/buyers-guide/search/search-filter.js

@@ -7,7 +7,7 @@ import {
   setupNavLinks,
   setupGoBackToAll,
   setupReviewLinks,
-  toggleProductReviewView,
+  showProductReviewView,
   toggleCategoryRelatedArticles,
 } from "./member-functions.js";
 
@@ -44,13 +44,13 @@ export class SearchFilter {
       mobileSearchBar,
       mobileSearchInput
     );
-    setupReviewLinks(this);
+    setupReviewLinks();
 
     if (
       (location.hash && location.hash === "#product-review") ||
       location.pathname.includes("categories")
     ) {
-      toggleProductReviewView();
+      showProductReviewView();
     }
 
     const subContainer = document.querySelector(`.subcategory-header`);
@@ -240,7 +240,7 @@ export class SearchFilter {
     document
       .querySelector("#pni-category-dropdown-select")
       .classList.add("tw-hidden");
-    toggleProductReviewView();
+    showProductReviewView();
     toggleCategoryRelatedArticles(category);
     Utils.filterProductsByCategory(category);
     Utils.toggleCtaForCategory(category);

source/js/buyers-guide/search/utils.js

@@ -32,43 +32,43 @@ export class Utils {
   /**
    * Update page header to the category passed in the argument
    *
-   * @param {*} category
-   * @param {*} parent
-   *
-   * @todo Improve the implementation to increase code readibility
+   * @param {String} category name of the category
+   * @param {String} parent name of the parent category
    */
   static updateHeader(category, parent) {
-    const headerText = document.querySelector(".category-header");
-
-    if (parent) {
-      document.querySelector(".category-header").dataset.name = parent;
-      headerText.textContent = parent;
-      if (document.querySelector(`#multipage-nav a[data-name="${parent}"]`)) {
-        document.querySelector(".category-header").href =
-          document.querySelector(
-            `#multipage-nav a[data-name="${parent}"]`
-          ).href;
-      }
-      document.querySelector(
-        `#pni-mobile-category-nav .active-link-label`
-      ).textContent = parent;
-    } else {
-      const header = category === "None" ? ALL_CATEGORY_LABEL : category;
-      headerText.textContent = header;
-      document.querySelector(".category-header").dataset.name = category;
-      if (document.querySelector(`#multipage-nav a[data-name="${category}"]`)) {
-        document.querySelector(".category-header").href =
-          document.querySelector(
-            `#multipage-nav a[data-name="${category}"]`
-          ).href;
-      }
-      document.querySelector(
-        `#pni-mobile-category-nav .active-link-label`
-      ).textContent =
-        category === "None"
-          ? document.querySelector(`#multipage-nav a[data-name="None"]`)
-              .textContent
-          : category;
+    const datasetName =
+      parent || (category === "None" ? ALL_CATEGORY_LABEL : category);
+    const headerText = parent || category;
+
+    this.setCategoryHeader(datasetName, headerText);
+  }
+
+  /**
+   * Set category header text in the DOM
+   *
+   * @param {String} datasetName The name to set in the header's data attribute
+   * @param {String} headerText The text content to show as the category header
+   */
+  static setCategoryHeader(datasetName, headerText) {
+    const categoryHeader = document.querySelector(".category-header");
+    const navLink = document.querySelector(
+      `#multipage-nav a[data-name="${headerText}"]`
+    );
+    const mobileActiveLink = document.querySelector(
+      `#pni-mobile-category-nav .active-link-label`
+    );
+
+    if (!categoryHeader) return;
+
+    categoryHeader.dataset.name = datasetName;
+    categoryHeader.textContent = headerText;
+
+    if (navLink) {
+      categoryHeader.href = navLink.href;
+    }
+
+    if (mobileActiveLink) {
+      mobileActiveLink.textContent = datasetName;
     }
   }