From c11a5d565dfac58c5d1032822c4adb2c850712d3 Mon Sep 17 00:00:00 2001
From: Andrew Yan <ayan@usgs.gov>
Date: Fri, 9 Feb 2018 13:26:22 -0600
Subject: [PATCH] rework legend marker creation

---
 .../scripts/components/hydrograph/index.js    |  7 +-
 .../scripts/components/hydrograph/legend.js   | 79 +++++++++++-------
 .../components/hydrograph/legend.spec.js      | 83 ++++++++++++++-----
 3 files changed, 116 insertions(+), 53 deletions(-)

diff --git a/assets/src/scripts/components/hydrograph/index.js b/assets/src/scripts/components/hydrograph/index.js
index 351bd7e3e..06dc5796b 100644
--- a/assets/src/scripts/components/hydrograph/index.js
+++ b/assets/src/scripts/components/hydrograph/index.js
@@ -14,7 +14,7 @@ const { ASPECT_RATIO_PERCENT, MARGIN, CIRCLE_RADIUS, layoutSelector } = require(
 const { pointsSelector, lineSegmentsSelector, isVisibleSelector } = require('./points');
 const { xScaleSelector, yScaleSelector } = require('./scales');
 const { Actions, configureStore } = require('./store');
-const { drawSimpleLegend, legendDisplaySelector } = require('./legend');
+const { drawSimpleLegend, legendDisplaySelector, createLegendMarkers } = require('./legend');
 
 
 // Function that returns the left bounding point for a given chart point.
@@ -123,8 +123,9 @@ const plotTooltips = function (elem, {xScale, yScale, data}) {
 };
 
 
-const plotLegend = function(elem, {markers}) {
+const plotLegend = function(elem, {displayItems}) {
     elem.select('.legend').remove();
+    let markers = createLegendMarkers(displayItems);
     drawSimpleLegend(elem, markers);
 };
 
@@ -183,7 +184,7 @@ const timeSeriesGraph = function (elem) {
                 isInteractive: () => true
             })))
             .call(link(plotLegend, createStructuredSelector({
-                markers: legendDisplaySelector
+                displayItems: legendDisplaySelector
             })))
             .append('g')
                 .attr('transform', `translate(${MARGIN.left},${MARGIN.top})`)
diff --git a/assets/src/scripts/components/hydrograph/legend.js b/assets/src/scripts/components/hydrograph/legend.js
index aa1d7a316..a72688d4c 100644
--- a/assets/src/scripts/components/hydrograph/legend.js
+++ b/assets/src/scripts/components/hydrograph/legend.js
@@ -3,6 +3,7 @@ const { createSelector } = require('reselect');
 const { defineLineMarker, defineCircleMarker } = require('./markers');
 const { CIRCLE_RADIUS } = require('./layout');
 
+
 /**
  * Create a simple horizontal legend
  *
@@ -72,13 +73,53 @@ function drawSimpleLegend(svg,
     legend.attr('transform', `translate(${legendXPosition}, ${svgBBox.height-15})`);
 }
 
+/**
+ * create elements for the legend in the svg
+ *
+ * @param dataPlotElements
+ */
+const createLegendMarkers = function(dataPlotElements) {
+    let text;
+    let marker;
+    let legendMarkers = [];
+    for (let dataItem of dataPlotElements.dataItems) {
+        if (dataItem === 'compare' || dataItem === 'current') {
+            text = 'Current Year';
+            let domId = `ts-${dataItem}`;
+            let svgGroup = `${dataItem}-line-marker`;
+            if (dataItem === 'compare') {
+                text = 'Last Year';
+            }
+            marker = defineLineMarker(domId, 'line', text, svgGroup);
+        }
+        else if (dataItem === 'medianStatistics') {
+            text = 'Median Discharge';
+            let beginYear = dataPlotElements.metadata.statistics.beginYear;
+            let endYear = dataPlotElements.metadata.statistics.endYear;
+            if (beginYear && endYear) {
+                text = `${text} ${beginYear} - ${endYear}`;
+            }
+            marker = defineCircleMarker(CIRCLE_RADIUS, null, 'median-data-series', text, 'median-circle-marker');
+        }
+        else {
+            marker = null;
+        }
+        if (marker) {
+            legendMarkers.push(marker);
+        }
+    }
+    return legendMarkers;
+};
 
