Merge pull request #265 from MITLibraries/use-135-sticky-tabs

Adds persistence to Tab selection across searches

Author: JPrevost

README.md

@@ -82,6 +82,7 @@ See `Optional Environment Variables` for more information.
 - `PRIMO_SCOPE`: The Primo Search API `scope` param (set to `cdi` for CDI-scoped results).
 - `PRIMO_TAB`: The Primo Search API `tab` param (typically `all`).
 - `PRIMO_VID`: The Primo Search API `vid` (or 'view ID`) param.
+- `SECRET_KEY_BASE`: You can generate this via `bin/rails secret`. Please do not re-use the production value locally.
 - `SYNDETICS_PRIMO_URL`: The Syndetics API URL for Primo. This is used to construct thumbnail URLs.
 - `TIMDEX_GRAPHQL`: Set this to the URL of the GraphQL endpoint. There is no default value in the application.
 
@@ -96,6 +97,7 @@ See `Optional Environment Variables` for more information.
 - `FEATURE_GEODATA`: Enables features related to geospatial data discovery. Setting this variable to `true` will trigger geodata
 mode. Note that this is currently intended _only_ for the geodata app and
 may have unexpected consequences if applied to other TIMDEX UI apps.
+- `FEATURE_SIMULATE_SEARCH_LATENCY`: DO NOT SET IN PRODUCTION. Set to ensure a minimum of a one second delay in returning search results. Useful to see spinners/loaders. Only introduces delay for results that take less than one second to complete.
 - `FILTER_ACCESS_TO_FILES`: The name to use instead of "Access to files" for that filter / aggregation.
 - `FILTER_CONTENT_TYPE`: The name to use instead of "Content type" for that filter / aggregation.
 - `FILTER_CONTRIBUTOR`: The name to use instead of "Contributor" for that filter / aggregation.

app/assets/stylesheets/partials/_search.scss

@@ -196,3 +196,8 @@
 .tab-link {
   color: #fff;
 }
+
+/* temp style to visualize active tab. Save us from this Dave! */
+#tabs .active{
+  background-color: red;
+}

app/controllers/application_controller.rb

@@ -1,3 +1,23 @@
 class ApplicationController < ActionController::Base
   helper Mitlibraries::Theme::Engine.helpers
+
+  # Set active tab based on feature flag and params
+  # Also stores the last used tab in a cookie for future searches when passed via params.
+  # We set this in a session cookie to persist user preference across searches.
+  # Clicking on a different tab will update the cookie.
+  def set_active_tab
+    @active_tab = if Feature.enabled?(:geodata)
+                    # Determine which tab to load - default to primo unless gdt is enabled
+                    'geodata'
+                  elsif params[:tab].present?
+                    # If params[:tab] is set, use it and set session
+                    cookies[:last_tab] = params[:tab]
+                  elsif cookies[:last_tab].present?
+                    # Otherwise, check for last used tab in session
+                    cookies[:last_tab]
+                  else
+                    # Default behavior when no tab is specified in params or session
+                    cookies[:last_tab] = 'primo'
+                  end
+  end
 end

app/controllers/basic_search_controller.rb

@@ -1,3 +1,5 @@
 class BasicSearchController < ApplicationController
+  before_action :set_active_tab
+
   def index; end
 end

app/controllers/search_controller.rb

@@ -1,5 +1,7 @@
 class SearchController < ApplicationController
   before_action :validate_q!, only: %i[results]
+  before_action :set_active_tab, only: %i[results]
+  around_action :sleep_if_too_fast, only: %i[results]
 
   before_action :validate_geobox_presence!, only: %i[results]
   before_action :validate_geobox_range!, only: %i[results]
@@ -13,34 +15,41 @@ def results
     # inject session preference for boolean type if it is present
     params[:booleanType] = cookies[:boolean_type] || 'AND'
 
-    # Determine which tab to load - default to primo unless gdt is enabled
-    @active_tab = if Feature.enabled?(:geodata)
-                    'gdt' # Keep existing GDT behavior unchanged
-                  else
-                    params[:tab] || 'primo' # Default to primo for new tabbed interface
-                  end
-
-    # Include the active tab in the enhanced query so it's available for pagination and other uses.
-    params[:tab] = @active_tab unless Feature.enabled?(:gdt)
     @enhanced_query = Enhancer.new(params).enhanced_query
 
     # Route to appropriate search based on active tab
