Redesign filter sidebar

Why these changes are being introduced:

The filter sidebar can become unwieldy, as all available filters
are shown at all times.

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/GDT-127
* https://mitlibraries.atlassian.net/browse/GDT-173
* https://mitlibraries.atlassian.net/browse/GDT-174

How this addresses that need:

This restyles the filter sidebar as dropdown menus. In so doing,
it also changes some behavior in the filter logic:

* Number of results is listed in the filter sidebar header.
* Clicking an applied filter removes that filter (this replaces the
previous removal mechanism, which removed all applied filters).
* Applied filters are highlighted with an `x` symbol, to indicate
that it can be removed on click. (Previously, the 'applied' filter
was added, but we hadn't done anything with it.)
* The first filter dropdown in the sidebar is always open, even if
no filters are applied.

Not addressed in this commit:

* Darcy asked that we persist menu state between page loads, such
that an open menu always remains open unless a user closes it
manually (e.g., if all filters have been removed in that category).
This proved relatively complex, so I opened GDT-173 to address it.
* There are some visual differences in how the sidebar appears
in smaller viewports. This is covered in GDT-174.

Side effects of this change:

* Some tests have been updated to reflect the new logic.
* One potential a11y issue with filter categories is having to
navigate through a bunch of filter opens before getting to the
next category. We might want to add skip-content links if this
feels onerous.
* The JS for the dropdown menus is so minimal that I included it
in the view as a `script` tag.
* The filter container is now above the results in the DOM.

app/assets/stylesheets/partials/_filters.scss

@@ -1,48 +1,107 @@
 #filters {
-  h3 {
+  .category {
+    position: relative;
+    margin-bottom: 0.5em;
+    border: 1px solid $gray-l3;
+    border-radius: 3px;
+  }
+
+  button.filter-label {
+    display: flex;
+    justify-content: space-between;
+    width: 100%;
+    text-align: left;
     margin-bottom: 0;
+    padding: 1rem;
+    border: 0;
+    color: $black;
+    background-color: $gray-l4;
+    font-size: $fs-small;
+    font-weight: $fw-bold;
+
+    &::after {
+      font-family: FontAwesome;
+      content: '\f054';
+    }
+    &.expanded {
+      border: 0;
+      border-radius: 0;
+      border-bottom: 1px solid $gray-l3;
+      & + .filter-options {
+        max-height: 200px;
+        overflow-y: scroll;
+        scrollbar-gutter: auto;
+        scroll-behavior: auto;
+      }
+    }
+    &.expanded:after {
+      font-family: FontAwesome;
+      content: '\f078';
+    }
+    &::-webkit-details-marker {
+      display: none;
+    }
   }
 
-  .category {
-    margin-bottom: 1em;
+  .filter-options {
+    max-height: 0;
+    transition: max-height 0.5s ease-in-out;
+    overflow: hidden;
   }
 
   ul.category-terms {
     border-collapse: separate;
     border-spacing: 0px 4px;
-    display: table;
-    margin-bottom: 0;
-    table-layout: fixed;
-    width: 100%;
+    margin: 1rem;
+    margin-bottom: 2rem;
 
     li.term {
-      display: contents;
+      display: block;
+      width: 100%;
+      float: none;
+      margin: 0;
+      font-size: $fs-small;
 
       a {
-        display: table-row;
+        display: block;
+        width: 100%;
         text-decoration: none;
 
-        &:hover,
-        &:focus,
-        &.applied {
-          background: #000;
-          color: #fff;
-        }
-
         span {
           display: table-cell;
           padding: 2px;
 
-            &.name {
-              width: 75%;
-              text-decoration: underline;
-            }
+          &.name {
+            width: 100%;
+            color: $blue;
+          }
 
-            &.count {
-              min-width: 25%;
-              text-align: right;
-              text-decoration: none;
+          &.count {
+            text-align: right;
+            text-decoration: none;
+          }
+        }
+
+        &:hover,
+        &:focus {
+          background: #000;
+          span {
+            color: #fff;
+          }
+        }
+        &.applied {
+          .name {
+            color: #000;
+            &:hover,
+            &:focus {
+              color: #fff;
             }
+          }
+          .name::after {
+            font-family: FontAwesome;
+            margin-left: 0.5rem;
+            content: '\f00d';
+          }
         }
       }
     }

app/helpers/filter_helper.rb

@@ -26,10 +26,22 @@ def nice_labels
     }
   end
 
-  def remove_filter(query, filter)
+  def remove_filter(query, filter, term)
     new_query = query.deep_dup
     new_query[:page] = 1
-    new_query.delete filter.to_sym
+
+    if new_query[filter].length > 1
+      new_query[filter].delete(term) # If more than one term is filtered, we only delete the selected term
+    else
+      new_query.delete(filter) # If only one term is filtered, delete the entire filter from the query
+    end
+
     new_query
   end
+
+  def filter_applied?(terms, term)
+    return if terms.blank?
+
+    terms.include?(term)
+  end
 end

app/helpers/results_helper.rb

@@ -0,0 +1,5 @@
+module ResultsHelper
+  def results_summary(hits)
+    hits.to_i >= 10_000 ? '10,000+ items' : "#{number_with_delimiter(hits)} items"
+  end
+end

app/views/search/_filter.html.erb

@@ -1,19 +1,29 @@
 <% return if values.empty? %>
 
 <div class="category">
-  <h3><%= nice_labels[category] || category %></h3>
-  <ul class="category-terms list-unbulleted">
-  <% values.each do |term| %>
-    <li class="term">
-      <a href="<%= results_path(add_filter(@enhanced_query, category, term['key'])) %>" class="<%= "applied" if @enhanced_query[category.to_sym] == term['key'] %>">
-        <span class="name"><%= term['key'] %></span>
-        <span class="count"><%= term['docCount'] %> <span class="sr">records</span></span>
-      </a>
-    </li>
-  <% end %>
-  </ul>
-  <% if @enhanced_query[category.to_sym].present? %>
-    <div><%= link_to "Show all #{nice_labels[category]&.downcase || category}", results_path(remove_filter(@enhanced_query, category)) %>
-    </div>
-  <% end %>
+  <button class="filter-label <%= 'expanded' if @enhanced_query[category.to_sym].present? || first == true %>"
+          onclick="toggleFilter(this)"><%= nice_labels[category] || category %></button>
+  <div class="filter-options">
+    <ul class="category-terms list-unbulleted">
+    <% values.each do |term| %>
+      <li class="term">
+        <% if filter_applied?(@enhanced_query[category.to_sym], term['key']) %>
+          <a href="<%= results_path(remove_filter(@enhanced_query, category.to_sym, term['key'])) %>" class="applied">
+          <span class="sr">Remove applied filter?</span>
+        <% else %>
+          <a href="<%= results_path(add_filter(@enhanced_query, category.to_sym, term['key'])) %>">
+        <% end %>
+          <span class="name"><%= term['key'] %></span>
+          <span class="count"><%= term['docCount'] %> <span class="sr">records</span></span>
+        </a>
+      </li>
+    <% end %>
+    </ul>
+  </div>
 </div>
+
+<script>
+  function toggleFilter(e) {
+    e.parentNode.getElementsByClassName("filter-label")[0].classList.toggle("expanded");
+  }
+</script>

app/views/search/results.html.erb

@@ -49,6 +49,22 @@
   <%= render(partial: 'shared/error', collection: @errors) %>
 
   <div class="<%= 'layout-1q3q' if @filters.present? %> layout-band top-space">
+    <% if @filters.present? %>
+      <aside class="col1q filter-container">
+        <div id="filters">
+          <h2 class="hd-3">Filter your results</h2>
+          <h3 class="hd-4"><em><%= results_summary(@pagination[:hits]) %></em></h3>
+          <% @filters&.each_with_index do |(category, values), index| %>
+            <% if index == 0 %>
+              <%= render(partial: 'search/filter', locals: {category: category, values: values, first: true}) %>
+            <% else %>
+              <%= render(partial: 'search/filter', locals: {category: category, values: values, first: false }) %>
+            <% end %>
+          <% end %>
+        </div>
+      </aside>
+    <% end %>
+
     <div class="col3q wrap-results">
       <% if @results.present? %>
         <ul id="results" class="list-unbulleted">
@@ -61,18 +77,6 @@
       <% end %>
     </div>
 
-    <% if @filters.present? %>
-      <aside class="col1q filter-container">
-        <div id="filters">
-          <h2>Available filters</h2>
-          <% @filters&.each do |category, values| %>
-            <%= render(partial: 'search/filter', locals: {category: category, values: values}) %>
-          <% end %>
-        </div>
-      </aside>
-    <% end %>
-  </div>
-
   <% if @results.present? %>
     <div id="pagination">  
       <%= render partial: "pagination" %>

test/controllers/search_controller_test.rb

@@ -135,7 +135,7 @@ class SearchControllerTest < ActionDispatch::IntegrationTest
       get '/results?q=data'
       assert_response :success
       assert_select '#filters'
-      assert_select '#filters .category h3', { minimum: 1 }
+      assert_select '#filters .category .filter-label', { minimum: 1 }
     end
   end
 

test/helpers/filter_helper_test.rb

@@ -71,25 +71,66 @@ class FilterHelperTest < ActionView::TestCase
     original_query = {
       page: 1,
       q: 'data',
-      contentType: 'dataset'
+      contentType: ['dataset']
     }
     expected_query = {
       page: 1,
       q: 'data'
     }
-    assert_equal expected_query, remove_filter(original_query, 'contentType')
+    assert_equal expected_query, remove_filter(original_query, :contentType, 'dataset')
   end
 
   test 'remove_filter will reset a page count when called' do
     original_query = {
       page: 3,
       q: 'data',
-      contentType: 'dataset'
+      contentType: ['dataset']
     }
     expected_query = {
       page: 1,
       q: 'data'
     }
-    assert_equal expected_query, remove_filter(original_query, 'contentType')
+    assert_equal expected_query, remove_filter(original_query, :contentType, 'dataset')
+  end
+
+  test 'remove_filter removes only one filter parameter if multiple are applied' do
+    original_query = {
+      page: 3,
+      q: 'data',
+      contentType: ['dataset', 'microfiche', 'vinyl record']
+    }
+    expected_query = {
+      page: 1,
+      q: 'data',
+      contentType: ['dataset', 'vinyl record']
+    }
+    assert_equal expected_query, remove_filter(original_query, :contentType, 'microfiche')
+  end
+
+  test 'filter_applied? returns true if a filter is applied' do
+    query = {
+      page: 3,
+      q: 'data',
+      contentType: ['dataset']
+    }
+    assert filter_applied?(query[:contentType], 'dataset')
+  end
+
+  test 'filter_applied? returns false if the filter does not include the target term' do
+    query = {
+      page: 3,
+      q: 'data',
+      contentType: ['dataset']
+    }
+    assert_not filter_applied?(query[:contentType], 'microfiche')
+  end
+
+  # This is an unlikely state to reach, but better safe than sorry
+  test 'filter_applied? returns false if no filter is supplied in the query' do
+    query = {
+      page: 3,
+      q: 'data',
+    }
+    assert_not filter_applied?(query[:contentType], 'dataset')
   end
 end

test/helpers/results_helper_test.rb

@@ -0,0 +1,20 @@
+require 'test_helper'
+
+class ResultsHelperTest < ActionView::TestCase
+  include ResultsHelper
+
+  test 'if number of hits is equal to 10,000, results summary returns "10,000+"' do
+    hits = 10000
+    assert_equal '10,000+ items', results_summary(hits)
+  end
+
+  test 'if number of hits is above 10,000, results summary returns "10,000+"' do
+    hits = 10500
+    assert_equal '10,000+ items', results_summary(hits)
+  end
+
+  test 'if number of hits is below 10,000, results summary returns actual number of results' do
+    hits = 9000
+    assert_equal '9,000 items', results_summary(hits)
+  end
+end