-1

I have a NavBar class that creates a navigation bar and then a function within the class that adds a click event listener to each link.

Obviously I can't add the click event listener to the element until it has rendered in the DOM and I am currently using a DOMContentLoaded event listener in the render function in order to wait before firing off the assignNavLinkHandlers function.

Is using the DOMContentLoaded event listener here best practice, or is there a more elegant/practical solution?

Below is the code which implements the above:

import { links } from "../../constants/navigationLinks.js";
import { initRouter, navigateTo } from "../../utils/router.js";

export default class NavBar {
  constructor() {
    this.navLinks = links;
  }

  assignNavLinkHandlers() {
    Object.values(this.navLinks).forEach((link) => {
      const { navLink, pageName } = link;
      const assignedLink = document.getElementById(navLink);
      assignedLink.addEventListener("click", async () => {
        pageName === "home" ? navigateTo("/") : navigateTo(`/${pageName}`);
      });
    });
  }

  startRouter() {
    initRouter();
  }

  render() {
    const navBar = document.createElement("nav");
    navBar.classList.add("navbar");

    const navList = document.createElement("ul");
    navList.classList.add("nav-list");

    for (const link in this.navLinks) {
      const navItem = document.createElement("li");
      navItem.classList.add("nav-item");

      const navLink = document.createElement("a");
      navLink.classList.add("nav-link");
      navLink.id = this.navLinks[link].navLink;
      navLink.textContent = this.navLinks[link].label;

      navItem.appendChild(navLink);
      navList.appendChild(navItem);
    }

    navBar.appendChild(navList);

    document.addEventListener("DOMContentLoaded", () => {
      this.assignNavLinkHandlers();
      this.startRouter();
    });

    return navBar;
  }
}
  • 1
    Yup, it's the best. – Kevin M. Mansour Jun 28 '23 at 13:08
  • Does this answer your question? [window.onload vs document.onload](https://stackoverflow.com/questions/588040/window-onload-vs-document-onload) – tom Jun 28 '23 at 13:08
  • 2
    Instead of adding listeners to all the items you could [use event delegation](https://dmitripavlutin.com/javascript-event-delegation/) and add one to the list element to catch events from its children as they bubble up the DOM. – Andy Jun 28 '23 at 13:12
  • @ztom That link is not relevant to the question being asked here. – Scott Marcus Jun 28 '23 at 13:39

1 Answers1

1

What you're doing is fine, however you can avoid setting up that event handler (and save a tiny amount of memory for the document object) by just placing your <script> element just prior to the closing body tag. By the time the HTML parser reaches that point, all the HTML in the body will have been parsed and available in the DOM.

Scott Marcus
  • 64,069
  • 6
  • 49
  • 71