Merge pull request #274 from projectblacklight/collapsible_textual_fa…

…cets

Textual facets in collapsible on screen

README.md

@@ -82,7 +82,7 @@ In order to calculate distribution segment ranges, we need to first know the min
 
 So, ordinarily, after we've gotten the result set, an additional round trip to back-end and solr will happen, with min max identified, to fetch segments.
 
-If you'd like to avoid this, you can turn off segment display altogether with the :segment option below; or you can set :assumed_boundaries below to use fixed boundaries for not-yet-limited segments instead of taking boundaries from the result set.
+If you'd like to avoid this, you can set :assumed_boundaries below to use fixed boundaries for not-yet-limited segments instead of taking boundaries from the result set. Or you can disable the segment behavior by setting `chart_js` and `textual_facets` both to false.
 
 Note that a drill-down will never require the second request, because boundaries on a drill-down are always taken from the specified limits.
 
@@ -99,7 +99,8 @@ config.add_facet_field 'pub_date', label: 'Publication Year',
                assumed_boundaries: [1100, Time.now.year + 2],
                segments: true,
                chart_js: true,
-               chart_replaces_text: true,
+               textual_facets: true,
+               textual_facets_collapsible: true,
                chart_segment_border_color: "rgba(0,0,0, 0.5)",
                chart_segment_bg_color: "#ccddcc",
                chart_aspect_ratio: "2"
@@ -111,11 +112,13 @@ config.add_facet_field 'pub_date', label: 'Publication Year',
   * Default 10. Approximately how many segments to divide the range into for segment facets, which become segments on the chart. Actual segments are calculated to be 'nice' values, so may not exactly match your setting.
 * **:assumed_boundaries** :
   * Default null. For a result set that has not yet been limited, instead of taking boundaries from results and making a second AJAX request to fetch segments, just assume these given boundaries. If you'd like to avoid this second AJAX Solr call, you can set :assumed_boundaries to a two-element array of integers instead, and the assumed boundaries will always be used. Note this is live ruby code, you can put calculations in there like Time.now.year + 2.
-* **:segments** :
-  * Default true. If set to false, then distribution segment facets will not be loaded at all,    you'll just get input boxes.
 * **chart_js**:
-  * Default true. If false, the Javascript chart is not loaded, you will still get textual facets for buckets.
-* **chart_replaces_text**: Default true. If false, when the chart is loaded purely textual facets will still remain on-screen too.
+  * Default true. If false, the Javascript chart is not loaded, you can still get textual facets for bucket with `textual_facets` config.
+* **textual_facets**: Default true. Should we show textual facet list too? Universal design
+  for accessibility, may have accessibilty concerns to turn off.
+* **textual_facets_collapsible**: Put the textual facets in a collapse/expand
+  disclosure. If you set chart_js to false, may make sense to set this to false too, to have
+  textual facets only instead of chart?
 * **chart_segment_border_color** / **chart_segment_bg_color** :
   * Set colors for the edge and fill of the segment bars in the histogram.
 * chart_aspect_ratio: for chart.js, will fill available width then this determines size of chart. defaults to 2

app/assets/javascripts/blacklight-range-limit/index.js

@@ -41,7 +41,9 @@ export default class BlacklightRangeLimit {
     });
   }
 
-  chartReplacesText = true;
+  textualFacets = true;
+  textualFacetsCollapsible = true;
+  rangeListHeadingLocalized = undefined;
 
   rangeBuckets = []; // array of objects with bucket range info
 