-    if Feature.enabled?(:geodata)
-      # Keep existing GDT behavior unchanged
+    case @active_tab
+    when 'primo'
+      load_primo_results
+    when 'timdex'
+      load_timdex_results
+    when 'geodata'
       load_gdt_results
       render 'results_geo'
-    else
-      case @active_tab
-      when 'primo'
-        load_primo_results
-      when 'timdex'
-        load_timdex_results
-      end
     end
   end
 
   private
 
+  # Sleep to simulate latency for testing loading indicators when responses are fast
+  def sleep_if_too_fast
+    start_time = Time.now
+
+    yield
+
+    end_time = Time.now
+    duration = end_time - start_time
+
+    return unless Feature.enabled?(:simulate_search_latency)
+
+    Rails.logger.debug "Action #{action_name} from controller #{controller_name} took #{duration.round(2)} seconds to execute."
+
+    return unless duration < 1
+
+    Rails.logger.debug("Sleeping for #{1 - duration}")
+    sleep(1 - duration)
+  end
+
   def load_gdt_results
     query = QueryBuilder.new(@enhanced_query).query
 
@@ -112,7 +121,7 @@ def query_timdex(query)
 
     # Builder hands off to wrapper which returns raw results here.
     Rails.cache.fetch("#{cache_key}/#{@active_tab}", expires_in: 12.hours) do
-      raw = if @active_tab == 'gdt'
+      raw = if @active_tab == 'geodata'
               execute_geospatial_query(query)
             elsif @active_tab == 'timdex'
               TimdexBase::Client.query(TimdexSearch::BaseQuery, variables: query)

app/javascript/loading_spinner.js

@@ -8,7 +8,35 @@ document.addEventListener('turbo:frame-render', function(event) {
     }
     // Clear the pending action
     window.pendingFocusAction = null;
-  }
+  };
+
+  if (window.pendingFocusAction === 'tab') {
+    // console.log("Tab change detected - focusing on first result");
+
+    const urlParams = new URLSearchParams(window.location.search);
+    const queryParam = urlParams.get('tab');
+    const searchInput = document.querySelector('input[name="tab"]');
+    
+    // update hidden form element to ensure correct tab is used for subsequent searches
+    if (searchInput && queryParam != null) {
+      searchInput.value = queryParam;
+      // console.log(`Updated tab input value to: ${queryParam}`);
+    }
+
+    // update active tab styling
+    // remove active class from all tabs
+    document.querySelectorAll('.tab-link').forEach((tab) => {
+      tab.classList.remove('active');
+    });
+    // add active class to current tab
+    const currentTabLink = document.querySelector(`.tab-link[href*="tab=${queryParam}"]`);
+    if (currentTabLink) {
+      currentTabLink.classList.add('active');
+    }
+
+    // Clear the pending action
+    window.pendingFocusAction = null;
+  };
 });
 
 document.addEventListener('click', function(event) {
@@ -20,4 +48,26 @@ document.addEventListener('click', function(event) {
     window.scrollTo({ top: 0, behavior: 'smooth' });
     window.pendingFocusAction = 'pagination';
   }
-});
+
+  // Handle tab clicks
+  if (clickedElement.closest('.tab-navigation')) {
+    window.scrollTo({ top: 0, behavior: 'smooth' });
+    window.pendingFocusAction = 'tab';
+  }
+});
+
+// On Turbo Frame render, update the search input value to match the current URL parameter
+// This ensures that after using the back button the search input reflects the correct query
+document.addEventListener('turbo:load', function(event) {
+  // update form element name 'q' to use current url paramater `q`
+  // console.log(`turbo:frame-render event detected for frame: ${event.target.id}`);
+  const urlParams = new URLSearchParams(window.location.search);
+  const queryParam = urlParams.get('q');
+  const searchInput = document.querySelector('input[name="q"]');
+  if (searchInput && queryParam != null) {
+    searchInput.value = queryParam;
+    // console.log(`Updated search input value to: ${queryParam}`);
+  }
+});
+
+

