Make filter sidebar more semantically meaningful

Why these changes are being introduced:

VoiceOver (and possibly other screen readers) do not interpret
the current sidebar drawers as disclosure triangles. As such,
one must tab through every available filter, even if that
filter category is collapsed.

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/GDT-161

How this addresses that need:

This refactors filter categories as `details` tags, so screen readers
will recognize them as expandable elements.

Side effects of this change:

* Some tests have been updated a result of changes to class names.
* We had previously been opening/closing categories with an `onclick`,
was a quick-and-dirty way to get things working but not very good
practice. I took this opportunity to change that too an event
listener and moved it to the `filters.js` file that we added for
the mobile filter toggle button.
* I've removed the transition rules for the filters, since `details`
does not support them natively.

Author: jazairi

app/assets/stylesheets/partials/_filters.scss

@@ -1,12 +1,12 @@
 #filters {
-  .category {
+  details.filter-category {
     position: relative;
     margin-bottom: 0.5em;
     border: 1px solid $gray-l3;
     border-radius: 3px;
   }
 
-  button.filter-label {
+  summary.filter-label {
     display: flex;
     justify-content: space-between;
     width: 100%;
@@ -47,12 +47,6 @@
     }
   }
 
-  .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;

app/javascript/filters.js

@@ -1,11 +1,16 @@
 // These elements aren't loaded with the initial DOM, they appear later.
 function initFilterToggle() {
   var filter_toggle = document.getElementById('filter-toggle');
-  var filter_panel = document.getElementById('filters');
+  var filter_panel = document.getElementById('filter-container');
+  var filter_categories = document.getElementsByClassName('filter-category');
   filter_toggle.addEventListener('click', event => {
     filter_panel.classList.toggle('hidden-md');
     filter_toggle.classList.toggle('expanded');
-
+  });
+  [...filter_categories].forEach(element => {
+    element.addEventListener('click', event => {
+      element.getElementsByClassName('filter-label')[0].classList.toggle('expanded');
+    });
   });
 }
 

app/views/search/_filter.html.erb

@@ -1,17 +1,19 @@
 <% return if values.blank? %>
 
-<div class="category">
-  <button class="filter-label <%= 'expanded' if @enhanced_query[category].present? || first == true %>"
-          onclick="toggleFilter(this)"><%= nice_labels[category] || category %></button>
+<details class="filter-category" <%= 'open' if @enhanced_query[category].present? || first == true %>>
+  <summary class="filter-label<%= ' expanded' if @enhanced_query[category].present? || first == true %>">
+    <%= nice_labels[category] || category %>
+  </summary>
   <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, term['key'])) %>" class="applied">
-          <span class="sr">Remove applied filter?</span>
+          <span class="sr">Remove applied filter:</span>
         <% else %>
           <a href="<%= results_path(add_filter(@enhanced_query, category, term['key'])) %>">
+          <span class="sr">Apply filter:</span>
         <% end %>
           <span class="name"><%= term['key'] %></span>
           <span class="count"><%= term['docCount'] %> <span class="sr">records</span></span>
@@ -20,10 +22,4 @@
     <% end %>
     </ul>
   </div>
-</div>
-
-<script>
-  function toggleFilter(e) {
-    e.parentNode.getElementsByClassName("filter-label")[0].classList.toggle("expanded");
-  }
-</script>
+</details>

app/views/search/results.html.erb

@@ -3,6 +3,7 @@
 <div class="space-wrap">
 
   <%= render partial: "shared/site_title" %>
+
   <%= render partial: "form" %>
   <%= render partial: "search_summary" %>
 
@@ -17,9 +18,9 @@
 
   <div class="<%= @filters.present? ? 'layout-1q3q' : 'layout-3q1q' %> layout-band top-space">
     <% if @filters.present? %>
-      <aside class="col1q filter-container">
+      <aside id="filters" class="col1q">
         <button id="filter-toggle"><span class="filter-toggle-name">Filter your results: <%= results_summary(@pagination[:hits]) %></span><span class="filter-toggle-hide">Hide filters</span></button>
-        <div id="filters" class="hidden-md">
+        <div id="filter-container" class="hidden-md">
           <div class="hidden-md">
             <h2 class="hd-3">Filter your results</h2>
             <h3 class="hd-4"><em><%= results_summary(@pagination[:hits]) %></em></h3>

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 .filter-label', { minimum: 1 }
+      assert_select '#filters .filter-category .filter-label', { minimum: 1 }
     end
   end
 
@@ -211,7 +211,7 @@ class SearchControllerTest < ActionDispatch::IntegrationTest
       assert_select '#filters', { count: 0 }
 
       # Filters are not shown
-      assert_select '#filters .category h3', { count: 0 }
+      assert_select '#filters .filter-category h3', { count: 0 }
 
       # Pagination is not shown
       assert_select '#pagination', { count: 0 }
@@ -427,31 +427,31 @@ def source_filter_count(controller)
       ClimateControl.modify ACTIVE_FILTERS: '' do
         get '/results?q=data'
         assert_response :success
-        assert_select '#filters .category .filter-label', { minimum: 1 }
+        assert_select '#filters .filter-category .filter-label', { minimum: 1 }
       end
 
       # Ask for a single filter, get that filter.
       ClimateControl.modify ACTIVE_FILTERS: 'subjects' do
         get '/results?q=data'
         assert_response :success
-        assert_select '#filters .category .filter-label', { count: 1 }
-        assert_select '#filters .category:first-of-type .filter-label', 'Subject'
+        assert_select '#filters .filter-category .filter-label', { count: 1 }
+        assert_select '#filters .filter-category:first-of-type .filter-label', 'Subject'
       end
 
       # The order of the terms matter, so now Format should be first.
       ClimateControl.modify ACTIVE_FILTERS: 'format, contentType, source' do
         get '/results?q=data'
         assert_response :success
-        assert_select '#filters .category .filter-label', { count: 3 }
-        assert_select '#filters .category:first-of-type .filter-label', 'Format'
+        assert_select '#filters .filter-category .filter-label', { count: 3 }
+        assert_select '#filters .filter-category:first-of-type .filter-label', 'Format'
       end
 
       # Including extra values does not affect anything - "nonsense" is extraneous.
       ClimateControl.modify ACTIVE_FILTERS: 'contentType, nonsense, source' do
         get '/results?q=data'
         assert_response :success
-        assert_select '#filters .category .filter-label', { count: 2 }
-        assert_select '#filters .category:first-of-type .filter-label', 'Content type'
+        assert_select '#filters .filter-category .filter-label', { count: 2 }
+        assert_select '#filters .filter-category:first-of-type .filter-label', 'Content type'
       end
     end
   end