From d4b0152ae5f233beb2ee0ba2d0d41eefa5a49d50 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Tue, 7 Aug 2018 15:51:03 -0400 Subject: [PATCH 1/9] Allow styling of split traces --- src/EditorControls.js | 22 +- src/components/containers/TraceAccordion.js | 201 +++++++++++++----- .../containers/TransformAccordion.js | 4 + src/components/fields/DataSelector.js | 10 + src/components/fields/MultiColorPicker.js | 52 +++-- src/default_panels/StyleTracesPanel.js | 2 +- src/lib/connectTraceToPlot.js | 50 ++++- src/lib/multiValues.js | 2 +- src/shame.js | 52 +++++ 9 files changed, 305 insertions(+), 90 deletions(-) diff --git a/src/EditorControls.js b/src/EditorControls.js index 4a6e47efc..53f4fad98 100644 --- a/src/EditorControls.js +++ b/src/EditorControls.js @@ -7,6 +7,7 @@ import { shamefullyAdjustAxisRef, shamefullyAdjustGeo, shamefullyAddTableColumns, + shamefullyCreateSplitStyles, } from './shame'; import {EDITOR_ACTIONS} from './lib/constants'; import isNumeric from 'fast-isnumeric'; @@ -70,12 +71,27 @@ class EditorControls extends Component { for (let i = 0; i < payload.traceIndexes.length; i++) { for (const attr in payload.update) { const traceIndex = payload.traceIndexes[i]; - const prop = nestedProperty(graphDiv.data[traceIndex], attr); + const splitTraceGroup = payload.splitTraceGroup + ? payload.splitTraceGroup.toString() + : null; + + let props = [nestedProperty(graphDiv.data[traceIndex], attr)]; const value = payload.update[attr]; - if (value !== void 0) { - prop.set(value); + if (splitTraceGroup) { + props = shamefullyCreateSplitStyles( + graphDiv, + attr, + traceIndex, + splitTraceGroup + ); } + + props.forEach(p => { + if (value !== void 0) { + p.set(value); + } + }); } } diff --git a/src/components/containers/TraceAccordion.js b/src/components/containers/TraceAccordion.js index 9b7f863fe..ede911949 100644 --- a/src/components/containers/TraceAccordion.js +++ b/src/components/containers/TraceAccordion.js @@ -10,78 +10,155 @@ import {Tab, Tabs, TabList, TabPanel} from 'react-tabs'; const TraceFold = connectTraceToPlot(PlotlyFold); class TraceAccordion extends Component { - render() { - const {data = [], localize: _} = this.context; - const { - canAdd, - canGroup, - children, - messageIfEmptyFold, - excludeFits, - } = this.props; + renderGroupedTraceFolds(groupedTraces) { + if (this.props.useFullData) { + const dataArrayPositionsByTraceType = {}; + const fullDataArrayPositionsByTraceType = {}; - // we don't want to include analysis transforms when we're in the create panel - const filteredData = data.filter(t => { - if (excludeFits) { - return !(t.transforms && t.transforms.every(tr => tr.type === 'fit')); - } - return true; - }); + Object.keys(groupedTraces).forEach(traceType => { + const dataIndexes = []; + const fullDataIndexes = []; + + this.context.fullData.forEach((trace, fullDataIndex) => { + if (trace.type === traceType && trace._expandedIndex) { + fullDataIndexes.push(trace._expandedIndex); + } + if (trace.type === traceType && !trace._expandedIndex) { + fullDataIndexes.push(fullDataIndex); + } + if (trace.type === traceType) { + dataIndexes.push(trace.index); + } + }); + + dataArrayPositionsByTraceType[traceType] = dataIndexes; + fullDataArrayPositionsByTraceType[traceType] = fullDataIndexes; + }); - const individualTraces = - filteredData.length && - filteredData.map((d, i) => { + return Object.keys(groupedTraces).map((traceType, index) => { return ( - {children} + {this.props.children} ); }); + } - if (canAdd) { - const addAction = { - label: _('Trace'), - handler: ({onUpdate}) => { - if (onUpdate) { - onUpdate({ - type: EDITOR_ACTIONS.ADD_TRACE, - }); - } - }, - }; + return Object.keys(groupedTraces).map((traceType, index) => { return ( - - {individualTraces ? individualTraces : null} - + + {this.props.children} + ); + }); + } + + renderIndividualTraceFolds(filteredTraces, filteredTracesFullDataPositions) { + if (this.props.useFullData) { + return filteredTraces.map((d, i) => { + return ( + + {this.props.children} + + ); + }); } - const tracesByGroup = filteredData.reduce((allTraces, nextTrace, index) => { - const traceType = plotlyTraceToCustomTrace(nextTrace); - if (!allTraces[traceType]) { - allTraces[traceType] = []; - } - allTraces[traceType].push(index); - return allTraces; - }, {}); - const groupedTraces = Object.keys(tracesByGroup).map((traceType, index) => { + return filteredTraces.map((d, i) => { return ( {this.props.children} ); }); + } + + renderCanAddPanel(filteredTraces, filteredTracesFullDataPositions) { + const _ = this.context.localize; + + const addAction = { + label: _('Trace'), + handler: ({onUpdate}) => { + if (onUpdate) { + onUpdate({ + type: EDITOR_ACTIONS.ADD_TRACE, + }); + } + }, + }; + + return ( + + {filteredTraces.length + ? this.renderIndividualTraceFolds( + filteredTraces, + filteredTracesFullDataPositions + ) + : null} + + ); + } + + render() { + const {data = [], localize: _, fullData = []} = this.context; + const {canAdd, canGroup, excludeFits, useFullData} = this.props; + const base = useFullData ? fullData : data; + + // we don't want to include analysis transforms when we're in the create panel + const filteredTracesFullDataPositions = []; + const filteredTraces = base.filter((t, i) => { + if (excludeFits) { + return !(t.transforms && t.transforms.every(tr => tr.type === 'fit')); + } + filteredTracesFullDataPositions.push(i); + return true; + }); + + const groupedTraces = filteredTraces.reduce( + (allTraces, nextTrace, index) => { + const traceType = plotlyTraceToCustomTrace(nextTrace); + const adjustedIndex = useFullData ? nextTrace.index : index; + if (!allTraces[traceType]) { + allTraces[traceType] = []; + } + allTraces[traceType].push(adjustedIndex); + return allTraces; + }, + {} + ); - if (canGroup && filteredData.length > 1 && groupedTraces.length > 0) { + if (canAdd) { + return this.renderCanAddPanel( + filteredTraces, + filteredTracesFullDataPositions + ); + } + + if ( + canGroup && + filteredTraces.length > 1 && + Object.keys(groupedTraces).length > 0 + ) { return ( @@ -91,19 +168,34 @@ class TraceAccordion extends Component { - {individualTraces ? individualTraces : null} + {filteredTraces.length + ? this.renderIndividualTraceFolds( + filteredTraces, + filteredTracesFullDataPositions + ) + : null} - {groupedTraces ? groupedTraces : null} + + {Object.keys(groupedTraces).length + ? this.renderGroupedTraceFolds(groupedTraces) + : null} + ); } + return ( - {individualTraces ? individualTraces : null} + {filteredTraces.length + ? this.renderIndividualTraceFolds( + filteredTraces, + filteredTracesFullDataPositions + ) + : null} ); } @@ -121,6 +213,7 @@ TraceAccordion.propTypes = { children: PropTypes.node, excludeFits: PropTypes.bool, messageIfEmptyFold: PropTypes.string, + useFullData: PropTypes.bool, }; export default TraceAccordion; diff --git a/src/components/containers/TransformAccordion.js b/src/components/containers/TransformAccordion.js index f1129c120..a37cdc01a 100644 --- a/src/components/containers/TransformAccordion.js +++ b/src/components/containers/TransformAccordion.js @@ -104,6 +104,10 @@ class TransformAccordion extends Component { payload.groups = null; } + if (type === 'groupby') { + payload.styles = []; + } + updateContainer({[key]: payload}); } }, diff --git a/src/components/fields/DataSelector.js b/src/components/fields/DataSelector.js index 57a8e551e..85da82ece 100644 --- a/src/components/fields/DataSelector.js +++ b/src/components/fields/DataSelector.js @@ -83,6 +83,16 @@ export class UnconnectedDataSelector extends Component { : null, } ); + + if ( + this.props.container.type && + this.props.container.type === 'groupby' && + data + ) { + const styles = data.map(groupEl => ({target: groupEl, value: {}})); + update.styles = styles; + } + this.props.updateContainer(update); } diff --git a/src/components/fields/MultiColorPicker.js b/src/components/fields/MultiColorPicker.js index 9d60f0899..9fb1aee0c 100644 --- a/src/components/fields/MultiColorPicker.js +++ b/src/components/fields/MultiColorPicker.js @@ -1,10 +1,11 @@ -import React, {Component} from 'react'; import Color from './Color'; import Colorscale from './Colorscale'; -import Info from './Info'; import Field from './Field'; -import RadioBlocks from '../widgets/RadioBlocks'; +import Info from './Info'; import PropTypes from 'prop-types'; +import RadioBlocks from '../widgets/RadioBlocks'; +import React, {Component} from 'react'; +import nestedProperty from 'plotly.js/src/lib/nested_property'; import {adjustColorscale, connectToContainer} from 'lib'; class UnconnectedMultiColorPicker extends Component { @@ -26,7 +27,7 @@ class UnconnectedMultiColorPicker extends Component { } setColors(colorscale, colorscaleType) { - const numberOfTraces = this.context.traceIndexes.length; + const numberOfTraces = this.props.tracesToColor.length; const colors = colorscale.map(c => c[1]); let adjustedColors = colors; @@ -133,6 +134,7 @@ UnconnectedMultiColorPicker.propTypes = { onConstantColorOptionChange: PropTypes.func, messageKeyWordSingle: PropTypes.string, messageKeyWordPlural: PropTypes.string, + tracesToColor: PropTypes.array, ...Field.propTypes, }; @@ -146,22 +148,32 @@ UnconnectedMultiColorPicker.contextTypes = { export default connectToContainer(UnconnectedMultiColorPicker, { modifyPlotProps(props, context, plotProps) { if (plotProps.isVisible) { - plotProps.fullValue = context.traceIndexes - .map(index => { - const trace = context.fullData.filter( - trace => trace.index === index - )[0]; - - const properties = props.attr.split('.'); - let value = trace; - - properties.forEach(prop => { - value = value[prop]; - }); - - return value; - }) - .map(c => [0, c]); + const colors = []; + let tracesToColor = []; + const dedupedTraceIndexes = []; + + context.traceIndexes.forEach(i => { + if (!dedupedTraceIndexes.includes(i)) { + dedupedTraceIndexes.push(i); + } + }); + + dedupedTraceIndexes.forEach(traceIndex => { + const traces = context.fullData.filter( + trace => trace.index === traceIndex + ); + tracesToColor = tracesToColor.concat(traces); + + traces.forEach(t => { + const value = nestedProperty(t, props.attr).get(); + if (value) { + colors.push(value); + } + }); + }); + + plotProps.tracesToColor = tracesToColor; + plotProps.fullValue = colors.map(c => [0, c]); } }, }); diff --git a/src/default_panels/StyleTracesPanel.js b/src/default_panels/StyleTracesPanel.js index f91fc5497..cbf088363 100644 --- a/src/default_panels/StyleTracesPanel.js +++ b/src/default_panels/StyleTracesPanel.js @@ -39,7 +39,7 @@ import { } from '../components/fields/derived'; const StyleTracesPanel = (props, {localize: _}) => ( - + 0 ? data[traceIndexes[0]] : {}; - + getFullTraceFromDataIndex(trace, context) { let fullTrace = {}; - for (let i = 0; i < fullData.length; i++) { - if (traceIndexes[0] === fullData[i]._fullInput.index) { + + for (let i = 0; i < context.fullData.length; i++) { + if ( + this.props.traceIndexes[0] === context.fullData[i]._fullInput.index + ) { /* * Fit transforms are custom transforms in our custom plotly.js bundle, * they are different from others as they create an extra trace in the @@ -52,15 +50,26 @@ export default function connectTraceToPlot(WrappedComponent) { trace.transforms && trace.transforms.every(t => t.type === 'fit') ) { - fullData[i]._fullInput = fullData[i]; + context.fullData[i]._fullInput = context.fullData[i]; } - fullTrace = fullData[i]._fullInput; - + fullTrace = context.fullData[i]._fullInput; break; } } + return fullTrace; + } + + setLocals(props, context) { + const {traceIndexes, fullDataArrayPosition} = props; + const {data, fullData, plotly} = context; + + const trace = data[traceIndexes[0]]; + const fullTrace = fullDataArrayPosition + ? fullData[fullDataArrayPosition[0]] + : this.getFullTraceFromDataIndex(trace, context); + this.childContext = { getValObject: attr => !plotly @@ -110,6 +119,12 @@ export default function connectTraceToPlot(WrappedComponent) { updateTrace(update) { if (this.context.onUpdate) { + const splitTraceGroup = this.props.fullDataArrayPosition + ? this.props.fullDataArrayPosition.map( + p => this.context.fullData[p]._group + ) + : null; + if (Array.isArray(update)) { update.forEach((u, i) => { this.context.onUpdate({ @@ -117,6 +132,18 @@ export default function connectTraceToPlot(WrappedComponent) { payload: { update: u, traceIndexes: [this.props.traceIndexes[i]], + splitTraceGroup: splitTraceGroup ? splitTraceGroup[i] : null, + }, + }); + }); + } else if (splitTraceGroup) { + this.props.traceIndexes.forEach((t, i) => { + this.context.onUpdate({ + type: EDITOR_ACTIONS.UPDATE_TRACES, + payload: { + update, + traceIndexes: [this.props.traceIndexes[i]], + splitTraceGroup: splitTraceGroup ? splitTraceGroup[i] : null, }, }); }); @@ -205,6 +232,7 @@ export default function connectTraceToPlot(WrappedComponent) { TraceConnectedComponent.propTypes = { traceIndexes: PropTypes.arrayOf(PropTypes.number).isRequired, + fullDataArrayPosition: PropTypes.arrayOf(PropTypes.number), }; TraceConnectedComponent.contextTypes = { diff --git a/src/lib/multiValues.js b/src/lib/multiValues.js index 2f702d9c4..ab0be3898 100644 --- a/src/lib/multiValues.js +++ b/src/lib/multiValues.js @@ -22,7 +22,7 @@ function setMultiValuedContainer(intoObj, fromObj, key, config = {}) { // don't merge private attrs if ( - (typeof key === 'string' && key.charAt(0) === '_') || + (typeof key === 'string' && key.charAt(0) === '_' && key !== '_group') || typeof intoVal === 'function' || key === 'module' ) { diff --git a/src/shame.js b/src/shame.js index 1c1c0e397..8a527b945 100644 --- a/src/shame.js +++ b/src/shame.js @@ -112,3 +112,55 @@ export const shamefullyAddTableColumns = (graphDiv, {traceIndexes, update}) => { update['header.values'] = null; } }; + +export const shamefullyCreateSplitStyles = ( + graphDiv, + attr, + traceIndex, + splitTraceGroup +) => { + if (!Array.isArray(splitTraceGroup)) { + splitTraceGroup = [splitTraceGroup]; // eslint-disable-line + } + + let indexOfSplitTransform = null; + + graphDiv.data[traceIndex].transforms.filter((t, i) => { + if (t.type === 'groupby') { + indexOfSplitTransform = i; + } + }); + + function getProp(group) { + let indexOfStyleObject = null; + + graphDiv.data[traceIndex].transforms[indexOfSplitTransform].styles.filter( + (s, i) => { + if (s.target.toString() === group) { + indexOfStyleObject = i; + } + } + ); + + let path = + graphDiv.data[traceIndex].transforms[indexOfSplitTransform].styles[ + indexOfStyleObject + ].value; + + attr.split('.').forEach(p => { + if (!path[p]) { + path[p] = {}; + } + path = path[p]; + }); + + return nestedProperty( + graphDiv.data[traceIndex].transforms[indexOfSplitTransform].styles[ + indexOfStyleObject + ].value, + attr + ); + } + + return splitTraceGroup.map(g => getProp(g)); +}; From 2767d66175950b95ca8d84d60c2370f4404f6d24 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Wed, 8 Aug 2018 13:51:43 -0400 Subject: [PATCH 2/9] More refactorings --- src/components/containers/TraceAccordion.js | 264 +++++++++----------- src/components/fields/MarkerColor.js | 14 +- src/default_panels/GraphSubplotsPanel.js | 2 +- src/default_panels/StyleTracesPanel.js | 2 +- 4 files changed, 124 insertions(+), 158 deletions(-) diff --git a/src/components/containers/TraceAccordion.js b/src/components/containers/TraceAccordion.js index ede911949..9d1cbf852 100644 --- a/src/components/containers/TraceAccordion.js +++ b/src/components/containers/TraceAccordion.js @@ -10,51 +10,60 @@ import {Tab, Tabs, TabList, TabPanel} from 'react-tabs'; const TraceFold = connectTraceToPlot(PlotlyFold); class TraceAccordion extends Component { - renderGroupedTraceFolds(groupedTraces) { - if (this.props.useFullData) { - const dataArrayPositionsByTraceType = {}; - const fullDataArrayPositionsByTraceType = {}; - - Object.keys(groupedTraces).forEach(traceType => { - const dataIndexes = []; - const fullDataIndexes = []; - - this.context.fullData.forEach((trace, fullDataIndex) => { - if (trace.type === traceType && trace._expandedIndex) { - fullDataIndexes.push(trace._expandedIndex); - } - if (trace.type === traceType && !trace._expandedIndex) { - fullDataIndexes.push(fullDataIndex); - } - if (trace.type === traceType) { - dataIndexes.push(trace.index); - } - }); + constructor(props, context) { + super(props, context); + this.setLocals(props, context); + } - dataArrayPositionsByTraceType[traceType] = dataIndexes; - fullDataArrayPositionsByTraceType[traceType] = fullDataIndexes; - }); + componentWillReceiveProps(nextProps, nextContext) { + this.setLocals(nextProps, nextContext); + } - return Object.keys(groupedTraces).map((traceType, index) => { - return ( - - {this.props.children} - - ); - }); + setLocals(props, context) { + // we don't want to include analysis transforms when we're in the create panel + const base = props.canGroup ? context.fullData : context.data; + + this.filteredTracesFullDataPositions = []; + this.filteredTraces = base.filter((t, i) => { + if (props.excludeFits) { + return !(t.transforms && t.transforms.every(tr => tr.type === 'fit')); + } + this.filteredTracesFullDataPositions.push(i); + return true; + }); + } + + renderGroupedTraceFolds() { + if (!this.filteredTraces.length || this.filteredTraces.length < 2) { + return null; } - return Object.keys(groupedTraces).map((traceType, index) => { + const dataArrayPositionsByTraceType = {}; + const fullDataArrayPositionsByTraceType = {}; + + this.filteredTraces.forEach((trace, index) => { + const traceType = plotlyTraceToCustomTrace(trace.type); + if (!dataArrayPositionsByTraceType[traceType]) { + dataArrayPositionsByTraceType[traceType] = []; + } + + if (!fullDataArrayPositionsByTraceType[traceType]) { + fullDataArrayPositionsByTraceType[traceType] = []; + } + + dataArrayPositionsByTraceType[traceType].push(trace.index); + fullDataArrayPositionsByTraceType[traceType].push( + this.filteredTracesFullDataPositions[index] + ); + }); + + return Object.keys(fullDataArrayPositionsByTraceType).map((type, index) => { return ( {this.props.children} @@ -62,30 +71,19 @@ class TraceAccordion extends Component { }); } - renderIndividualTraceFolds(filteredTraces, filteredTracesFullDataPositions) { - if (this.props.useFullData) { - return filteredTraces.map((d, i) => { - return ( - - {this.props.children} - - ); - }); + renderUngroupedTraceFolds() { + if (!this.filteredTraces.length) { + return null; } - return filteredTraces.map((d, i) => { + return this.filteredTraces.map((d, i) => { return ( {this.props.children} @@ -93,111 +91,78 @@ class TraceAccordion extends Component { }); } - renderCanAddPanel(filteredTraces, filteredTracesFullDataPositions) { - const _ = this.context.localize; + renderTraceFolds() { + if (!this.filteredTraces.length) { + return null; + } - const addAction = { - label: _('Trace'), - handler: ({onUpdate}) => { - if (onUpdate) { - onUpdate({ - type: EDITOR_ACTIONS.ADD_TRACE, - }); - } - }, - }; - - return ( - - {filteredTraces.length - ? this.renderIndividualTraceFolds( - filteredTraces, - filteredTracesFullDataPositions - ) - : null} - - ); + return this.filteredTraces.map((d, i) => { + return ( + + {this.props.children} + + ); + }); } render() { - const {data = [], localize: _, fullData = []} = this.context; - const {canAdd, canGroup, excludeFits, useFullData} = this.props; - const base = useFullData ? fullData : data; - - // we don't want to include analysis transforms when we're in the create panel - const filteredTracesFullDataPositions = []; - const filteredTraces = base.filter((t, i) => { - if (excludeFits) { - return !(t.transforms && t.transforms.every(tr => tr.type === 'fit')); - } - filteredTracesFullDataPositions.push(i); - return true; - }); - - const groupedTraces = filteredTraces.reduce( - (allTraces, nextTrace, index) => { - const traceType = plotlyTraceToCustomTrace(nextTrace); - const adjustedIndex = useFullData ? nextTrace.index : index; - if (!allTraces[traceType]) { - allTraces[traceType] = []; - } - allTraces[traceType].push(adjustedIndex); - return allTraces; - }, - {} - ); + const {canAdd, canGroup} = this.props; + const _ = this.context.localize; if (canAdd) { - return this.renderCanAddPanel( - filteredTraces, - filteredTracesFullDataPositions - ); - } + const addAction = { + label: _('Trace'), + handler: ({onUpdate}) => { + if (onUpdate) { + onUpdate({ + type: EDITOR_ACTIONS.ADD_TRACE, + }); + } + }, + }; - if ( - canGroup && - filteredTraces.length > 1 && - Object.keys(groupedTraces).length > 0 - ) { return ( - - - - {_('Individually')} - {_('By Type')} - - - - {filteredTraces.length - ? this.renderIndividualTraceFolds( - filteredTraces, - filteredTracesFullDataPositions - ) - : null} - - - - - {Object.keys(groupedTraces).length - ? this.renderGroupedTraceFolds(groupedTraces) - : null} - - - - + + {this.renderTraceFolds()} + ); } - return ( - - {filteredTraces.length - ? this.renderIndividualTraceFolds( - filteredTraces, - filteredTracesFullDataPositions - ) - : null} - - ); + if (canGroup) { + if (this.filteredTraces.length === 1) { + return ( + + {this.renderUngroupedTraceFolds()} + + ); + } + + if (this.filteredTraces.length > 1) { + return ( + + + + {_('Individually')} + {_('By Type')} + + + {this.renderUngroupedTraceFolds()} + + + {this.renderGroupedTraceFolds()} + + + + ); + } + } + + return {this.renderTraceFolds()}; } } @@ -213,7 +178,6 @@ TraceAccordion.propTypes = { children: PropTypes.node, excludeFits: PropTypes.bool, messageIfEmptyFold: PropTypes.string, - useFullData: PropTypes.bool, }; export default TraceAccordion; diff --git a/src/components/fields/MarkerColor.js b/src/components/fields/MarkerColor.js index b1e2018a4..f5688c6ca 100644 --- a/src/components/fields/MarkerColor.js +++ b/src/components/fields/MarkerColor.js @@ -91,8 +91,10 @@ class UnconnectedMarkerColor extends Component { (Array.isArray(this.props.fullValue) && this.props.fullValue.includes(MULTI_VALUED)) || (this.props.container.marker && + this.props.container.marker.colorscale && this.props.container.marker.colorscale === MULTI_VALUED) || (this.props.container.marker && + this.props.container.marker.colorsrc && this.props.container.marker.colorsrc === MULTI_VALUED) || (this.props.container.marker && this.props.container.marker.color && @@ -130,12 +132,12 @@ class UnconnectedMarkerColor extends Component { renderVariableControls() { const multiValued = - (this.props.container && - this.props.container.marker && - (this.props.container.marker.colorscale && - this.props.container.marker.colorscale === MULTI_VALUED)) || - (this.props.container.marker.colorsrc && - this.props.container.marker.colorsrc === MULTI_VALUED); + this.props.container && + this.props.container.marker && + ((this.props.container.marker.colorscale && + this.props.container.marker.colorscale === MULTI_VALUED) || + (this.props.container.marker.colorsrc && + this.props.container.marker.colorsrc === MULTI_VALUED)); return ( diff --git a/src/default_panels/GraphSubplotsPanel.js b/src/default_panels/GraphSubplotsPanel.js index 04ed56e41..3aa101da5 100644 --- a/src/default_panels/GraphSubplotsPanel.js +++ b/src/default_panels/GraphSubplotsPanel.js @@ -17,7 +17,7 @@ import { import {TRACE_TO_AXIS} from '../lib/constants'; const GraphSubplotsPanel = (props, {localize: _}) => ( - + diff --git a/src/default_panels/StyleTracesPanel.js b/src/default_panels/StyleTracesPanel.js index cbf088363..f91fc5497 100644 --- a/src/default_panels/StyleTracesPanel.js +++ b/src/default_panels/StyleTracesPanel.js @@ -39,7 +39,7 @@ import { } from '../components/fields/derived'; const StyleTracesPanel = (props, {localize: _}) => ( - + Date: Wed, 8 Aug 2018 14:30:54 -0400 Subject: [PATCH 3/9] Properly handle creation of transform.styles --- src/components/fields/DataSelector.js | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/components/fields/DataSelector.js b/src/components/fields/DataSelector.js index 85da82ece..766a4c838 100644 --- a/src/components/fields/DataSelector.js +++ b/src/components/fields/DataSelector.js @@ -87,12 +87,36 @@ export class UnconnectedDataSelector extends Component { if ( this.props.container.type && this.props.container.type === 'groupby' && + this.props.container.styles && + !this.props.container.styles.length && data ) { - const styles = data.map(groupEl => ({target: groupEl, value: {}})); + const dedupedGroups = []; + data.forEach(group => { + if (!dedupedGroups.includes(group)) { + dedupedGroups.push(group); + } + }); + + const styles = dedupedGroups.map(groupEl => ({ + target: groupEl, + value: {}, + })); + update.styles = styles; } + // When clearing the data selector of groupby transforms, we want to clear + // all the styles we've added + if ( + this.props.container.type && + this.props.container.type === 'groupby' && + this.props.container.styles.length && + value === null + ) { + update.styles = []; + } + this.props.updateContainer(update); } @@ -146,6 +170,7 @@ UnconnectedDataSelector.contextTypes = { toSrc: PropTypes.func.isRequired, fromSrc: PropTypes.func.isRequired, }), + container: PropTypes.object, }; function modifyPlotProps(props, context, plotProps) { From bb52e27b870b998eb4505c9ce1f9f15fb131d3da Mon Sep 17 00:00:00 2001 From: VeraZab Date: Wed, 8 Aug 2018 15:54:45 -0400 Subject: [PATCH 4/9] Disable variable marker.color controls on split traces and in grouped panel --- src/components/fields/DataSelector.js | 2 +- src/components/fields/MarkerColor.js | 142 ++++++++++++++------------ 2 files changed, 80 insertions(+), 64 deletions(-) diff --git a/src/components/fields/DataSelector.js b/src/components/fields/DataSelector.js index 766a4c838..a5be794b0 100644 --- a/src/components/fields/DataSelector.js +++ b/src/components/fields/DataSelector.js @@ -49,7 +49,7 @@ export class UnconnectedDataSelector extends Component { Array.isArray(this.fullValue); } - this.hasData = props.attr in props.container; + this.hasData = props.container ? props.attr in props.container : false; } updatePlot(value) { diff --git a/src/components/fields/MarkerColor.js b/src/components/fields/MarkerColor.js index f5688c6ca..30fd0a5b2 100644 --- a/src/components/fields/MarkerColor.js +++ b/src/components/fields/MarkerColor.js @@ -156,72 +156,87 @@ class UnconnectedMarkerColor extends Component { render() { const {attr} = this.props; - const {localize: _} = this.context; - const {type} = this.state; - const options = [ - {label: _('Constant'), value: 'constant'}, - {label: _('Variable'), value: 'variable'}, - ]; + const {localize: _, container} = this.context; - return ( - - - - - - {!type ? null : ( - - {type === 'constant' - ? _('All points in a trace are colored in the same color.') - : _('Each point in a trace is colored according to data.')} - - )} + // TO DO: https://github.com/plotly/react-chart-editor/issues/654 + const noSplitsPresent = + container && + (!container.transforms || + !container.transforms.filter(t => t.type === 'groupby').length); + + if (noSplitsPresent) { + const {type} = this.state; + const options = [ + {label: _('Constant'), value: 'constant'}, + {label: _('Variable'), value: 'variable'}, + ]; + + return ( + + + + + + {!type ? null : ( + + {type === 'constant' + ? _('All points in a trace are colored in the same color.') + : _('Each point in a trace is colored according to data.')} + + )} + + + {!type + ? null + : type === 'constant' + ? this.renderConstantControls() + : this.renderVariableControls()} + {type === 'constant' ? null : ( + + + + + + + + + )} + + ); + } - {!type - ? null - : type === 'constant' - ? this.renderConstantControls() - : this.renderVariableControls()} - - {type === 'constant' ? null : ( - - - - - - - - - )} - + return ( + + {this.renderConstantControls()} + ); } } @@ -236,6 +251,7 @@ UnconnectedMarkerColor.contextTypes = { localize: PropTypes.func, updateContainer: PropTypes.func, traceIndexes: PropTypes.array, + container: PropTypes.object, }; export default connectToContainer(UnconnectedMarkerColor); From 7e156a025956cf129e9e106225c615331191b6b6 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Wed, 8 Aug 2018 16:16:13 -0400 Subject: [PATCH 5/9] Cannot have 2 split transforms on one trace --- src/components/containers/TransformAccordion.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/components/containers/TransformAccordion.js b/src/components/containers/TransformAccordion.js index a37cdc01a..6211bc54e 100644 --- a/src/components/containers/TransformAccordion.js +++ b/src/components/containers/TransformAccordion.js @@ -83,9 +83,17 @@ class TransformAccordion extends Component { )); + // cannot have 2 Split transforms on one trace: + // https://github.com/plotly/plotly.js/issues/1742 + const addActionOptions = + container.transforms && + container.transforms.some(t => t.type === 'groupby') + ? transformTypes.filter(t => t.type !== 'groupby') + : transformTypes; + const addAction = { label: _('Transform'), - handler: transformTypes.map(({label, type}) => { + handler: addActionOptions.map(({label, type}) => { return { label, handler: context => { From ef3ff2220ac7611f0077a04396384033bb3790f1 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Wed, 8 Aug 2018 17:03:29 -0400 Subject: [PATCH 6/9] Fix trace type computation --- src/components/containers/TraceAccordion.js | 2 +- src/lib/customTraceType.js | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/components/containers/TraceAccordion.js b/src/components/containers/TraceAccordion.js index 9d1cbf852..9d1fbd7df 100644 --- a/src/components/containers/TraceAccordion.js +++ b/src/components/containers/TraceAccordion.js @@ -42,7 +42,7 @@ class TraceAccordion extends Component { const fullDataArrayPositionsByTraceType = {}; this.filteredTraces.forEach((trace, index) => { - const traceType = plotlyTraceToCustomTrace(trace.type); + const traceType = plotlyTraceToCustomTrace(trace); if (!dataArrayPositionsByTraceType[traceType]) { dataArrayPositionsByTraceType[traceType] = []; } diff --git a/src/lib/customTraceType.js b/src/lib/customTraceType.js index def6ceaee..5b1e3017d 100644 --- a/src/lib/customTraceType.js +++ b/src/lib/customTraceType.js @@ -1,6 +1,12 @@ import {COLORS} from 'lib/constants'; export function plotlyTraceToCustomTrace(trace) { + if (typeof trace !== 'object') { + throw new Error( + `trace provided to plotlyTraceToCustomTrace function should be an object, received ${typeof trace}` + ); + } + const gl = 'gl'; const type = trace.type ? trace.type.endsWith(gl) From e5049c1416e4e5c488e0bd124a239ad4818b98bc Mon Sep 17 00:00:00 2001 From: VeraZab Date: Wed, 8 Aug 2018 17:03:41 -0400 Subject: [PATCH 7/9] Use for Each --- src/shame.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shame.js b/src/shame.js index 8a527b945..9ccebe5bd 100644 --- a/src/shame.js +++ b/src/shame.js @@ -125,7 +125,7 @@ export const shamefullyCreateSplitStyles = ( let indexOfSplitTransform = null; - graphDiv.data[traceIndex].transforms.filter((t, i) => { + graphDiv.data[traceIndex].transforms.forEach((t, i) => { if (t.type === 'groupby') { indexOfSplitTransform = i; } @@ -134,7 +134,7 @@ export const shamefullyCreateSplitStyles = ( function getProp(group) { let indexOfStyleObject = null; - graphDiv.data[traceIndex].transforms[indexOfSplitTransform].styles.filter( + graphDiv.data[traceIndex].transforms[indexOfSplitTransform].styles.forEach( (s, i) => { if (s.target.toString() === group) { indexOfStyleObject = i; From f6c305922cbd2de0040b7b1ef27a80c3f05935da Mon Sep 17 00:00:00 2001 From: VeraZab Date: Wed, 8 Aug 2018 17:12:46 -0400 Subject: [PATCH 8/9] Remove extra panel --- src/components/containers/TraceAccordion.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/containers/TraceAccordion.js b/src/components/containers/TraceAccordion.js index 9d1fbd7df..faad5d797 100644 --- a/src/components/containers/TraceAccordion.js +++ b/src/components/containers/TraceAccordion.js @@ -137,7 +137,7 @@ class TraceAccordion extends Component { if (this.filteredTraces.length === 1) { return ( - {this.renderUngroupedTraceFolds()} + {this.renderUngroupedTraceFolds()} ); } From 6dab0a43ffcf98646c92af0d02f5f26bcf88d5d0 Mon Sep 17 00:00:00 2001 From: VeraZab Date: Wed, 8 Aug 2018 17:59:54 -0400 Subject: [PATCH 9/9] Move DataSelector logic into shame --- src/EditorControls.js | 6 ++-- src/components/fields/DataSelector.js | 33 ---------------------- src/shame.js | 40 ++++++++++++++++++++++++++- 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/src/EditorControls.js b/src/EditorControls.js index 53f4fad98..e21ae8f33 100644 --- a/src/EditorControls.js +++ b/src/EditorControls.js @@ -7,7 +7,8 @@ import { shamefullyAdjustAxisRef, shamefullyAdjustGeo, shamefullyAddTableColumns, - shamefullyCreateSplitStyles, + shamefullyCreateSplitStyleProps, + shamefullyAdjustSplitStyleTargetContainers, } from './shame'; import {EDITOR_ACTIONS} from './lib/constants'; import isNumeric from 'fast-isnumeric'; @@ -67,6 +68,7 @@ class EditorControls extends Component { shamefullyClearAxisTypes(graphDiv, payload); shamefullyAdjustAxisRef(graphDiv, payload); shamefullyAddTableColumns(graphDiv, payload); + shamefullyAdjustSplitStyleTargetContainers(graphDiv, payload); for (let i = 0; i < payload.traceIndexes.length; i++) { for (const attr in payload.update) { @@ -79,7 +81,7 @@ class EditorControls extends Component { const value = payload.update[attr]; if (splitTraceGroup) { - props = shamefullyCreateSplitStyles( + props = shamefullyCreateSplitStyleProps( graphDiv, attr, traceIndex, diff --git a/src/components/fields/DataSelector.js b/src/components/fields/DataSelector.js index a5be794b0..29e928477 100644 --- a/src/components/fields/DataSelector.js +++ b/src/components/fields/DataSelector.js @@ -84,39 +84,6 @@ export class UnconnectedDataSelector extends Component { } ); - if ( - this.props.container.type && - this.props.container.type === 'groupby' && - this.props.container.styles && - !this.props.container.styles.length && - data - ) { - const dedupedGroups = []; - data.forEach(group => { - if (!dedupedGroups.includes(group)) { - dedupedGroups.push(group); - } - }); - - const styles = dedupedGroups.map(groupEl => ({ - target: groupEl, - value: {}, - })); - - update.styles = styles; - } - - // When clearing the data selector of groupby transforms, we want to clear - // all the styles we've added - if ( - this.props.container.type && - this.props.container.type === 'groupby' && - this.props.container.styles.length && - value === null - ) { - update.styles = []; - } - this.props.updateContainer(update); } diff --git a/src/shame.js b/src/shame.js index 9ccebe5bd..1608d0db8 100644 --- a/src/shame.js +++ b/src/shame.js @@ -113,7 +113,45 @@ export const shamefullyAddTableColumns = (graphDiv, {traceIndexes, update}) => { } }; -export const shamefullyCreateSplitStyles = ( +export const shamefullyAdjustSplitStyleTargetContainers = ( + graphDiv, + {traceIndexes, update} +) => { + for (const attr in update) { + if (attr && attr.startsWith('transforms') && attr.endsWith('groups')) { + const transformIndex = parseInt(attr.split('[')[1], 10); + const transform = + graphDiv.data[traceIndexes[0]].transforms[transformIndex]; + + if (transform && transform.type === 'groupby' && transform.styles) { + // Create style containers for all groups + if (!transform.styles.length && update[attr]) { + const dedupedGroups = []; + update[attr].forEach(group => { + if (!dedupedGroups.includes(group)) { + dedupedGroups.push(group); + } + }); + + const styles = dedupedGroups.map(groupEl => ({ + target: groupEl, + value: {}, + })); + + update[`transforms[${transformIndex}].styles`] = styles; + } + + // When clearing the data selector of groupby transforms, we want to clear + // all the styles we've added + if (transform.styles.length && !update[attr]) { + update[`transforms[${transformIndex}].styles`] = []; + } + } + } + } +}; + +export const shamefullyCreateSplitStyleProps = ( graphDiv, attr, traceIndex,