From a3797e2ef1cb39f7c7663fb64884350601af7239 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Thu, 28 Dec 2017 14:18:45 -0500 Subject: [PATCH 1/5] Cleanup --- src/components/fields/Field.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/components/fields/Field.js b/src/components/fields/Field.js index 8283139a0..ddf9690b0 100644 --- a/src/components/fields/Field.js +++ b/src/components/fields/Field.js @@ -6,18 +6,6 @@ import {bem, localize} from 'lib'; import {getMultiValueText} from 'lib/constants'; class Field extends Component { - renderPostfix() { - const {units} = this.props; - if (!units) { - return null; - } - return ( -
-
{units}
-
- ); - } - render() { const { center, From b6025670123fdb76db4eaa8906c76b6e27b1810f Mon Sep 17 00:00:00 2001 From: VeraZab Date: Thu, 28 Dec 2017 14:27:18 -0500 Subject: [PATCH 2/5] Create separate functions for plot type recognition + adjust line type --- src/components/fields/TraceSelector.js | 29 ++++------------------- src/lib/customTraceType.js | 32 ++++++++++++++++++++++++++ src/lib/index.js | 5 +++- 3 files changed, 40 insertions(+), 26 deletions(-) create mode 100644 src/lib/customTraceType.js diff --git a/src/components/fields/TraceSelector.js b/src/components/fields/TraceSelector.js index 02336222e..f002b4e9a 100644 --- a/src/components/fields/TraceSelector.js +++ b/src/components/fields/TraceSelector.js @@ -1,8 +1,7 @@ import {UnconnectedDropdown} from './Dropdown'; import PropTypes from 'prop-types'; import React, {Component} from 'react'; -import nestedProperty from 'plotly.js/src/lib/nested_property'; -import {connectToContainer} from '../../lib'; +import {connectToContainer, customToPlotly, plotlyToCustom} from '../../lib'; function computeTraceOptionsFromSchema(schema) { const capitalize = s => s.charAt(0).toUpperCase() + s.substring(1); @@ -62,18 +61,7 @@ class TraceSelector extends Component { this.traceOptions = [{label: 'Scatter', value: 'scatter'}]; } - // If we used fullData mode or fill it may be undefined if the fullTrace - // is not visible and therefore does not have these values computed. - const mode = nestedProperty(props.container, 'mode').get(); - const fill = nestedProperty(props.container, 'fill').get(); - const fullValue = props.fullValue; - if (fullValue === 'scatter' && this.fillTypes.includes(fill)) { - this.fullValue = 'area'; - } else if (fullValue === 'scatter' && mode === 'lines') { - this.fullValue = 'line'; - } else { - this.fullValue = fullValue; - } + this.fullValue = plotlyToCustom(props.fullContainer); } componentWillReceiveProps(nextProps, nextContext) { @@ -81,16 +69,7 @@ class TraceSelector extends Component { } updatePlot(value) { - let update; - if (value === 'line') { - update = {type: 'scatter', mode: 'lines', fill: 'none'}; - } else if (value === 'scatter') { - update = {type: 'scatter', mode: 'markers', fill: 'none'}; - } else if (value === 'area') { - update = {type: 'scatter', fill: 'tozeroy'}; - } else { - update = {type: value}; - } + const update = customToPlotly(value); if (this.props.updateContainer) { this.props.updateContainer(update); @@ -114,7 +93,7 @@ TraceSelector.contextTypes = { TraceSelector.propTypes = { getValObject: PropTypes.func, - container: PropTypes.object.isRequired, + fullContainer: PropTypes.object.isRequired, fullValue: PropTypes.any.isRequired, updateContainer: PropTypes.func, }; diff --git a/src/lib/customTraceType.js b/src/lib/customTraceType.js new file mode 100644 index 000000000..a43464500 --- /dev/null +++ b/src/lib/customTraceType.js @@ -0,0 +1,32 @@ +export function plotlyToCustom(trace) { + if ( + trace.type === 'scatter' && + ['tozeroy', 'tozerox', 'tonexty', 'tonextx', 'toself', 'tonext'].includes( + trace.fill + ) + ) { + return 'area'; + } else if ( + trace.type === 'scatter' && + (trace.mode === 'lines' || trace.mode === 'lines+markers') + ) { + return 'line'; + } + return trace.type; +} + +export function customToPlotly(customTraceType) { + if (customTraceType === 'line') { + return {type: 'scatter', mode: 'lines', fill: 'none'}; + } + + if (customTraceType === 'scatter') { + return {type: 'scatter', mode: 'markers', fill: 'none'}; + } + + if (customTraceType === 'area') { + return {type: 'scatter', fill: 'tozeroy'}; + } + + return {type: customTraceType}; +} diff --git a/src/lib/index.js b/src/lib/index.js index 438c93e1d..5679e6735 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -10,6 +10,7 @@ import localize, {localizeString} from './localize'; import tinyColor from 'tinycolor2'; import unpackPlotProps from './unpackPlotProps'; import walkObject, {isPlainObject} from './walkObject'; +import {customToPlotly, plotlyToCustom} from './customTraceType'; function clamp(value, min, max) { return Math.max(min, Math.min(max, value)); @@ -33,14 +34,16 @@ export { connectLayoutToPlot, connectToContainer, connectTraceToPlot, + customToPlotly, dereference, + findFullTraceIndex, getDisplayName, getLayoutContext, - findFullTraceIndex, icon, isPlainObject, localize, localizeString, + plotlyToCustom, unpackPlotProps, walkObject, }; From eada18f0103c540a6388e1044a8a7ca6372d4a48 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Thu, 28 Dec 2017 14:27:24 -0500 Subject: [PATCH 3/5] Adjust starting plots --- examples/simple/src/App.js | 2 +- src/PlotlyEditor.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/simple/src/App.js b/examples/simple/src/App.js index 11d344ad2..3b1f05325 100644 --- a/examples/simple/src/App.js +++ b/examples/simple/src/App.js @@ -24,7 +24,7 @@ class App extends Component { // overwritten with a full DOM node that contains data, layout, _fullData, // _fullLayout etc in handlePlotUpdate() const graphDiv = { - data: [{type: 'scatter', xsrc: 'col1', ysrc: 'col2'}], + data: [{type: 'scatter', xsrc: 'col1', ysrc: 'col2', mode: 'markers'}], layout: {title: 'Room readings'}, }; diff --git a/src/PlotlyEditor.js b/src/PlotlyEditor.js index 080072870..88f676170 100644 --- a/src/PlotlyEditor.js +++ b/src/PlotlyEditor.js @@ -93,7 +93,7 @@ class PlotlyEditor extends Component { if (this.props.onAddTrace) { this.props.onAddTrace(payload); } - graphDiv.data.push({x: [], y: []}); + graphDiv.data.push({x: [], y: [], type: 'scatter', mode: 'markers'}); if (this.props.onUpdate) { this.props.onUpdate(); } From 13691c46f38b1fdc7ba3d37430b32e20f095b023 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Thu, 28 Dec 2017 15:37:05 -0500 Subject: [PATCH 4/5] Review comments --- src/components/fields/TraceSelector.js | 6 +++--- .../fields/__tests__/TraceSelector-test.js | 18 ++++++++++++++++++ src/lib/customTraceType.js | 4 ++-- src/lib/index.js | 6 +++--- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/components/fields/TraceSelector.js b/src/components/fields/TraceSelector.js index f002b4e9a..cc8d7db30 100644 --- a/src/components/fields/TraceSelector.js +++ b/src/components/fields/TraceSelector.js @@ -1,7 +1,7 @@ import {UnconnectedDropdown} from './Dropdown'; import PropTypes from 'prop-types'; import React, {Component} from 'react'; -import {connectToContainer, customToPlotly, plotlyToCustom} from '../../lib'; +import {connectToContainer, customTraceToPlotlyTrace, plotlyTraceToCustomTrace} from '../../lib'; function computeTraceOptionsFromSchema(schema) { const capitalize = s => s.charAt(0).toUpperCase() + s.substring(1); @@ -61,7 +61,7 @@ class TraceSelector extends Component { this.traceOptions = [{label: 'Scatter', value: 'scatter'}]; } - this.fullValue = plotlyToCustom(props.fullContainer); + this.fullValue = plotlyTraceToCustomTrace(props.fullContainer); } componentWillReceiveProps(nextProps, nextContext) { @@ -69,7 +69,7 @@ class TraceSelector extends Component { } updatePlot(value) { - const update = customToPlotly(value); + const update = customTraceToPlotlyTrace(value); if (this.props.updateContainer) { this.props.updateContainer(update); diff --git a/src/components/fields/__tests__/TraceSelector-test.js b/src/components/fields/__tests__/TraceSelector-test.js index f9db1ca1a..f6fdc50eb 100644 --- a/src/components/fields/__tests__/TraceSelector-test.js +++ b/src/components/fields/__tests__/TraceSelector-test.js @@ -45,6 +45,24 @@ describe('TraceSelector', () => { expect(innerDropdown.prop('value')).toEqual('line'); }); + it('interprets scatter + mode=lines+markers as type=line', () => { + const editorProps = { + ...fixtures.scatter({data: [{mode: 'lines+markers'}]}), + onUpdate: jest.fn(), + }; + const wrapper = mount( + + + + + + ).find(TraceSelector); + + const innerDropdown = wrapper.find(Dropdown); + + expect(innerDropdown.prop('value')).toEqual('line'); + }); + it('updates type=scatter mode=lines when type=line', () => { const onUpdateTraces = jest.fn(); const editorProps = { diff --git a/src/lib/customTraceType.js b/src/lib/customTraceType.js index a43464500..58b5115cf 100644 --- a/src/lib/customTraceType.js +++ b/src/lib/customTraceType.js @@ -1,4 +1,4 @@ -export function plotlyToCustom(trace) { +export function plotlyTraceToCustomTrace(trace) { if ( trace.type === 'scatter' && ['tozeroy', 'tozerox', 'tonexty', 'tonextx', 'toself', 'tonext'].includes( @@ -15,7 +15,7 @@ export function plotlyToCustom(trace) { return trace.type; } -export function customToPlotly(customTraceType) { +export function customTraceToPlotlyTrace(customTraceType) { if (customTraceType === 'line') { return {type: 'scatter', mode: 'lines', fill: 'none'}; } diff --git a/src/lib/index.js b/src/lib/index.js index 5679e6735..28a023ec2 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -10,7 +10,7 @@ import localize, {localizeString} from './localize'; import tinyColor from 'tinycolor2'; import unpackPlotProps from './unpackPlotProps'; import walkObject, {isPlainObject} from './walkObject'; -import {customToPlotly, plotlyToCustom} from './customTraceType'; +import {customTraceToPlotlyTrace, plotlyTraceToCustomTrace} from './customTraceType'; function clamp(value, min, max) { return Math.max(min, Math.min(max, value)); @@ -34,7 +34,7 @@ export { connectLayoutToPlot, connectToContainer, connectTraceToPlot, - customToPlotly, + customTraceToPlotlyTrace, dereference, findFullTraceIndex, getDisplayName, @@ -43,7 +43,7 @@ export { isPlainObject, localize, localizeString, - plotlyToCustom, + plotlyTraceToCustomTrace, unpackPlotProps, walkObject, }; From 8f85a6cbe1da929e127f3b8591d649e3d9f80e19 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Thu, 28 Dec 2017 15:55:23 -0500 Subject: [PATCH 5/5] Styling adjustments --- src/components/fields/TraceSelector.js | 6 +++++- src/lib/index.js | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/components/fields/TraceSelector.js b/src/components/fields/TraceSelector.js index cc8d7db30..d5d3fda59 100644 --- a/src/components/fields/TraceSelector.js +++ b/src/components/fields/TraceSelector.js @@ -1,7 +1,11 @@ import {UnconnectedDropdown} from './Dropdown'; import PropTypes from 'prop-types'; import React, {Component} from 'react'; -import {connectToContainer, customTraceToPlotlyTrace, plotlyTraceToCustomTrace} from '../../lib'; +import { + connectToContainer, + customTraceToPlotlyTrace, + plotlyTraceToCustomTrace, +} from '../../lib'; function computeTraceOptionsFromSchema(schema) { const capitalize = s => s.charAt(0).toUpperCase() + s.substring(1); diff --git a/src/lib/index.js b/src/lib/index.js index 28a023ec2..5fcc5513a 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -10,7 +10,10 @@ import localize, {localizeString} from './localize'; import tinyColor from 'tinycolor2'; import unpackPlotProps from './unpackPlotProps'; import walkObject, {isPlainObject} from './walkObject'; -import {customTraceToPlotlyTrace, plotlyTraceToCustomTrace} from './customTraceType'; +import { + customTraceToPlotlyTrace, + plotlyTraceToCustomTrace, +} from './customTraceType'; function clamp(value, min, max) { return Math.max(min, Math.min(max, value));