Async Hamburger Menu Implementation + Caching Optimizations (#18818)

app/controllers/admin/navigation_links_controller.rb

@@ -1,6 +1,8 @@
 module Admin
   class NavigationLinksController < Admin::ApplicationController
     after_action :bust_content_change_caches, only: %i[create update destroy]
+    after_action :bust_navigation_links_cache, only: %i[create update destroy]
+
     ALLOWED_PARAMS = %i[
       name url icon display_to position section
     ].freeze
@@ -52,5 +54,10 @@ module Admin
     def navigation_link_params
       params.require(:navigation_link).permit(ALLOWED_PARAMS)
     end
+
+    def bust_navigation_links_cache
+      Rails.cache.delete("navigation_links")
+      EdgeCache::Bust.call("/async_info/navigation_links")
+    end
   end
 end

app/controllers/async_info_controller.rb

@@ -1,6 +1,7 @@
 # @note No pundit policy. All actions are unrestricted.
 class AsyncInfoController < ApplicationController
   NUMBER_OF_MINUTES_FOR_CACHE_EXPIRY = 15
+  before_action :set_cache_control_headers, only: %i[navigation_links]
 
   def base_data
     flash.discard(:notice)
@@ -61,4 +62,10 @@ class AsyncInfoController < ApplicationController
     #{current_user&.articles_count}__
     #{current_user&.blocking_others_count}__"
   end
+
+  def navigation_links
+    # We're sending HTML over the wire hence 'render layout: false' enforces rails NOT TO look for a layout file to wrap
+    # the view file - it allows us to not include the HTML headers for sending back to client.
+    render layout: false
+  end
 end

app/javascript/packs/base.jsx

@@ -9,6 +9,14 @@ import { trackCreateAccountClicks } from '@utilities/ahoy/trackEvents';
 import { showWindowModal, closeWindowModal } from '@utilities/showModal';
 import * as Runtime from '@utilities/runtime';
 
+Document.prototype.ready = new Promise((resolve) => {
+  if (document.readyState !== 'loading') {
+    return resolve();
+  }
+  document.addEventListener('DOMContentLoaded', () => resolve());
+  return null;
+});
+
 // Namespace for functions which need to be accessed in plain JS initializers
 window.Forem = {
   preactImport: undefined,
@@ -62,17 +70,17 @@ function getPageEntries() {
   });
 }
 
+/**
+ * Initializes the left hand side hamburger menu
+ */
 function initializeNav() {
   const { currentPage } = document.getElementById('page-content').dataset;
   const menuTriggers = [
-    ...document.querySelectorAll(
-      '.js-hamburger-trigger, .hamburger a:not(.js-nav-more-trigger)',
-    ),
+    ...document.querySelectorAll('.js-hamburger-trigger, .hamburger a'),
   ];
-  const moreMenus = [...document.getElementsByClassName('js-nav-more-trigger')];
 
   setCurrentPageIconLink(currentPage, getPageEntries());
-  initializeMobileMenu(menuTriggers, moreMenus);
+  initializeMobileMenu(menuTriggers);
 }
 
 const memberMenu = document.getElementById('crayons-header__menu');
@@ -82,6 +90,25 @@ if (memberMenu) {
   initializeMemberMenu(memberMenu, menuNavButton);
 }
 
+/**
+ * Fetches the html for the navigation_links from an endpoint and dynamically insterts it in the DOM.
+ */
+async function getNavigation() {
+  const placeholderElement = document.getElementsByClassName(
+    'js-navigation-links-container',
+  )[0];
+
+  if (placeholderElement.innerHTML.trim() === '') {
+    const response = await window.fetch(`/async_info/navigation_links`);
+    const htmlContent = await response.text();
+
+    const generatedElement = document.createElement('div');
+    generatedElement.innerHTML = htmlContent;
+
+    placeholderElement.appendChild(generatedElement);
+  }
+}
+
 // Initialize when asset pipeline (sprockets) initializers have executed
 waitOnBaseData()
   .then(() => {
@@ -103,6 +130,7 @@ waitOnBaseData()
     Honeybadger.notify(error);
   });
 
