From e877e796ac9591cbf7783c2c2cddb227c1543725 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Thu, 4 Jan 2018 11:50:56 -0500 Subject: [PATCH 1/5] Fix trace Delete typo --- src/PlotlyEditor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PlotlyEditor.js b/src/PlotlyEditor.js index 7e553be97..5e7868711 100644 --- a/src/PlotlyEditor.js +++ b/src/PlotlyEditor.js @@ -106,7 +106,7 @@ class PlotlyEditor extends Component { this.props.onDeleteTrace(payload); } if (payload.traceIndexes && payload.traceIndexes.length) { - graphDiv.data.splice(payload[0], 1); + graphDiv.data.splice(payload.traceIndexes[0], 1); if (this.props.onUpdate) { this.props.onUpdate(); } From 9621b2332c4da8184e725b25df3fe9beacc5fb8e Mon Sep 17 00:00:00 2001 From: VeraZab Date: Thu, 4 Jan 2018 12:57:19 -0500 Subject: [PATCH 2/5] Force mode when !trace.mode, but _fullTrace.mode is present --- src/components/fields/TraceSelector.js | 24 +++++++++++++++++++++--- src/lib/customTraceType.js | 2 ++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/components/fields/TraceSelector.js b/src/components/fields/TraceSelector.js index 6538c45a8..749e0a507 100644 --- a/src/components/fields/TraceSelector.js +++ b/src/components/fields/TraceSelector.js @@ -52,7 +52,11 @@ class TraceSelector extends Component { 'tonext', ]; } - + this.setTraceDefaults( + props.container, + props.fullContainer, + props.updateContainer + ); this.setLocals(props, context); } @@ -64,17 +68,30 @@ class TraceSelector extends Component { } else { this.traceOptions = [{label: 'Scatter', value: 'scatter'}]; } + this.fullValue = plotlyTraceToCustomTrace(props.container); + } - this.fullValue = plotlyTraceToCustomTrace(props.fullContainer); + setTraceDefaults(container, fullContainer, updateContainer) { + if ( + (container.type === 'scatter' && !container.mode) || + (!container.type && fullContainer.type === 'scatter') + ) { + updateContainer({ + type: 'scatter', + mode: fullContainer.mode || 'markers', + fill: fullContainer.fill || container.fill, + }); + } } componentWillReceiveProps(nextProps, nextContext) { + const {container, fullContainer, updateContainer} = nextProps; + this.setTraceDefaults(container, fullContainer, updateContainer); this.setLocals(nextProps, nextContext); } updatePlot(value) { const update = customTraceToPlotlyTrace(value); - if (this.props.updateContainer) { this.props.updateContainer(update); } @@ -97,6 +114,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 index 58b5115cf..514cf235a 100644 --- a/src/lib/customTraceType.js +++ b/src/lib/customTraceType.js @@ -1,6 +1,7 @@ export function plotlyTraceToCustomTrace(trace) { if ( trace.type === 'scatter' && + trace.fill && ['tozeroy', 'tozerox', 'tonexty', 'tonextx', 'toself', 'tonext'].includes( trace.fill ) @@ -8,6 +9,7 @@ export function plotlyTraceToCustomTrace(trace) { return 'area'; } else if ( trace.type === 'scatter' && + trace.mode && (trace.mode === 'lines' || trace.mode === 'lines+markers') ) { return 'line'; From 89943e8c7820b757947de37038234dcad1060b14 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Thu, 4 Jan 2018 13:05:31 -0500 Subject: [PATCH 3/5] Add/adjust tests --- .../fields/__tests__/DataSelector-test.js | 1 + .../fields/__tests__/TraceSelector-test.js | 61 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/components/fields/__tests__/DataSelector-test.js b/src/components/fields/__tests__/DataSelector-test.js index 7a7f6d153..1ffae96dc 100644 --- a/src/components/fields/__tests__/DataSelector-test.js +++ b/src/components/fields/__tests__/DataSelector-test.js @@ -37,6 +37,7 @@ describe('DataSelector', () => { it('calls updatePlot with srcAttr and data when present', () => { const onUpdateTraces = jest.fn(); const wrapper = render({onUpdateTraces}).find(DropdownWidget); + onUpdateTraces.mockClear(); wrapper.prop('onChange')('y1'); expect(onUpdateTraces.mock.calls[0][0]).toEqual({ update: {xsrc: 'y1', x: [2, 3, 4]}, diff --git a/src/components/fields/__tests__/TraceSelector-test.js b/src/components/fields/__tests__/TraceSelector-test.js index f7ea24edc..d9a189ee9 100644 --- a/src/components/fields/__tests__/TraceSelector-test.js +++ b/src/components/fields/__tests__/TraceSelector-test.js @@ -9,6 +9,67 @@ import {connectTraceToPlot} from 'lib'; describe('TraceSelector', () => { const TraceSection = connectTraceToPlot(Section); + it('sets mode to markers if trace scatter, no data or mode provided', () => { + const editorProps = { + ...fixtures.scatter({data: [{mode: null, xsrc: null, ysrc: null}]}), + onUpdate: jest.fn(), + }; + const wrapper = mount( + + + + + + ).find(TraceSelector); + + const innerDropdown = wrapper.find(Dropdown); + + expect(wrapper.props().plotProps.container.mode).toBe('markers'); + expect(innerDropdown.prop('value')).toEqual('scatter'); + }); + + it('if no data provided, but mode is provided, displays correct trace type', () => { + const editorProps = { + ...fixtures.scatter({ + data: [{mode: 'lines+markers', xsrc: null, ysrc: null}], + }), + onUpdate: jest.fn(), + }; + const wrapper = mount( + + + + + + ).find(TraceSelector); + + const innerDropdown = wrapper.find(Dropdown); + + expect(innerDropdown.prop('value')).toEqual('line'); + }); + + it('if data provided, but no mode is provided, chooses mode according to fullData', () => { + const editorProps = { + ...fixtures.scatter(), + onUpdate: jest.fn(), + }; + + expect(!editorProps.graphDiv.data[0].mode).toBe(true); + expect(editorProps.graphDiv._fullData[0].mode).toBe('lines+markers'); + + const wrapper = mount( + + + + + + ).find(TraceSelector); + + const innerDropdown = wrapper.find(Dropdown); + expect(wrapper.props().plotProps.container.mode).toBe('lines+markers'); + expect(innerDropdown.prop('value')).toEqual('line'); + }); + it('interprets scatter + fill as type=area', () => { const editorProps = { ...fixtures.scatter({data: [{fill: 'tonexty'}]}), From b20743031836c2d52308b422ae7685173df2ef47 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Thu, 4 Jan 2018 13:58:47 -0500 Subject: [PATCH 4/5] Review comments --- src/components/fields/TraceSelector.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/components/fields/TraceSelector.js b/src/components/fields/TraceSelector.js index 749e0a507..353cdd581 100644 --- a/src/components/fields/TraceSelector.js +++ b/src/components/fields/TraceSelector.js @@ -72,14 +72,10 @@ class TraceSelector extends Component { } setTraceDefaults(container, fullContainer, updateContainer) { - if ( - (container.type === 'scatter' && !container.mode) || - (!container.type && fullContainer.type === 'scatter') - ) { + if (!container.mode && fullContainer.type === 'scatter') { updateContainer({ type: 'scatter', mode: fullContainer.mode || 'markers', - fill: fullContainer.fill || container.fill, }); } } From 9398c337fb5b28cb340b542f73175655c7b221ed Mon Sep 17 00:00:00 2001 From: VeraZab Date: Thu, 4 Jan 2018 14:10:33 -0500 Subject: [PATCH 5/5] Review 2 --- src/lib/customTraceType.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib/customTraceType.js b/src/lib/customTraceType.js index 514cf235a..58b5115cf 100644 --- a/src/lib/customTraceType.js +++ b/src/lib/customTraceType.js @@ -1,7 +1,6 @@ export function plotlyTraceToCustomTrace(trace) { if ( trace.type === 'scatter' && - trace.fill && ['tozeroy', 'tozerox', 'tonexty', 'tonextx', 'toself', 'tonext'].includes( trace.fill ) @@ -9,7 +8,6 @@ export function plotlyTraceToCustomTrace(trace) { return 'area'; } else if ( trace.type === 'scatter' && - trace.mode && (trace.mode === 'lines' || trace.mode === 'lines+markers') ) { return 'line';