From dd24a75cfd3f3aa10e65abd184c41e784612f2b3 Mon Sep 17 00:00:00 2001
From: Janell Fry <Janell_Fry@usgs.gov>
Date: Fri, 13 Mar 2020 11:17:53 -0700
Subject: [PATCH] IOW-307 Fixing few peer review comments from Mary.
---
.../components/dailyValueHydrograph/legend.js | 80 +------------
.../selectors/legend-data.spec.js | 87 ++-------------
.../scripts/components/hydrograph/layout.js | 4 +-
.../scripts/components/hydrograph/legend.js | 78 +------------
.../components/hydrograph/legend.spec.js | 9 +-
assets/src/scripts/d3-rendering/legend.js | 77 +++++++++++++
.../src/scripts/d3-rendering/legend.spec.js | 105 ++++++++++++++++++
assets/src/scripts/index.spec.js | 1 +
8 files changed, 201 insertions(+), 240 deletions(-)
create mode 100644 assets/src/scripts/d3-rendering/legend.js
create mode 100644 assets/src/scripts/d3-rendering/legend.spec.js
diff --git a/assets/src/scripts/components/dailyValueHydrograph/legend.js b/assets/src/scripts/components/dailyValueHydrograph/legend.js
index f957ae84b..39500d89b 100644
--- a/assets/src/scripts/components/dailyValueHydrograph/legend.js
+++ b/assets/src/scripts/components/dailyValueHydrograph/legend.js
@@ -1,88 +1,10 @@
// functions to facilitate DV legend creation for a d3 plot
import {createStructuredSelector} from 'reselect';
-
import {getLayout} from './selectors/layout';
import {getLegendMarkerRows} from './selectors/legend-data';
-
-import config from '../../config';
-import {mediaQuery} from '../../utils';
+import {drawSimpleLegend} from '../../d3-rendering/legend';
import {link} from '../../lib/d3-redux';
-
-/**
- * Create a simple legend
- *
- * @param {Object} div - d3 selector where legend should be created
- * @param {Object} legendMarkerRows - Array of rows. Each row should be an array of legend markers.
- * @param {Object} layout - width and height of svg.
- */
-export const drawSimpleLegend = function(div, {legendMarkerRows, layout}) {
- div.selectAll('.legend-svg').remove();
-
- if (!legendMarkerRows.length) {
- return;
- }
-
- const markerGroupXOffset = 15;
- const verticalRowOffset = 18;
-
- let svg = div.append('svg')
- .attr('class', 'legend-svg');
- let legend = svg
- .append('g')
- .attr('class', 'legend')
- .attr('transform', `translate(${mediaQuery(config.USWDS_MEDIUM_SCREEN) ? layout.margin.left : 0}, 0)`);
-
- legendMarkerRows.forEach((rowMarkers, rowIndex) => {
- let xPosition = 0;
- let yPosition = verticalRowOffset * (rowIndex + 1);
-
- rowMarkers.forEach((marker) => {
- let markerArgs = {
- x: xPosition,
- y: yPosition,
- text: marker.text,
- domId: marker.domId,
- domClass: marker.domClass,
- width: 20,
- height: 10,
- length: 20,
- r: marker.r,
- fill: marker.fill
- };
-
- let markerGroup = marker.type(legend, markerArgs);
- let markerGroupBBox;
- // Long story short, firefox is unable to get the bounding box if
- // the svg element isn't actually taking up space and visible. Folks on the
- // internet seem to have gotten around this by setting `visibility:hidden`
- // to hide things, but that would still mean the elements will take up space.
- // which we don't want. So, here's some error handling for getBBox failures.
- // This handling ends up not creating the legend, but that's okay because the
- // graph is being shown anyway. A more detailed discussion of this can be found at:
- // https://bugzilla.mozilla.org/show_bug.cgi?id=612118 and
- // https://stackoverflow.com/questions/28282295/getbbox-of-svg-when-hidden.
- try {
- markerGroupBBox = markerGroup.node().getBBox();
- xPosition = markerGroupBBox.x + markerGroupBBox.width + markerGroupXOffset;
-
- } catch(error) {
- // See above explanation
- }
- });
- });
-
- // Set the size of the containing svg node to the size of the legend.
- let bBox;
- try {
- bBox = legend.node().getBBox();
- } catch(error) {
- return;
- }
- svg.attr('viewBox', `-4 0 ${layout.width} ${bBox.height + 20}`);
-};
-
-
export const drawTimeSeriesLegend = function(elem, store) {
elem.append('div')
.classed('hydrograph-container', true)
diff --git a/assets/src/scripts/components/dailyValueHydrograph/selectors/legend-data.spec.js b/assets/src/scripts/components/dailyValueHydrograph/selectors/legend-data.spec.js
index 0081b66d5..4714c5db4 100644
--- a/assets/src/scripts/components/dailyValueHydrograph/selectors/legend-data.spec.js
+++ b/assets/src/scripts/components/dailyValueHydrograph/selectors/legend-data.spec.js
@@ -1,7 +1,7 @@
import {select, selectAll} from 'd3-selection';
-import {drawSimpleLegend, drawTimeSeriesLegend} from '../legend';
-import {lineMarker, rectangleMarker, textOnlyMarker} from '../../../d3-rendering/markers';
+import {drawTimeSeriesLegend} from '../legend';
import {getLegendMarkerRows} from './legend-data';
+import {lineMarker, textOnlyMarker} from '../../../d3-rendering/markers';
import {configureStore} from '../../../store';
@@ -26,89 +26,18 @@ describe('DV: Legend module', () => {
}
}
},
+ observationsState: {
+ currentTimeSeriesId: '12345',
+ showSeries: {
+ current: true
+ }
+ },
ui: {
windowWidth: 1024,
width: 800
}
};
- describe('DV: drawSimpleLegend', () => {
- let container;
-
- const legendMarkerRows = [
- [{
- type: lineMarker,
- length: 20,
- domId: 'some-id',
- domClass: 'some-class',
- text: 'Some Text'
- }, {
- type: rectangleMarker,
- domId: 'some-rectangle-id',
- domClass: 'some-rectangle-class',
- text: 'Rectangle Marker'
- }],
- [{
- type: textOnlyMarker,
- domId: 'text-id',
- domClass: 'text-class',
- text: 'Label'
- }, {
- type: lineMarker,
- domId: null,
- domClass: 'some-other-class',
- text: 'Median Label'
- }]
- ];
- const layout = {
- width: 100,
- height: 100,
- margin: {
- top: 0,
- right: 0,
- bottom: 0,
- left: 0
- }
- };
-
- beforeEach(() => {
- container = select('body').append('div');
- });
-
- afterEach(() => {
- container.remove();
- });
-
- it('Does not add a legend svg if no markers are provided', () => {
- drawSimpleLegend(container, {
- legendMarkerRows: [],
- layout: layout
- });
-
- expect(container.select('svg').size()).toBe(0);
- });
-
- it('Does not add a legend if the layout is null', () => {
- drawSimpleLegend(container, {
- legendMarkerRows: legendMarkerRows,
- layout: undefined
- });
-
- expect(container.select('svg').size()).toBe(0);
- });
-
- it('Adds a legend when width is provided', () => {
- drawSimpleLegend(container, {legendMarkerRows, layout});
-
- expect(container.select('svg').size()).toBe(1);
- expect(container.selectAll('line').size()).toBe(2);
- expect(container.selectAll('rect').size()).toBe(1);
- expect(container.selectAll('text').size()).toBe(4);
- });
-
- });
-
-
describe('DV: legendMarkerRowSelector', () => {
it('Should return no markers if no time series to show', () => {
diff --git a/assets/src/scripts/components/hydrograph/layout.js b/assets/src/scripts/components/hydrograph/layout.js
index 5f1adcd50..a286966a8 100644
--- a/assets/src/scripts/components/hydrograph/layout.js
+++ b/assets/src/scripts/components/hydrograph/layout.js
@@ -8,8 +8,6 @@ import config from '../../config';
import { mediaQuery } from '../../utils';
import { tickSelector } from './domain';
-
-
export const ASPECT_RATIO = 1 / 2;
export const ASPECT_RATIO_PERCENT = `${100 * ASPECT_RATIO}%`;
const MARGIN = {
@@ -24,7 +22,7 @@ const MARGIN_SMALL_DEVICE = {
bottom: 10,
left: 0
};
-export const CIRCLE_RADIUS = 4;
+//export const CIRCLE_RADIUS = 4;
export const CIRCLE_RADIUS_SINGLE_PT = 1;
export const BRUSH_HEIGHT = 100;
diff --git a/assets/src/scripts/components/hydrograph/legend.js b/assets/src/scripts/components/hydrograph/legend.js
index 296b27aa0..e647f11c4 100644
--- a/assets/src/scripts/components/hydrograph/legend.js
+++ b/assets/src/scripts/components/hydrograph/legend.js
@@ -3,12 +3,12 @@ import {set} from 'd3-collection';
import memoize from 'fast-memoize';
import {createSelector, createStructuredSelector} from 'reselect';
-import {CIRCLE_RADIUS, getMainLayout} from './layout';
+import {CIRCLE_RADIUS, drawSimpleLegend} from '../../d3-rendering/legend';
import {defineLineMarker, defineTextOnlyMarker, defineRectangleMarker} from '../../d3-rendering/markers';
+
+import {getMainLayout} from './layout';
import {currentVariableLineSegmentsSelector, HASH_ID, MASK_DESC} from './drawing-data';
-import config from '../../config';
import {getCurrentVariableMedianMetadata} from '../../selectors/median-statistics-selector';
-import {mediaQuery} from '../../utils';
import {link} from '../../lib/d3-redux';
const TS_LABEL = {
@@ -105,78 +105,6 @@ const createLegendMarkers = function(displayItems) {
return legendMarkers;
};
-/**
- * Create a simple legend
- *
- * @param {Object} div - d3 selector where legend should be created
- * @param {Object} legendMarkerRows - Array of rows. Each row should be an array of legend markers.
- * @param {Object} layout - width and height of svg.
- */
-export const drawSimpleLegend = function(div, {legendMarkerRows, layout}) {
- div.selectAll('.legend-svg').remove();
-
- if (!legendMarkerRows.length || !layout) {
- return;
- }
-
- const markerGroupXOffset = 15;
- const verticalRowOffset = 18;
-
- let svg = div.append('svg')
- .attr('class', 'legend-svg');
- let legend = svg
- .append('g')
- .attr('class', 'legend')
- .attr('transform', `translate(${mediaQuery(config.USWDS_MEDIUM_SCREEN) ? layout.margin.left : 0}, 0)`);
-
- legendMarkerRows.forEach((rowMarkers, rowIndex) => {
- let xPosition = 0;
- let yPosition = verticalRowOffset * (rowIndex + 1);
-
- rowMarkers.forEach((marker) => {
- let markerArgs = {
- x: xPosition,
- y: yPosition,
- text: marker.text,
- domId: marker.domId,
- domClass: marker.domClass,
- width: 20,
- height: 10,
- length: 20,
- r: marker.r,
- fill: marker.fill
- };
-
- let markerGroup = marker.type(legend, markerArgs);
- let markerGroupBBox;
- // Long story short, firefox is unable to get the bounding box if
- // the svg element isn't actually taking up space and visible. Folks on the
- // internet seem to have gotten around this by setting `visibility:hidden`
- // to hide things, but that would still mean the elements will take up space.
- // which we don't want. So, here's some error handling for getBBox failures.
- // This handling ends up not creating the legend, but that's okay because the
- // graph is being shown anyway. A more detailed discussion of this can be found at:
- // https://bugzilla.mozilla.org/show_bug.cgi?id=612118 and
- // https://stackoverflow.com/questions/28282295/getbbox-of-svg-when-hidden.
- try {
- markerGroupBBox = markerGroup.node().getBBox();
- xPosition = markerGroupBBox.x + markerGroupBBox.width + markerGroupXOffset;
-
- } catch(error) {
- // See above explanation
- }
- });
- });
-
- // Set the size of the containing svg node to the size of the legend.
- let bBox;
- try {
- bBox = legend.node().getBBox();
- } catch(error) {
- return;
- }
- svg.attr('viewBox', `-${CIRCLE_RADIUS} 0 ${layout.width} ${bBox.height + 10}`);
-};
const uniqueClassesSelector = memoize(tsKey => createSelector(
currentVariableLineSegmentsSelector(tsKey),
diff --git a/assets/src/scripts/components/hydrograph/legend.spec.js b/assets/src/scripts/components/hydrograph/legend.spec.js
index 7e97f2373..59988d89e 100644
--- a/assets/src/scripts/components/hydrograph/legend.spec.js
+++ b/assets/src/scripts/components/hydrograph/legend.spec.js
@@ -1,9 +1,10 @@
-import { select, selectAll } from 'd3-selection';
-import { drawSimpleLegend, legendMarkerRowsSelector, drawTimeSeriesLegend } from './legend';
-import { lineMarker, rectangleMarker, textOnlyMarker } from '../../d3-rendering/markers';
+import {select, selectAll} from 'd3-selection';
+import {legendMarkerRowsSelector, drawTimeSeriesLegend} from './legend';
+import {drawSimpleLegend} from '../../d3-rendering/legend';
+import {lineMarker, rectangleMarker, textOnlyMarker} from '../../d3-rendering/markers';
import {Actions, configureStore} from '../../store';
-describe('Legend module', () => {
+describe('UV: Legend module', () => {
const TEST_DATA = {
series: {
diff --git a/assets/src/scripts/d3-rendering/legend.js b/assets/src/scripts/d3-rendering/legend.js
new file mode 100644
index 000000000..dbac0f5f4
--- /dev/null
+++ b/assets/src/scripts/d3-rendering/legend.js
@@ -0,0 +1,77 @@
+import {mediaQuery} from "../utils";
+import config from "../config";
+
+export const CIRCLE_RADIUS = 4;
+
+/**
+ * Create a simple legend
+ *
+ * @param {Object} div - d3 selector where legend should be created
+ * @param {Object} legendMarkerRows - Array of rows. Each row should be an array of legend markers.
+ * @param {Object} layout - width and height of svg.
+ */
+export const drawSimpleLegend = function(div, {legendMarkerRows, layout}) {
+ div.selectAll('.legend-svg').remove();
+
+ if (!legendMarkerRows.length || !layout) {
+ return;
+ }
+
+ const markerGroupXOffset = 15;
+ const verticalRowOffset = 18;
+
+ let svg = div.append('svg')
+ .attr('class', 'legend-svg');
+ let legend = svg
+ .append('g')
+ .attr('class', 'legend')
+ .attr('transform', `translate(${mediaQuery(config.USWDS_MEDIUM_SCREEN) ? layout.margin.left : 0}, 0)`);
+
+ legendMarkerRows.forEach((rowMarkers, rowIndex) => {
+ let xPosition = 0;
+ let yPosition = verticalRowOffset * (rowIndex + 1);
+
+ rowMarkers.forEach((marker) => {
+ let markerArgs = {
+ x: xPosition,
+ y: yPosition,
+ text: marker.text,
+ domId: marker.domId,
+ domClass: marker.domClass,
+ width: 20,
+ height: 10,
+ length: 20,
+ r: marker.r,
+ fill: marker.fill
+ };
+
+ let markerGroup = marker.type(legend, markerArgs);
+ let markerGroupBBox;
+ // Long story short, firefox is unable to get the bounding box if
+ // the svg element isn't actually taking up space and visible. Folks on the
+ // internet seem to have gotten around this by setting `visibility:hidden`
+ // to hide things, but that would still mean the elements will take up space.
+ // which we don't want. So, here's some error handling for getBBox failures.
+ // This handling ends up not creating the legend, but that's okay because the
+ // graph is being shown anyway. A more detailed discussion of this can be found at:
+ // https://bugzilla.mozilla.org/show_bug.cgi?id=612118 and
+ // https://stackoverflow.com/questions/28282295/getbbox-of-svg-when-hidden.
+ try {
+ markerGroupBBox = markerGroup.node().getBBox();
+ xPosition = markerGroupBBox.x + markerGroupBBox.width + markerGroupXOffset;
+
+ } catch(error) {
+ // See above explanation
+ }
+ });
+ });
+
+ // Set the size of the containing svg node to the size of the legend.
+ let bBox;
+ try {
+ bBox = legend.node().getBBox();
+ } catch(error) {
+ return;
+ }
+ svg.attr('viewBox', `-${CIRCLE_RADIUS} 0 ${layout.width} ${bBox.height + 10}`);
+};
\ No newline at end of file
diff --git a/assets/src/scripts/d3-rendering/legend.spec.js b/assets/src/scripts/d3-rendering/legend.spec.js
new file mode 100644
index 000000000..33aea5c10
--- /dev/null
+++ b/assets/src/scripts/d3-rendering/legend.spec.js
@@ -0,0 +1,105 @@
+import {select, selectAll} from 'd3-selection';
+import {lineMarker, rectangleMarker, textOnlyMarker} from "./markers";
+import {drawSimpleLegend} from "./legend";
+
+describe('D3-rendering: Legend module', () => {
+
+ const TEST_STATE = {
+ observationsData: {
+ timeSeries: {
+ '12345': {
+ type: 'Feature',
+ id: '12345',
+ properties: {
+ phenomenonTimeStart: '2018-01-02',
+ phenomenonTimeEnd: '2018-01-05',
+ timeStep: ['2018-01-02', '2018-01-03', '2018-01-04', '2018-01-05'],
+ result: ['5.0', '4.0', '6.1', '3.2'],
+ approvals: [['Approved'], ['Approved'], ['Approved'], ['Estimated']],
+ nilReason: [null, 'AA', null, null],
+ qualifiers: [null, null, ['ICE'], ['ICE']],
+ grades: [['50'], ['50'], ['60'], ['60']]
+ }
+ }
+ }
+ },
+ observationsState: {
+ currentTimeSeriesId: '12345',
+ showSeries: {
+ current: true
+ }
+ },
+ ui: {
+ windowWidth: 1024,
+ width: 800
+ }
+ };
+
+ describe('drawSimpleLegend', () => {
+ let container;
+
+ const legendMarkerRows = [
+ [{
+ type: lineMarker,
+ length: 20,
+ domId: 'some-id',
+ domClass: 'some-class',
+ text: 'Some Text'
+ }, {
+ type: rectangleMarker,
+ domId: 'some-rectangle-id',
+ domClass: 'some-rectangle-class',
+ text: 'Rectangle Marker'
+ }],
+ [{
+ type: textOnlyMarker,
+ domId: 'text-id',
+ domClass: 'text-class',
+ text: 'Label'
+ }, {
+ type: lineMarker,
+ domId: null,
+ domClass: 'some-other-class',
+ text: 'Median Label'
+ }]
+ ];
+ const layout = {
+ width: 100,
+ height: 100,
+ margin: {
+ top: 0,
+ right: 0,
+ bottom: 0,
+ left: 0
+ }
+ };
+
+ beforeEach(() => {
+ container = select('body').append('div');
+ });
+
+ afterEach(() => {
+ container.remove();
+ });
+
+ it('Does not add a legend svg if no markers are provided', () => {
+ drawSimpleLegend(container, {
+ legendMarkerRows: [],
+ layout: layout
+ });
+
+ expect(container.select('svg').size()).toBe(0);
+ });
+
+ it('Adds a legend when width is provided', () => {
+ drawSimpleLegend(container, {legendMarkerRows, layout});
+
+ expect(container.select('svg').size()).toBe(1);
+ expect(container.selectAll('line').size()).toBe(2);
+ expect(container.selectAll('rect').size()).toBe(1);
+ expect(container.selectAll('text').size()).toBe(4);
+ });
+
+ });
+
+});
\ No newline at end of file
diff --git a/assets/src/scripts/index.spec.js b/assets/src/scripts/index.spec.js
index f65b8fa79..220329bea 100644
--- a/assets/src/scripts/index.spec.js
+++ b/assets/src/scripts/index.spec.js
@@ -18,6 +18,7 @@ import './d3-rendering/cursor-slider.spec';
import './d3-rendering/loading-indicator.spec';
import './d3-rendering/graph-tooltip.spec';
import './d3-rendering/markers.spec';
+import './d3-rendering/legend.spec';
import './components/dailyValueHydrograph/selectors/labels.spec';
import './components/dailyValueHydrograph/selectors/scales.spec';
--
GitLab