+// we need to call initializeNav here for the initial page load
 initializeNav();
 
 async function loadCreatorSettings() {
@@ -125,6 +153,13 @@ if (document.location.pathname === '/admin/creator_settings/new') {
   loadCreatorSettings();
 }
 
+document.ready.then(() => {
+  const hamburgerTrigger = document.getElementsByClassName(
+    'js-hamburger-trigger',
+  )[0];
+  hamburgerTrigger.addEventListener('click', getNavigation);
+});
+
 trackCreateAccountClicks('authentication-hamburger-actions');
 trackCreateAccountClicks('authentication-top-nav-actions');
 trackCreateAccountClicks('comments-locked-cta');

app/javascript/topNavigation/utilities.js

@@ -42,7 +42,7 @@ export function isTouchDevice() {
 /**
  * Initializes the member navigation menu events.
  *
- * @param {HTMLElement} memberTopMenu The member menu in the top navigation.
+ * @param {HTMLElement} memberTopMenu The member menu in the top right navigation.
  * @param {HTMLElement} menuNavButton The button to activate the member navigation menu.
  */
 export function initializeMemberMenu(memberTopMenu, menuNavButton) {
@@ -122,11 +122,6 @@ function toggleBurgerMenu() {
     leftNavState === 'open' ? 'closed' : 'open';
 }
 
-function showMoreMenu({ target }) {
-  target.nextElementSibling.classList.remove('hidden');
-  target.classList.add('hidden');
-}
-
 /**
  * Gets a reference to InstantClick
  *
@@ -155,17 +150,13 @@ export async function getInstantClick(waitTime = 2000) {
 /**
  * Initializes the hamburger menu for mobile navigation
  *
- * @param {HTMLElement[]} menuTriggers
- * @param {HTMLElement[]} moreMenus
+ * @param {HTMLElement[]} menuTriggers The menuTriggers include the hamburger to open the menu,
+ * the close icon to close the menu and the overlay to close the menu.
  */
-export function initializeMobileMenu(menuTriggers, moreMenus) {
+export function initializeMobileMenu(menuTriggers) {
   menuTriggers.forEach((trigger) => {
     trigger.onclick = toggleBurgerMenu;
   });
-
-  moreMenus.forEach((trigger) => {
-    trigger.onclick = showMoreMenu;
-  });
 }
 
 /**

app/views/async_info/navigation_links.html.erb

@@ -0,0 +1,2 @@
+<%= render partial: "shared/auth_widget", locals: { tracking_id: "ca_hamburger_home_page", source: "mobile_hamburger_nav" } unless user_signed_in? %>
+<%= render partial: "layouts/main_nav", locals: { context: "hamburger" } %>

app/views/layouts/_main_nav.html.erb

@@ -1,26 +1,26 @@
-<% default_nav_links = NavigationLink.default_section.ordered.to_a %>
-<% other_nav_links = NavigationLink.other_section.ordered.to_a %>
+<% navigation_links = Rails.cache.fetch("navigation_links", expires_in: 15.minutes) do
+{
+    default_nav_ids: NavigationLink.default_section.ordered.ids,
+    other_nav_ids: NavigationLink.other_section.ordered.ids,
+  }
+end %>
 
 <nav class="mb-4 <% unless user_signed_in? %>mt-4<% end %>" data-testid="main-nav" aria-label="<%= community_name %>">
   <ul class="default-navigation-links sidebar-navigation-links spec-sidebar-navigation-links">
-    <% default_nav_links.each do |link| %>
-      <%= render "layouts/sidebar_nav_link", link: link %>
-    <% end %>
+    <%= render partial: "layouts/sidebar_nav_link", collection: NavigationLink.where(id: navigation_links[:default_nav_ids]).ordered, as: :link, cached: true %>
   </ul>
 </nav>
 
 <%# Reading Note: There's a faulty assumption that all "other_nav_links" are
     visible based on the current state (see
     ApplicationHelper#display_navigation_link? for details).  %>
-<% if other_nav_links.any? %>
+<% if navigation_links[:other_nav_ids].any? %>
   <nav class="mb-4" data-testid="other-nav" aria-labelledby="other-nav-heading-<%= context %>">
     <h2 id="other-nav-heading-<%= context %>" class="crayons-subtitle-3 py-2 pl-3">
       <%= t("views.main.nav.other") %>
     </h2>
     <ul class="other-navigation-links sidebar-navigation-links spec-sidebar-navigation-links">
-      <% other_nav_links.each do |link| %>
-        <%= render "layouts/sidebar_nav_link", link: link %>
-      <% end %>
+      <%= render partial: "layouts/sidebar_nav_link", collection: NavigationLink.where(id: navigation_links[:other_nav_ids]).ordered, as: :link, cached: true %>
     </ul>
   </nav>
 <% end %>

app/views/shared/_hamburger.html.erb

@@ -8,9 +8,7 @@
       </button>
     </header>
 
-    <div class="p-2" id="authentication-hamburger-actions">
-      <%= render partial: "shared/auth_widget", locals: { tracking_id: "ca_hamburger_home_page", source: "mobile_hamburger_nav" } unless user_signed_in? %>
-      <%= render partial: "layouts/main_nav", locals: { context: "hamburger" } %>
+    <div class="p-2 js-navigation-links-container" id="authentication-hamburger-actions">
     </div>
   </div>
   <div class="hamburger__overlay js-hamburger-trigger"></div>

config/routes.rb

@@ -186,6 +186,7 @@ Rails.application.routes.draw do
     get "/social_previews/article/:id", to: "social_previews#article", as: :article_social_preview
 
     get "/async_info/base_data", controller: "async_info#base_data", defaults: { format: :json }
+    get "/async_info/navigation_links", controller: "async_info#navigation_links"
 
     # Settings
     post "users/join_org", to: "users#join_org"

spec/requests/async_info_spec.rb

@@ -1,6 +1,6 @@
 require "rails_helper"
 
-RSpec.describe "AsyncInfo", type: :request do
+RSpec.describe "AsyncInfo" do
   let(:controller_instance) { AsyncInfoController.new }
 
   before do
@@ -30,4 +30,30 @@ RSpec.describe "AsyncInfo", type: :request do
       end
     end
   end
+
+  describe "GET /async_info/navigation_links" do
+    it "returns html" do
+      get "/async_info/navigation_links"
+      expect(response).to have_http_status(:ok)
+      expect(response.body).to include("<div")
+    end
+
+    it "returns the correct 'default' navigation links" do
+      default_navigation_link = create(:navigation_link)
+      get "/async_info/navigation_links"
+      expect(response.body).to include CGI.escapeHTML(default_navigation_link.name)
+      expect(response.body).to include(default_navigation_link.url)
+      expect(response.body).to include(default_navigation_link.icon)
+    end
+
+    it "returns the correct 'other' navigation links" do
+      other_navigation_link = create(:navigation_link)
+      other_navigation_link.other_section!
+
+      get "/async_info/navigation_links"
+      expect(response.body).to include CGI.escapeHTML(other_navigation_link.name)
+      expect(response.body).to include(other_navigation_link.url)
+      expect(response.body).to include(other_navigation_link.icon)
+    end
+  end
 end

spec/system/ahoy/tracking_create_account_spec.rb

@@ -13,16 +13,19 @@ RSpec.describe "Tracking 'Clicked on Create Account'" do
       visit root_path
     end
 
-    it "expects page to have the necessary tracking elements", :aggregate_failures do
-      # rubocop:disable RSpec/Capybara/SpecificMatcher
+    it "has the necessary initial tracking elements", :aggregate_failures do
       expect(page).to have_selector('a[data-tracking-id="ca_top_nav"]')
       expect(page).to have_selector('a[data-tracking-id="ca_left_sidebar_home_page"]')
-      expect(page).to have_selector('a[data-tracking-id="ca_hamburger_home_page"]')
       expect(page).to have_selector('a[data-tracking-id="ca_feed_home_page"]')
-      # rubocop:enable RSpec/Capybara/SpecificMatcher
     end
 
-    it "tracks a click with the correct source", { js: true, aggregate_failures: true } do
+    it "has the create account tracking element in the hamburger", { aggregate_failures: true, js: true } do
+      Capybara.current_session.driver.resize(425, 694)
+      first(".js-hamburger-trigger").click
+      expect(page).to have_selector('a[data-tracking-id="ca_hamburger_home_page"]')
+    end
+
+    it "tracks a click with the correct source", { aggregate_failures: true, js: true } do
       expect do
         find('[data-tracking-id="ca_top_nav"]').click
       end.to change(Ahoy::Event, :count).by(1)
@@ -38,7 +41,7 @@ RSpec.describe "Tracking 'Clicked on Create Account'" do
   end
 
   context "when tracking through the modal" do
-    it "adds an ahoy event", { js: true, aggregate_failures: true } do
+    it "adds an ahoy event", { aggregate_failures: true, js: true } do
       article = create(:article, user: create(:user))
       visit article.path
       find(".follow-action-button").click