@@ -52,7 +54,7 @@ export default class BlacklightRangeLimit {
   container;  // div.range-limit wrapping entire facet display box
   chartCanvasElement; // <canvas> DOM element
 
-  // container should be a `div.range-limit` that will have within it a `.profile .distribution`
+  // container should be a `div.range-limit` that will have within it a `.distribution`
   // with textual distributions that will be turned into a histogram chart.
   constructor(container) {
     this.container = container;
@@ -61,7 +63,7 @@ export default class BlacklightRangeLimit {
       throw new Error("BlacklightRangeLimit missing argument")
     }
 
-    this.distributionElement = container.querySelector(".profile .distribution")
+    this.distributionElement = container.querySelector(".distribution")
 
     // If there is no distribution element on page, it means we don't have data,
     // nothing to do.
@@ -78,10 +80,13 @@ export default class BlacklightRangeLimit {
       this.whenBecomesVisible(container, target => this.setup());
     }
 
-    if (this.container.getAttribute("data-chart-replaces-text") == "false") {
-      this.chartReplacesText = false;
+    if (this.container.getAttribute("data-textual-facets") == "false") {
+      this.textualFacets = false;
     }
-
+    if (this.container.getAttribute("data-textual-facets-collapsible") == "false") {
+      this.textualFacetsCollapsible = false;
+    }
+    this.rangeListHeadingLocalized = this.container.getAttribute("data-range-list-heading-localized") || "Range List";
   }
 
   // if the range fetch link is still in DOM, fetch ranges from back-end,
@@ -96,7 +101,7 @@ export default class BlacklightRangeLimit {
 
     // What we'll do to put the chart on page whether or not we need to load --
     // when query has range limits, we don't need to load, it's already there.
-    let handleOnPageData = () => {
+    let conditonallySetupChart = () => {
       if (this.distributionElement.classList.contains("chart_js")) {
         this.extractBucketData();
         this.chartCanvasElement = this.setupDomForChart();
@@ -111,13 +116,14 @@ export default class BlacklightRangeLimit {
         then( response => response.ok ? response.text() : Promise.reject(response)).
         then( responseBody => new DOMParser().parseFromString(responseBody, "text/html")).
         then( responseDom => responseDom.querySelector(".facet-values")).
-        then( element =>  this.distributionElement.innerHTML = element.outerHTML  ).
-        then( _ => { handleOnPageData()  }).
+        then( element => this.placeFacetValuesListElement(element)).
+        then( _ => { conditonallySetupChart()  }).
         catch( error => {
           console.error(error);
         });
     } else {
-      handleOnPageData();
+      this.placeFacetValuesListElement(this.distributionElement.querySelector(".facet-values"));
+      conditonallySetupChart();
     }
   }
 
@@ -161,6 +167,32 @@ export default class BlacklightRangeLimit {
     return undefined;
   }
 
+  // Take HTML element with facet list values
+  //
+  // Possibly hide or wrap it with open/close disclosure, depending on
+  // configuration.
+  //
+  // Place it onto page.
+  placeFacetValuesListElement(listElement) {
+    if (!listElement) {
+      return;
+    }
+
+    listElement.classList.add("mt-3");
+
+    if (! this.textualFacets) {
+      listElement.style["display"] = "none"
+    } else if (this.textualFacetsCollapsible) {
+      const detailsEl = this.container.ownerDocument.createElement("details");
+      detailsEl.innerHTML = "<summary>" + this.rangeListHeadingLocalized + "</summary>";
+      detailsEl.classList.add("mt-4", "text-muted");
+      detailsEl.appendChild( listElement );
+      listElement = detailsEl;
+    }
+
+    this.distributionElement.innerHTML  = listElement.outerHTML;
+  }
+
   setupDomForChart() {
     if(this.chartCanvasElement) {
       // already there, we're good.
@@ -170,11 +202,13 @@ export default class BlacklightRangeLimit {
     const listDiv = this.distributionElement.querySelector(".facet-values");
     const wrapperDiv = this.container.querySelector("*[data-chart-wrapper=true]");
 
-    if (this.chartReplacesText) {
-      // We keep the textual facet data as accessible screen-reader, add .sr-only to it though
-      listDiv.classList.add("sr-only")
-      listDiv.classList.add("visually-hidden");
-    }
+
+
+    // if (this.chartReplacesText) {
+    //   // We keep the textual facet data as accessible screen-reader, add .sr-only to it though
+    //   listDiv.classList.add("sr-only")
+    //   listDiv.classList.add("visually-hidden");
+    // }
 
     // We create a <chart>, insert it into DOM in wrapper
     this.chartCanvasElement = this.container.ownerDocument.createElement("canvas");

app/components/blacklight_range_limit/range_facet_component.html.erb

@@ -7,43 +7,41 @@
     <div class="limit_content range_limit <%= @facet_field.key %>-config blrl-plot-config"
         data-chart-segment-border-color="<%= range_config[:chart_segment_border_color] %>"
         data-chart-segment-bg-color="<%= range_config[:chart_segment_bg_color] %>"
-        data-chart-replaces-text="<%= range_config[:chart_replaces_text] %>"
+        data-textual-facets="<%= !! range_config[:textual_facets] %>"
+        data-textual-facets-collapsible="<%= !! range_config[:textual_facets_collapsible] %>"
+        data-range-list-heading-localized="<%= t('blacklight.range_limit.range_list_heading') %>"
     >
       <% if @facet_field.selected_range_facet_item %>
         <%= render BlacklightRangeLimit::RangeSegmentsComponent.new(facet_field: @facet_field, facet_items: [@facet_field.selected_range_facet_item], classes: ['current', 'mb-3']) %>
       <% end %>
 
       <!-- no results profile if missing is selected -->
       <% unless @facet_field.missing_selected? %>
-        <!-- this has to be on page if you want calculated facets to show up, JS sniffs it. -->
-        <div class="profile mb-3">
-          <%# if was very hard to get chart.js to be succesfully resonsive, required this wrapper!
-          https://github.com/chartjs/Chart.js/issues/11005 %>
-          <div class="chart-wrapper" data-chart-wrapper="true" style="display: none; position: relative; width: 100%; aspect-ratio: <%= range_config[:chart_aspect_ratio] %>;">
-          </div>
-
-          <% if (min = @facet_field.min) &&
-                (max = @facet_field.max) %>
-
-            <% if range_config[:segments] != false %>
-              <div class="distribution subsection <%= 'chart_js' unless range_config[:chart_js] == false %>">
-                <!-- if  we already fetched segments from solr, display them
-                     here. Otherwise, display a link to fetch them, which JS
-                     will AJAX fetch.  -->
-                <% if @facet_field.range_queries.any? %>
-                  <%= render BlacklightRangeLimit::RangeSegmentsComponent.new(facet_field: @facet_field) %>
-                <% else %>
-                  <%= link_to(t('blacklight.range_limit.view_distribution'), range_limit_url(range_start: min, range_end: max), class: "load_distribution", "data-loading-message-html": t('blacklight.range_limit.loading_html')) %>
-                <% end %>
-              </div>
-            <% end %>
-          <% end %>
+        <%# this has to be on page if you want calculated facets to show up, JS sniffs it.
+          it was very hard to get chart.js to be succesfully resonsive, required this wrapper!
+          https://github.com/chartjs/Chart.js/issues/11005 -%>
+        <div class="chart-wrapper mb-3" data-chart-wrapper="true" style="display: none; position: relative; width: 100%; aspect-ratio: <%= range_config[:chart_aspect_ratio] %>;">
         </div>
 
         <%= render BlacklightRangeLimit::RangeFormComponent.new(facet_field: @facet_field, classes: @classes) %>
 
-        <% if @facet_field.missing_facet_item && !request.xhr? && range_config[:segments] != false %>
-          <%= render BlacklightRangeLimit::RangeSegmentsComponent.new(facet_field: @facet_field, facet_items: [@facet_field.missing_facet_item], classes: ['missing', 'subsection', 'mt-3']) %>
+        <% if uses_distribution? &&
+              (min = @facet_field.min) &&
+              (max = @facet_field.max) %>
+          <div class="distribution <%= 'chart_js' unless range_config[:chart_js] == false %>">
+            <!-- if  we already fetched segments from solr, display them
+                 here. Otherwise, display a link to fetch them, which JS
+                 will AJAX fetch.  -->
+            <% if @facet_field.range_queries.any? %>
+              <%= render BlacklightRangeLimit::RangeSegmentsComponent.new(facet_field: @facet_field) %>
+            <% else %>
+              <%= link_to(t('blacklight.range_limit.view_distribution'), range_limit_url(range_start: min, range_end: max), class: "load_distribution", "data-loading-message-html": t('blacklight.range_limit.loading_html')) %>
+            <% end %>
+          </div>
+        <% end %>
+
+        <% if @facet_field.missing_facet_item && !request.xhr? && uses_distribution? %>
+          <%= render BlacklightRangeLimit::RangeSegmentsComponent.new(facet_field: @facet_field, facet_items: [@facet_field.missing_facet_item], classes: ['missing', 'mt-3']) %>
         <% end %>
       <% end %>
     </div>

app/components/blacklight_range_limit/range_facet_component.rb

@@ -17,5 +17,9 @@ def range_config
     def range_limit_url(options = {})
       helpers.main_app.url_for(@facet_field.search_state.to_h.merge(range_field: @facet_field.key, action: 'range_limit').merge(options))
     end
+
+    def uses_distribution?
+      range_config[:chart_js] || range_config[:textual_facets]
+    end
   end
 end

config/locales/blacklight_range_limit.ar.yml

@@ -9,6 +9,7 @@ ar:
       range_end: "%{field_label} نهاية المدة"
       range_end_short: نهاية
       range_html: <span class="from" data-blrl-begin="%{begin_value}">%{begin}</span> الى <span class="to" data-blrl-end="%{end_value}">%{end}</span>
+      range_list_heading: قائمة النطاق
       remove_limit: حذف
       results_range_html: تمتد النتائج الحالية من <span class="min">%{min}</span> إلى<span class="max">%{max}</span>
       single_html: <span class="single" data-blrl-single="%{begin_value}">%{begin}</span>

config/locales/blacklight_range_limit.de.yml

@@ -9,6 +9,7 @@ de:
       range_end: "%{field_label} Bereichsende"
       range_end_short: Ende
       range_html: <span class="from" data-blrl-begin="%{begin_value}">%{begin}</span> bis <span class="to" data-blrl-end="%{end_value}">%{end}</span>
+      range_list_heading: Bereichsliste
       remove_limit: Entfernen
       results_range_html: Aktuelle Ergebnisse reichen von <span class="min">%{min}</span> bis <span class="max">%{max}</span>
       single_html: <span class="single" data-blrl-single="%{begin_value}">%{begin}</span>

config/locales/blacklight_range_limit.en.yml

@@ -14,3 +14,4 @@ en:
       single_html: '<span class="single" data-blrl-single="%{begin_value}">%{begin}</span>'
       range_html: '<span class="from" data-blrl-begin="%{begin_value}">%{begin}</span> to <span class="to" data-blrl-end="%{end_value}">%{end}</span>'
       loading_html: "Loading..."
+      range_list_heading: "Range List"

config/locales/blacklight_range_limit.es.yml

@@ -9,6 +9,7 @@ es:
       range_end: Fin del rango %{field_label}
       range_end_short: Fin
       range_html: <span class="from" data-blrl-begin="%{begin_value}">%{begin}</span> a <span class="to" data-blrl-end="%{end_value}">%{end}</span>
+      range_list_heading: Lista de rangos
       remove_limit: eliminar
       results_range_html: Los resultados actuales varían de <span class="min">%{min}</span> a <span class="max">%{max}</span>
       single_html: <span class="single" data-blrl-single="%{begin_value}">%{begin}</span>

config/locales/blacklight_range_limit.it.yml

@@ -9,6 +9,7 @@ it:
       range_end: "%{field_label} a"
       range_end_short: Fine
       range_html: <span class="from" data-blrl-begin="%{begin_value}">%{begin}</span> a <span class="to" data-blrl-end="%{end_value}">%{end}</span>
+      range_list_heading: Elenco intervalli
       remove_limit: cancella
       results_range_html: Risultati attuali vanno da <span class="min">%{min}</span> a <span class="max">%{max}</span>
       single_html: <span class="single" data-blrl-single="%{begin_value}">%{begin}</span>

lib/blacklight_range_limit.rb

@@ -15,7 +15,7 @@ class InvalidRange < TypeError; end
   mattr_accessor :classes
 
   self.classes = {
-    form: 'range_limit_form subsection',
+    form: 'range_limit_form',
     submit: 'submit btn btn-sm btn-secondary'
   }
 
@@ -25,11 +25,11 @@ def self.default_range_config
       range_config: {
         num_segments: 10,
         chart_js: true,
+        textual_facets: true,
+        textual_facets_collapsible: true,
         chart_segment_border_color: 'rgb(54, 162, 235)',
         chart_segment_bg_color: 'rgba(54, 162, 235, 0.5)',
         chart_aspect_ratio: 2,
-        segments: true,
-        chart_replaces_text: true,
         assumed_boundaries: nil
       },
       filter_class: BlacklightRangeLimit::FilterField,

lib/blacklight_range_limit/range_limit_builder.rb

@@ -22,7 +22,7 @@ def add_range_limit_params(solr_params)
         solr_params["stats.field"] << config.field
 
         range_config = config.range_config
-        next if range_config[:segments] == false
+        next unless range_config[:chart_js] || range_config[:textual_facets]
 
         selected_value = search_state.filter(config.key).values.first
         range = (selected_value if selected_value.is_a? Range) || range_config[:assumed_boundaries]

spec/components/range_facet_component_spec.rb

@@ -26,7 +26,7 @@
       min: nil,
       max: nil,
       search_state: Blacklight::SearchState.new({}, nil),
-      range_config: {},
+      range_config: BlacklightRangeLimit.default_range_config[:range_config],
       modal_path: nil,
       facet_field: facet_config,
       **facet_field_params,
@@ -50,11 +50,6 @@
       .and have_selector('div.collapse')
   end
 
-  # This is JS api
-  it 'renders a placeholder profile area' do
-    expect(rendered).to have_selector('div.profile', text: '')
-  end
-
   context 'with min/max' do
     let(:facet_field_params) do
       {
@@ -68,7 +63,7 @@
     it "renders a link to fetch distribution info" do
       # need request_url for routing of links generated
       with_request_url '/catalog' do
-        expect(rendered).to have_selector("a.load_distribution[href]")
+        expect(rendered).to have_selector(".distribution a.load_distribution[href]")
       end
     end
   end
@@ -86,9 +81,9 @@
     end
 
     it 'renders the range data into the profile' do
-      expect(rendered).to have_selector('.profile li', count: 2)
-        .and have_selector('.profile li', text: '100 to 199')
-        .and have_selector('.profile li', text: '200 to 300')
+      expect(rendered).to have_selector('.distribution li', count: 2)
+        .and have_selector('.distribution li', text: '100 to 199')
+        .and have_selector('.distribution li', text: '200 to 300')
     end
   end
 

spec/features/run_through_spec.rb

@@ -38,7 +38,7 @@
       expect(find("input#range_pub_date_si_end").value).to be_present
 
       # expect "missing" facet
-      within 'ul.subsection.missing' do
+      within 'ul.missing' do
         expect(page).to have_link '[Missing]'
       end