From 3eccbe24d57b71c53525b55dbfd5751fc87d1bd9 Mon Sep 17 00:00:00 2001
From: Mary Bucknell <mbucknell@usgs.gov>
Date: Mon, 26 Mar 2018 14:08:15 -0500
Subject: [PATCH] Refactored the code that was setting the visibility of the
time series graph and the code that set the disabled state of the compare
toggle. Fixed tests
---
.../scripts/components/hydrograph/index.js | 72 +++++++-----------
.../components/hydrograph/index.spec.js | 73 +++++++------------
.../components/hydrograph/timeseries.js | 1 +
.../components/hydrograph/timeseries.spec.js | 4 +-
4 files changed, 58 insertions(+), 92 deletions(-)
diff --git a/assets/src/scripts/components/hydrograph/index.js b/assets/src/scripts/components/hydrograph/index.js
index 3f1c5174d..bcace6c0f 100644
--- a/assets/src/scripts/components/hydrograph/index.js
+++ b/assets/src/scripts/components/hydrograph/index.js
@@ -1,6 +1,7 @@
/**
* Hydrograph charting module.
*/
+
const { select } = require('d3-selection');
const { extent } = require('d3-array');
const { line: d3Line } = require('d3-shape');
@@ -274,24 +275,6 @@ const plotSROnlyTable = function (elem, {tsKey, variable, methods, visible, data
}
};
-/**
- * Determine if the last year checkbox should be enabled or disabled
- *
- * @param elem
- * @param compareTimeseries
- */
-const controlLastYearSelect = function(elem, compareTimeseries) {
- const comparePoints = compareTimeseries[Object.keys(compareTimeseries)[0]] ? compareTimeseries[Object.keys(compareTimeseries)[0]].points : [];
- let checkbox = elem.select('.hydrograph-last-year-input');
- if (comparePoints.length > 0) {
- checkbox.property('disabled', false);
- } else {
- checkbox
- .property('disabled', true)
- .property('checked', false)
- .dispatch('change');
- }
-};
const createTitle = function(elem) {
elem.append('div')
@@ -300,32 +283,10 @@ const createTitle = function(elem) {
elem.html(title);
}, titleSelector));
};
-/**
- * Modify styling to hide or display the plot area.
- *
- * @param elem
- * @param currentTimeseries
- */
-const controlGraphDisplay = function (elem, currentTimeseries) {
- const seriesWithPoints = Object.values(currentTimeseries).filter(x => x.points.length > 0);
- if (seriesWithPoints.length === 0) {
- elem.attr('hidden', true);
- } else {
- // the div.compare-container is set to not display by default
- // this prevents the user from seeing the checkbox in the
- // time between the html loading and the javascript loading;
- // if there are timeseries available, the div.compare-container
- // should be displayed
- elem.select('div.compare-container').attr('hidden', null);
- elem.attr('hidden', null);
- }
-};
const timeSeriesGraph = function (elem) {
- elem.call(link(controlGraphDisplay, timeSeriesSelector('current')))
- .call(link(controlLastYearSelect, currentVariableTimeSeriesSelector('compare')))
- .append('div')
+ elem.append('div')
.attr('class', 'hydrograph-container')
.call(createTitle)
.append('svg')
@@ -420,17 +381,37 @@ const graphControls = function(elem) {
.on('click', dispatch(function() {
return Actions.toggleTimeseries('compare', this.checked);
}))
- .call(link(function(elem, {checked}) {
+ .call(link(function(elem, compareTimeseries) {
+ const exists = Object.keys(compareTimeseries) ?
+ Object.values(compareTimeseries).filter(tsValues => tsValues.points.length).length > 0 : false;
+ elem.property('disabled', !exists);
+ if (!exists) {
+ dispatch(function () {
+ return Actions.toggleTimeseries('compare', false);
+ });
+ }
+ }, currentVariableTimeSeriesSelector('compare')))
+ .call(link(function(elem, checked) {
elem.property('checked', checked);
- }, createStructuredSelector({
- checked: isVisibleSelector('compare')
- })));
+ }, isVisibleSelector('compare')));
compareControlDiv.append('label')
.attr('id', 'last-year-label')
.attr('for', 'last-year-checkbox')
.text('Show last year');
};
+/**
+ * Modify styling to hide or display the plot area.
+ *
+ * @param elem
+ * @param currentTimeseries
+ */
+const controlGraphDisplay = function (elem, currentTimeseries) {
+ const seriesWithPoints = Object.values(currentTimeseries).filter(x => x.points.length > 0);
+ elem.attr('hidden', seriesWithPoints.length === 0 ? true : null);
+};
+
+
const attachToNode = function (store, node, {siteno} = {}) {
if (!siteno) {
@@ -441,6 +422,7 @@ const attachToNode = function (store, node, {siteno} = {}) {
store.dispatch(Actions.resizeUI(window.innerWidth, node.offsetWidth));
select(node)
.call(provide(store))
+ .call(link(controlGraphDisplay, timeSeriesSelector('current')))
.call(timeSeriesGraph)
.call(cursorSlider);
select(node).append('div')
diff --git a/assets/src/scripts/components/hydrograph/index.spec.js b/assets/src/scripts/components/hydrograph/index.spec.js
index b9dbcd633..58c14609c 100644
--- a/assets/src/scripts/components/hydrograph/index.spec.js
+++ b/assets/src/scripts/components/hydrograph/index.spec.js
@@ -1,5 +1,5 @@
const { select, selectAll } = require('d3-selection');
-const { provide, dispatch } = require('../../lib/redux');
+const { provide } = require('../../lib/redux');
const { attachToNode, timeSeriesGraph, timeSeriesLegend } = require('./index');
const { Actions, configureStore } = require('../../store');
@@ -119,7 +119,7 @@ const TEST_STATE = {
currentVariableID: '45807197',
showSeries: {
current: true,
- compare: false,
+ compare: true,
median: true
},
width: 400
@@ -173,7 +173,6 @@ describe('Hydrograph charting module', () => {
select(graphNode)
.call(provide(store))
.call(timeSeriesGraph);
- expect(select('#hydrograph').attr('hidden')).toBeTruthy();
});
});
@@ -237,7 +236,7 @@ describe('Hydrograph charting module', () => {
},
showSeries: {
current: true,
- compare: false,
+ compare: true,
median: true
},
title: 'My Title',
@@ -303,38 +302,6 @@ describe('Hydrograph charting module', () => {
//TODO: Consider adding a test which checks that the y axis is rescaled by
// examining the contents of the text labels.
- describe('Adding and removing compare time series', () => {
- let store;
- beforeEach(() => {
- store = configureStore(TEST_STATE);
- attachToNode(store, graphNode, {siteno: '12345678'});
- });
-
- it('Should render the compare toggle unchecked', () => {
- const checkbox = select('#last-year-checkbox');
- expect(checkbox.size()).toBe(1);
- expect(checkbox.property('checked')).toBe(false);
- });
-
- it('Should render the compare toggle checked', () => {
- store.dispatch(Actions.toggleTimeseries('compare', true));
- const checkbox = select('#last-year-checkbox');
- expect(checkbox.size()).toBe(1);
- expect(checkbox.property('checked')).toBe(true);
- });
-
- it('Should render one lines', () => {
- store.dispatch(Actions.toggleTimeseries('compare', true));
- expect(selectAll('#ts-compare-group .line-segment').size()).toBe(1);
- });
-
- it('Should remove the lines when removing the compare time series', () => {
- store.dispatch(Actions.toggleTimeseries('compare', true));
- store.dispatch(Actions.toggleTimeseries('compare', false));
- expect(selectAll('#ts-compare-group .line-segment').size()).toBe(0);
- });
- });
-
describe('legends should render', () => {
let store;
beforeEach(() => {
@@ -365,22 +332,38 @@ describe('Hydrograph charting module', () => {
let store;
beforeEach(() => {
store = configureStore(TEST_STATE);
- select(graphNode)
- .call(provide(store))
- .call(timeSeriesGraph)
- .select('.hydrograph-last-year-input')
- .on('change', dispatch(function () {
- return Actions.toggleTimeseries('compare', this.checked);
- }));
+ attachToNode(store, graphNode, {siteno: '12345678'});
+ });
+
+ it('Should render the compare toggle checked', () => {
+ const checkbox = select('#last-year-checkbox');
+ expect(checkbox.size()).toBe(1);
+ expect(checkbox.property('checked')).toBe(true);
+ });
+
+ it('Should render the compare toggle unchecked', () => {
+ store.dispatch(Actions.toggleTimeseries('compare', false));
+ const checkbox = select('#last-year-checkbox');
+ expect(checkbox.size()).toBe(1);
+ expect(checkbox.property('checked')).toBe(false);
});
it('should be enabled if there are last year data', () => {
- expect(select('.hydrograph-last-year-input').property('disabled')).toBeFalsy();
+ expect(select('#last-year-checkbox').property('disabled')).toBeFalsy();
});
it('should be disabled if there are no last year data', () => {
store.dispatch(Actions.setCurrentParameterCode('00010', '45807190'));
- expect(select('.hydrograph-last-year-input').property('disabled')).toBeTruthy();
+ expect(select('#last-year-checkbox').property('disabled')).toBeTruthy();
+ });
+
+ it('Should render one lines', () => {
+ expect(selectAll('#ts-compare-group .line-segment').size()).toBe(1);
+ });
+
+ it('Should remove the lines when removing the compare time series', () => {
+ store.dispatch(Actions.toggleTimeseries('compare', false));
+ expect(selectAll('#ts-compare-group .line-segment').size()).toBe(0);
});
});
});
diff --git a/assets/src/scripts/components/hydrograph/timeseries.js b/assets/src/scripts/components/hydrograph/timeseries.js
index 2161ef541..1a674ceb0 100644
--- a/assets/src/scripts/components/hydrograph/timeseries.js
+++ b/assets/src/scripts/components/hydrograph/timeseries.js
@@ -78,6 +78,7 @@ export const currentVariableTimeSeriesSelector = memoize(tsKey => createSelector
}
));
+
/**
* Returns a selector that, for a given tsKey:
* Selects all time series data.
diff --git a/assets/src/scripts/components/hydrograph/timeseries.spec.js b/assets/src/scripts/components/hydrograph/timeseries.spec.js
index 37a3e6832..94081aa98 100644
--- a/assets/src/scripts/components/hydrograph/timeseries.spec.js
+++ b/assets/src/scripts/components/hydrograph/timeseries.spec.js
@@ -1,6 +1,6 @@
const { variablesSelector, currentVariableSelector, timeSeriesSelector, isVisibleSelector, yLabelSelector,
- titleSelector, descriptionSelector, currentVariableTimeSeriesSelector, allTimeSeriesSelector,
- requestTimeRangeSelector} = require('./timeseries');
+ titleSelector, descriptionSelector, currentVariableTimeSeriesSelector,
+ allTimeSeriesSelector, requestTimeRangeSelector} = require('./timeseries');
const TEST_DATA = {
--
GitLab