-
-
Notifications
You must be signed in to change notification settings - Fork 115
Multi-value support in trace tabs #314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import React, {Component} from 'react'; | |
| import SymbolSelectorWidget from '../widgets/SymbolSelector'; | ||
| import nestedProperty from 'plotly.js/src/lib/nested_property'; | ||
| import {connectToContainer, tooLight} from 'lib'; | ||
| import {MULTI_VALUED} from '../../lib/constants'; | ||
|
|
||
| // TODO compute these from plotly.js | ||
| const SYMBOLS = [ | ||
|
|
@@ -353,27 +354,38 @@ const SYMBOLS = [ | |
| ]; | ||
|
|
||
| class SymbolSelector extends Component { | ||
| constructor(props) { | ||
| super(props); | ||
| this.setLocals(props); | ||
| constructor(props, context) { | ||
| super(props, context); | ||
| this.setLocals(props, context); | ||
| } | ||
|
|
||
| componentWillReceiveProps(nextProps) { | ||
| this.setLocals(nextProps); | ||
| componentWillReceiveProps(nextProps, nextContext) { | ||
| this.setLocals(nextProps, nextContext); | ||
| } | ||
|
|
||
| setLocals(props) { | ||
| setLocals(props, context) { | ||
| const {fullContainer} = props; | ||
| const {defaultContainer} = context; | ||
|
|
||
| this.markerColor = nestedProperty(fullContainer, 'marker.color').get(); | ||
| this.borderWidth = nestedProperty(fullContainer, 'marker.line.width').get(); | ||
|
|
||
| if (this.markerColor === MULTI_VALUED) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is very tricky: we don't actually end up using default values here and I couldn't figure out how to do it... we use the values from the first trace. I've added a note to fit'n'finish
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, that's what the current workspace does, sounds ok to me! |
||
| this.markerColor = nestedProperty(defaultContainer, 'marker.color').get(); | ||
| } | ||
|
|
||
| this.borderColor = this.markerColor; | ||
| if (this.borderWidth) { | ||
| this.borderColor = nestedProperty( | ||
| fullContainer, | ||
| 'marker.line.color' | ||
| ).get(); | ||
| if (this.borderColor === MULTI_VALUED) { | ||
| this.borderColor = nestedProperty( | ||
| defaultContainer, | ||
| 'marker.line.color' | ||
| ).get(); | ||
| } | ||
| } | ||
|
|
||
| if (this.props.is3D) { | ||
|
|
@@ -403,11 +415,14 @@ class SymbolSelector extends Component { | |
| } | ||
|
|
||
| SymbolSelector.propTypes = { | ||
| defaultValue: PropTypes.number, | ||
| defaultValue: PropTypes.string, | ||
| fullValue: PropTypes.any, | ||
| updatePlot: PropTypes.func, | ||
| ...Field.propTypes, | ||
| }; | ||
| SymbolSelector.contextTypes = { | ||
| defaultContainer: PropTypes.object, | ||
| }; | ||
|
|
||
| SymbolSelector.defaultProps = { | ||
| showArrows: true, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,27 +20,27 @@ import { | |
| TraceOrientation, | ||
| ColorscalePicker, | ||
| HoverInfo, | ||
| Dropdown, | ||
| FillDropdown, | ||
| FontSelector, | ||
| } from '../components'; | ||
|
|
||
| import {localize} from '../lib'; | ||
|
|
||
| const StyleTracesPanel = ({localize: _}) => ( | ||
| <TraceAccordion canGroup> | ||
| <Section name={_('Trace')} attr="name"> | ||
| <TextEditor label={_('Name')} attr="name" richTextOnly /> | ||
| <TraceOrientation | ||
| label={_('Orientation')} | ||
| attr="orientation" | ||
| options={[ | ||
| {label: _('Vertical'), value: 'v'}, | ||
| {label: _('Horizontal'), value: 'h'}, | ||
| ]} | ||
| /> | ||
| <TextEditor label={_('Name')} attr="name" richTextOnly /> | ||
| <TraceOrientation | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh yeah 👍 |
||
| label={_('Orientation')} | ||
| attr="orientation" | ||
| options={[ | ||
| {label: _('Vertical'), value: 'v'}, | ||
| {label: _('Horizontal'), value: 'h'}, | ||
| ]} | ||
| /> | ||
|
|
||
| <Numeric label={_('Opacity')} step={0.1} attr="opacity" /> | ||
| <ColorPicker label={_('Color')} attr="color" /> | ||
| </Section> | ||
| <Numeric label={_('Opacity')} step={0.1} attr="opacity" /> | ||
| <ColorPicker label={_('Color')} attr="color" /> | ||
|
|
||
| <Section name={_('Text Attributes')}> | ||
| <Flaglist | ||
|
|
@@ -143,6 +143,28 @@ const StyleTracesPanel = ({localize: _}) => ( | |
| <LineShapeSelector label={_('Shape')} attr="line.shape" /> | ||
| </TraceTypeSection> | ||
|
|
||
| <TraceTypeSection name={_('Text')} traceTypes={['scatter']}> | ||
| <FontSelector label={_('Typeface')} attr="textfont.family" /> | ||
| <Numeric label={_('Font Size')} attr="textfont.size" units="px" /> | ||
| <ColorPicker label={_('Font Color')} attr="textfont.color" /> | ||
| <Dropdown | ||
| label={_('Text Position')} | ||
| attr="textposition" | ||
| clearable={false} | ||
| options={[ | ||
| {label: _('Top Left'), value: 'top left'}, | ||
| {label: _('Top Center'), value: 'top center'}, | ||
| {label: _('Top Right'), value: 'top right'}, | ||
| {label: _('Middle Left'), value: 'middle left'}, | ||
| {label: _('Middle Center'), value: 'middle center'}, | ||
| {label: _('Middle Right'), value: 'middle right'}, | ||
| {label: _('Bottom Left'), value: 'bottom left'}, | ||
| {label: _('Bottom Center'), value: 'bottom center'}, | ||
| {label: _('Bottom Right'), value: 'bottom right'}, | ||
| ]} | ||
| /> | ||
| </TraceTypeSection> | ||
|
|
||
| <Section name={_('Colorscale')}> | ||
| <ColorscalePicker label={_('Colorscale')} attr="colorscale" /> | ||
| <Radio | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import {MULTI_VALUED} from './constants'; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was moved from |
||
| import {isPlainObject} from '../lib'; | ||
|
|
||
| /** | ||
| * Deep-copies the value using JSON. Underscored (private) keys are removed. | ||
| * @param {*} value Some nested value from the plotDiv object. | ||
| * @returns {*} A deepcopy of the value. | ||
| */ | ||
| function deepCopyPublic(value) { | ||
| if (typeof value === 'undefined') { | ||
| return value; | ||
| } | ||
|
|
||
| const skipPrivateKeys = (key, value) => (key.startsWith('_') ? 0 : value); | ||
|
|
||
| return window.JSON.parse(window.JSON.stringify(value, skipPrivateKeys)); | ||
| } | ||
|
|
||
| function setMultiValuedContainer(intoObj, fromObj, key, config = {}) { | ||
| var intoVal = intoObj[key], | ||
| fromVal = fromObj[key]; | ||
|
|
||
| var searchArrays = config.searchArrays; | ||
|
|
||
| // don't merge private attrs | ||
| if ( | ||
| (typeof key === 'string' && key.charAt(0) === '_') || | ||
| typeof intoVal === 'function' || | ||
| key === 'module' | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| // already a mixture of values, can't get any worse | ||
| if (intoVal === MULTI_VALUED) { | ||
| return; | ||
| } else if (intoVal === void 0) { | ||
| // if the original doesn't have the key it's because that key | ||
| // doesn't do anything there - so use the new value | ||
| // note that if fromObj doesn't have a key in intoObj we will not | ||
| // attempt to merge them at all, so this behavior makes the merge | ||
| // independent of order. | ||
| intoObj[key] = fromVal; | ||
| } else if (key === 'colorscale') { | ||
| // colorscales are arrays... need to stringify before comparing | ||
| // (other vals we don't want to stringify, as differences could | ||
| // potentially be real, like 'false' and false) | ||
| if (String(intoVal) !== String(fromVal)) { | ||
| intoObj[key] = MULTI_VALUED; | ||
| } | ||
| } else if (Array.isArray(intoVal)) { | ||
| // in data, other arrays are data, which we don't care about | ||
| // for styling purposes | ||
| if (!searchArrays) { | ||
| return; | ||
| } | ||
| // in layout though, we need to recurse into arrays | ||
| for (var i = 0; i < fromVal.length; i++) { | ||
| setMultiValuedContainer(intoVal, fromVal, i, searchArrays); | ||
| } | ||
| } else if (isPlainObject(fromVal)) { | ||
| // recurse into objects | ||
| if (!isPlainObject(intoVal)) { | ||
| throw new Error('tried to merge object into non-object: ' + key); | ||
| } | ||
| Object.keys(fromVal).forEach(function(key2) { | ||
| setMultiValuedContainer(intoVal, fromVal, key2, searchArrays); | ||
| }); | ||
| } else if (isPlainObject(intoVal)) { | ||
| throw new Error('tried to merge non-object into object: ' + key); | ||
| } else if (intoVal !== fromVal) { | ||
| // different non-empty values - | ||
| intoObj[key] = MULTI_VALUED; | ||
| } | ||
| } | ||
|
|
||
| export {deepCopyPublic, setMultiValuedContainer}; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why we were checking
_fullInputhere but it was failing in multi-valued contextsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah _fullInput is where the true trace type for financial traces lie
_fullData.type for an ohlc trace will show scatter, _fullData._fullInput.type, will show ohlc
And all traces are supposed to have a _fullInput key, with type..so that's why I'm looking here
We could also look in Data.type..
But yeah, if you don't look there, it won't properly look for financials
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, right, I keep forgetting to check stuff with financial traces. right now they cause crashes apparently... joy!