+/**
+ * Select attributes from the state useful for legend creation
+ */
 const legendDisplaySelector = createSelector(
     (state) => state.showSeries,
     (state) => state.statisticalMetaData,
     (showSeries, statisticalMetaData) => {
         let shownSeries = [];
-        let displayMarkers = [];
+        let dataPlotElements = {};
         let text;
         let marker;
         for (let key in showSeries) {
@@ -88,35 +129,17 @@ const legendDisplaySelector = createSelector(
                 }
             }
         }
-        for (let seriesName of shownSeries) {
-            if (seriesName === 'compare' || seriesName === 'current') {
-                text = 'Current Year';
-                let domId = `ts-${seriesName}`;
-                let svgGroup = `${seriesName}-line-marker`;
-                if (seriesName === 'compare') {
-                    text = 'Last Year';
-                }
-                marker = defineLineMarker(domId, 'line', text, svgGroup);
-            }
-            else if (seriesName === 'medianStatistics') {
-                text = 'Median Discharge';
-                let beginYear = statisticalMetaData.beginYear;
-                let endYear = statisticalMetaData.endYear;
-                if (beginYear && endYear) {
-                    text = `Median Discharge ${beginYear} - ${endYear}`;
-                }
-                marker = defineCircleMarker(CIRCLE_RADIUS, null, 'median-data-series', text, 'median-circle-marker');
-            }
-            else {
-                marker = null;
-            }
-            if (marker) {
-                displayMarkers.push(marker);
+
+        dataPlotElements.dataItems = shownSeries;
+        dataPlotElements.metadata = {
+            statistics: {
+                beginYear: statisticalMetaData.beginYear ? statisticalMetaData.beginYear : undefined,
+                endYear: statisticalMetaData.endYear ? statisticalMetaData.endYear : undefined
             }
-        }
-        return displayMarkers;
+        };
+        return dataPlotElements;
     }
 );
 
 
-module.exports = {drawSimpleLegend, legendDisplaySelector};
+module.exports = {drawSimpleLegend, createLegendMarkers, legendDisplaySelector};
diff --git a/assets/src/scripts/components/hydrograph/legend.spec.js b/assets/src/scripts/components/hydrograph/legend.spec.js
index cbe2a4890..0bf320d91 100644
--- a/assets/src/scripts/components/hydrograph/legend.spec.js
+++ b/assets/src/scripts/components/hydrograph/legend.spec.js
@@ -1,7 +1,7 @@
 const { namespaces } = require('d3');
 const { select, selectAll } = require('d3-selection');
 
-const { drawSimpleLegend, legendDisplaySelector } = require('./legend');
+const { drawSimpleLegend, legendDisplaySelector, createLegendMarkers } = require('./legend');
 const { lineMarker, circleMarker } = require('./markers');
 
 describe('Legend module', () => {
@@ -57,20 +57,17 @@ describe('Legend module', () => {
 
     });
 
-    describe('legendDisplaySelector', () => {
+    describe('createLegendMarkers', () => {
 
-        it('should return a marker if a time series is shown', () => {
-            let result = legendDisplaySelector({
-                showSeries:
-                    {
-                        current: true,
-                        compare: false,
-                        medianStatistics: true
-                    },
-                statisticalMetaData: {
-                    'beginYear': 2010,
-                    'endYear': 2012
+        it('should return markers for display', () => {
+            let result = createLegendMarkers({
+                dataItems: ['current', 'medianStatistics'],
+                metadata: {
+                    statistics: {
+                        beginYear: 2010,
+                        endYear: 2012
                     }
+                }
             });
             expect(result).toEqual([
                 {
@@ -92,17 +89,56 @@ describe('Legend module', () => {
         });
 
         it('should return an empty array if keys do not match', () => {
+            let result = createLegendMarkers({
+                dataItems: ['blah1', 'blah2'],
+                metadata: {
+                    statistics: {
+                        beginYear: 2010,
+                        endYear: 2012
+                    }
+                }
+            });
+            expect(result.length).toEqual(0);
+        });
+
+        it('should still work if stat begin and end years are absent', () => {
+            let result = createLegendMarkers({
+                dataItems: ['medianStatistics'],
+                metadata: {
+                    statistics: {
+                        beginYear: undefined,
+                        endYear: undefined
+                    }
+                }
+            });
+            expect(result[0].text).toEqual('Median Discharge');
+        })
+    });
+
+    describe('legendDisplaySelector', () => {
+
+        it('should return a marker if a time series is shown', () => {
             let result = legendDisplaySelector({
-                showSeries: {
-                    blah: true,
-                    blah2: true
-                },
+                showSeries:
+                    {
+                        current: true,
+                        compare: false,
+                        medianStatistics: true
+                    },
                 statisticalMetaData: {
-                    beginYear: 2010,
-                    endYear: 2012
+                    'beginYear': 2010,
+                    'endYear': 2012
+                    }
+            });
+            expect(result).toEqual({
+                dataItems: ['current', 'medianStatistics'],
+                metadata: {
+                    statistics: {
+                        beginYear: 2010,
+                        endYear: 2012
+                    }
                 }
-            }) ;
-            expect(result.length).toEqual(0);
+            });
         });
 
         it('should not choke if statisticalMetadata years are absent', () => {
@@ -113,7 +149,10 @@ describe('Legend module', () => {
                     },
                 statisticalMetaData: {}
             });
-            expect(result[0].text).toEqual('Median Discharge');
+            expect(result.metadata.statistics).toEqual({
+                beginYear: undefined,
+                endYear: undefined
+            });
         });
     });
 
-- 
GitLab