app/models/feature.rb

@@ -33,7 +33,7 @@
 #
 class Feature
   # List of all valid features in the application
-  VALID_FEATURES = %i[geodata boolean_picker].freeze
+  VALID_FEATURES = %i[geodata boolean_picker simulate_search_latency].freeze
 
   # Check if a feature is enabled by name
   #

app/views/search/_form.html.erb

@@ -2,6 +2,7 @@
   <h3>What can we help you find?</h3>
   <div class="form-wrapper">
     <input id="basic-search-main" type="search" class="field field-text basic-search-input" name="q" title="Keyword anywhere" placeholder="Search for anything" value="<%= params[:q] %>" required>
+    <input id="tab-to-target" type="hidden" name="tab" value="<%= @active_tab %>">
     <button type="submit" class="btn button-primary">Search</button>
   </div>
 </form>

config/initializers/session_store.rb

@@ -0,0 +1,5 @@
+# Be sure to restart your server when you modify this file.
+# Note, you must set `SECRET_KEY_BASE` environment variable before running the application or sessions will not work.
+# Changing `SECRET_KEY_BASE` will invalidate all existing sessions.
+# You can generate a new secret key by running `bin/rails secret` command.
+Rails.application.config.session_store :cookie_store, key: '_use_session', secure: false, same_site: :strict

test/controllers/application_controller_test.rb

@@ -0,0 +1,73 @@
+require 'test_helper'
+
+class ApplicationControllerTest < ActionDispatch::IntegrationTest
+  test 'set_active_tab sets default to primo when no feature flag or params' do
+    assert_nil cookies[:last_tab]
+
+    get root_path
+    assert_select '#tab-to-target' do
+      assert_select '[value=?]', 'primo'
+      refute_select '[value=?]', 'geodata'
+      refute_select '[value=?]', 'timdex'
+    end
+
+    assert_equal cookies[:last_tab], 'primo'
+  end
+
+  test 'set_active_tab sets to geodata when feature flag enabled' do
+    skip 'Geodata uses a different form so we do no set the tab this way'
+    ClimateControl.modify FEATURE_GEODATA: 'true' do
+      get root_path
+      assert_select '#tab-to-target' do
+        refute_select '[value=?]', 'primo'
+        assert_select '[value=?]', 'geodata'
+        refute_select '[value=?]', 'timdex'
+      end
+    end
+  end
+
+  test 'set_active_tab sets to geodata when feature flag enabled even if param is passed' do
+    skip 'Geodata uses a different form'
+    ClimateControl.modify FEATURE_GEODATA: 'true' do
+      get root_path, params: { tab: 'primo' }
+      assert_select '#tab-to-target' do
+        refute_select '[value=?]', 'primo'
+        assert_select '[value=?]', 'geodata'
+        refute_select '[value=?]', 'timdex'
+      end
+    end
+  end
+
+  test 'set_active_tab sets to param tab when provided' do
+    get root_path, params: { tab: 'timdex' }
+
+    assert_select '#tab-to-target' do
+      refute_select '[value=?]', 'primo'
+      refute_select '[value=?]', 'geodata'
+      assert_select '[value=?]', 'timdex'
+    end
+  end
+
+  test 'set_active_tab sets to param tab when provided even if cookie is set and updates cookie' do
+    cookies[:last_tab] = 'timdex'
+    get root_path, params: { tab: 'geodata' }
+
+    assert_select '#tab-to-target' do
+      refute_select '[value=?]', 'primo'
+      assert_select '[value=?]', 'geodata'
+      refute_select '[value=?]', 'timdex'
+    end
+
+    assert_equal cookies[:last_tab], 'geodata'
+  end
+
+  test 'set_active_tab uses cookie last_tab when no param provided' do
+    cookies[:last_tab] = 'timdex'
+    get root_path
+    assert_select '#tab-to-target' do
+      refute_select '[value=?]', 'primo'
+      refute_select '[value=?]', 'geodata'
+      assert_select '[value=?]', 'timdex'
+    end
+  end
+end