Refactor stimulus nav controllers to follow stimulus best practices. (#…

…4422) Remove unused javascript from nav.
rubygems · Feb 19, 2024 · 11c620b · 11c620b
1 parent 551149a
commit 11c620b
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 207 deletions.
diff --git a/app/assets/stylesheets/modules/header.css b/app/assets/stylesheets/modules/header.css
@@ -185,25 +185,25 @@
   border-radius: 22px; }
 
 @media (min-width: 1020px) {
-  .header__popup__nav-links, .home__header__popup__nav-links {
+  .header__popup__nav-links.hidden {
     display: none; }
-    .header__popup__nav-links.is-expanded, .home__header__popup__nav-links.is-expanded {
-      margin-top: -7px;
-      display: block;
-      float: right;
-      position: absolute;
-      right: 0;
-      text-align: right; }
-    .header__popup__nav-links .header__nav-link, .home__header__popup__nav-links .header__nav-link {
-      margin-left: 0;
-      padding-top: 15px;
-      padding-right: 30px;
-      padding-bottom: 15px;
-      display: block;
-      width: 100%; } }
+  .header__popup__nav-links {
+    margin-top: -7px;
+    display: block;
+    float: right;
+    position: absolute;
+    right: 0;
+    text-align: right; }
+  .header__popup__nav-links .header__nav-link {
+    margin-left: 0;
+    padding-top: 15px;
+    padding-right: 30px;
+    padding-bottom: 15px;
+    display: block;
+    width: 100%; } }
 
 @media (min-width: 1020px) {
-  .header__popup__nav-links.is-expanded {
+  .header__popup__nav-links {
     width: 180px;
     border-bottom-right-radius: 3px;
     border-bottom-left-radius: 3px;
@@ -213,7 +213,7 @@
     border-bottom: 1px solid rgba(255, 255, 255, 0.3); } }
 
 @media (min-width: 1020px) {
-  .body--index .header__popup__nav-links.is-expanded {
+  .body--index .header__popup__nav-links {
     width: 150px;
     top: 0;
     padding-top: 74px;

diff --git a/app/javascript/controllers/dropdown_controller.js b/app/javascript/controllers/dropdown_controller.js
@@ -0,0 +1,18 @@
+import { Controller } from "@hotwired/stimulus"
+
+// I tested the stimulus-dropdown component but it has too many deps.
+// This mimics the basic stimulus-dropdown api, so we could swap it in later.
+export default class extends Controller {
+  static targets = ["menu"]
+
+  hide(e) {
+    !this.element.contains(e.target) &&
+      !this.menuTarget.classList.contains("hidden") &&
+      this.menuTarget.classList.add('hidden');
+  }
+
+  toggle(e) {
+    e.preventDefault();
+    this.menuTarget.classList.toggle('hidden');
+  }
+}
diff --git a/app/javascript/controllers/nav_controller.js b/app/javascript/controllers/nav_controller.js
@@ -2,76 +2,38 @@ import { Controller } from "@hotwired/stimulus"
 
 export default class extends Controller {
   static targets = [
-    "dropdownButton", // carrot icon in full size
-    "dropdown", // full size dropdown
-    "sandwichIcon", // mobile sandwich icon
-    "header", 
-    "main",
-    "footer",
-    "headerSearch",
-    "headerLogo"
+    "collapse", // targets that receive expanded class when mobile nav shows
+    "header", // target on which clicks don't hide mobile nav
+    "logo",
+    "search",
   ]
-
-  static mobileNavExpandedClass = 'mobile-nav-is-expanded';
-
-  connect() {
-    // variable to support mobile nav tab behaviour
-    // * skipSandwichIcon is for skipping sandwich icon
-    //   when you tab from "gem" icon
-    // * tabDirection is for hiding and showing navbar
-    //   when you tab in and out
-    this.skipSandwichIcon = true;
-
-    document.addEventListener("click", (e) => {
-      // Must check both dropdowns otherwise you always close
-      // the dropdown because one or the other isn't being clicked
-      if (!this.dropdownTarget.contains(e.target) && !this.headerTarget.contains(e.target)) {
-        this.dropdownTarget.classList.remove('is-expanded');
-        this.collapseMobileNav();
-      }
-    });
-  }
-
-  dropdownButtonTargetConnected(el) {
-    el.addEventListener("click", (e) => {
-      e.preventDefault();
-      this.dropdownTarget.classList.toggle('is-expanded');
-    });
+  static classes = ["expanded"]
+
+  connect() { this.skipSandwichIcon = true }
+
+  toggle(e) {
+    e.preventDefault();
+    if (this.collapseTarget.classList.contains(this.expandedClass)) {
+      this.leave()
+      this.logoTarget.focus();
+    } else {
+      this.enter()
+    }
   }
 
-  sandwichIconTargetConnected(el) {
-    el.addEventListener("click", (e) => {
-      e.preventDefault();
-
-      if (this.headerTarget.classList.contains(this.constructor.mobileNavExpandedClass)) {
-        this.collapseMobileNav();
-      } else {
-        this.expandMobileNav();
-      }
-    });
-
-    el.addEventListener('focusin', () => {
-      if (this.skipSandwichIcon) {
-        this.expandeMobileNav();
-        this.headerSearchTarget.focus();
-        this.skipSandwichIcon = false;
-      } else {
-        this.collapseMobileNav();
-        this.headerLogoTarget.focus();
-        this.skipSandwichIcon = true;
-      }
-    });
+  focus() {
+    if (this.skipSandwichIcon) { // skip sandwich icon when you tab from "gem" icon
+      this.enter();
+      this.hasSearchTarget && this.searchTarget.focus();
+      this.skipSandwichIcon = false;
+    } else {
+      this.leave();
+      this.logoTarget.focus();
+      this.skipSandwichIcon = true;
+    }
   }
 
-  collapseMobileNav() {
-    this.headerTarget.classList.remove(this.constructor.mobileNavExpandedClass);
-    this.mainTarget.classList.remove(this.constructor.mobileNavExpandedClass);
-    this.footerTarget.classList.remove(this.constructor.mobileNavExpandedClass);
-  }
-
-  expandMobileNav() {
-    this.headerTarget.classList.add(this.constructor.mobileNavExpandedClass);
-    this.mainTarget.classList.add(this.constructor.mobileNavExpandedClass);
-    this.footerTarget.classList.add(this.constructor.mobileNavExpandedClass);
-  }
+  hide(e) { !this.headerTarget.contains(e.target) && this.leave() }
+  leave() { this.collapseTargets.forEach(el => el.classList.remove(this.expandedClass)) }
+  enter() { this.collapseTargets.forEach(el => el.classList.add(this.expandedClass)) }
 }
diff --git a/app/javascript/src/handle-click.js b/app/javascript/src/handle-click.js
diff --git a/app/javascript/src/mobile-nav.js b/app/javascript/src/mobile-nav.js
diff --git a/app/javascript/src/popup-nav.js b/app/javascript/src/popup-nav.js
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
@@ -24,34 +24,36 @@
     <%= javascript_importmap_tags %>
   </head>
 
-  <body class="<%= 'body--index' if request.path_info == '/' %>" data-controller="nav">
+  <body class="<%= 'body--index' if request.path_info == '/' %>" data-controller="nav" data-nav-expanded-class="mobile-nav-is-expanded">
     <!-- Top banner -->
     <% if content_for?(:banner) %>
       <div class="banner">
         <%= yield :banner %>
       </div>
     <% end %>
-    <header class="header <%= 'header--interior' if request.path_info != '/' %>" data-nav-target="header">
+    <header class="header <%= 'header--interior' if request.path_info != '/' %>" data-nav-target="header collapse">
       <div class="l-wrap--header">
-        <%= link_to(root_path, title: 'RubyGems', class: 'header__logo-wrap', data: { nav_target: "headerLogo" }) do %>
+        <%= link_to(root_path, title: 'RubyGems', class: 'header__logo-wrap', data: { nav_target: "logo" }) do %>
           <span class="header__logo" data-icon="⬡">⬢</span>
           <span class="t-hidden">RubyGems</span>
         <% end %>
-        <a class="header__club-sandwich" href="#" data-nav-target="sandwichIcon">
+        <a class="header__club-sandwich" href="#" data-action="nav#toggle focusin->nav#focus click@window->nav#hide">
           <span class="t-hidden">Navigation menu</span>
         </a>
 
         <div class="header__nav-links-wrap">
-          <%= form_tag search_path, class: search_form_class, method: :get do %>
-            <%= search_field_tag :query, params[:query], placeholder: t(".header.search_gem_html"), class: "header__search", autocomplete: "off", data: { nav_target: "headerSearch" } %>
-            <ul id="suggest" class="suggest-list"></ul>
-            <%= label_tag :query do %>
-              <span class="t-hidden"><%= t(".header.search_gem_html") %></span>
+          <% if request.path_info != "/" %>
+            <%= form_tag search_path, class: search_form_class, method: :get do %>
+              <%= search_field_tag :query, params[:query], placeholder: t(".header.search_gem_html"), class: "header__search", autocomplete: "off", data: { nav_target: "search" } %>
+              <ul id="suggest" class="suggest-list"></ul>
+              <%= label_tag :query do %>
+                <span class="t-hidden"><%= t(".header.search_gem_html") %></span>
+              <% end %>
+              <%= submit_tag "⌕", id: "search_submit", name: nil, class: "header__search__icon" %>
             <% end %>
-            <%= submit_tag "⌕", id: "search_submit", name: nil, class: "header__search__icon" %>
           <% end %>
 
-          <nav class="header__nav-links">
+          <nav class="header__nav-links" data-controller="dropdown">
             <% if signed_in? %>
               <a href="<%= profile_path(current_user.display_id) %>" class="header__nav-link mobile__header__nav-link">
                 <span><%= current_user.name %></span>
@@ -75,10 +77,10 @@
                 <span><%= current_user.name %></span>
                 <%= avatar 80, "user_gravatar", theme: :dark %>
               </a>
-              <a href="#" class="header__popup-link" data-icon="▼" data-nav-target="dropdownButton">
+              <a href="#" class="header__popup-link" data-icon="▼" data-action="dropdown#toggle click@window->dropdown#hide">
                 <span class="t-hidden">More items</span>
               </a>
-              <div class="header__popup__nav-links" data-nav-target="dropdown">
+              <div class="header__popup__nav-links hidden" data-dropdown-target="menu" >
                 <%= link_to t('.header.settings'), edit_settings_path, class: "header__nav-link" %>
                 <%= link_to t('.header.dashboard'), dashboard_path, class: "header__nav-link" %>
                 <%= link_to t('.header.edit_profile'), edit_profile_path, class: "header__nav-link" %>
@@ -109,7 +111,7 @@
       </div>
     <% end %>
 
-    <main class="<%= 'main--interior' if request.path_info != '/' %>" data-nav-target="main">
+    <main class="<%= 'main--interior' if request.path_info != '/' %>" data-nav-target="collapse">
       <% if request.path_info != '/' %>
         <div class="l-wrap--b">
           <% if @title %>
@@ -129,7 +131,7 @@
       <% end %>
     </main>
 
-    <footer class="footer" data-nav-target="footer">
+    <footer class="footer" data-nav-target="collapse">
       <div class="l-wrap--footer">
         <div class="l-overflow">
           <div class="nav--v l-col--r--pad">