-
-
Notifications
You must be signed in to change notification settings - Fork 112
Test nested connections and add nested connection #91
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 5 commits
adfecc9
74674e2
7ac3006
1ba847d
04c83ef
c3445bd
f1ce3ed
92661db
1c137f3
cd37d4c
94a27c1
96d2ba0
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 |
---|---|---|
|
@@ -24,6 +24,8 @@ import { | |
import {DEFAULT_FONTS} from './constants'; | ||
import {localize, connectAxesToLayout, connectLayoutToPlot} from './lib'; | ||
|
||
import {BoxGap} from './shame'; | ||
|
||
const LayoutPanel = connectLayoutToPlot(Panel); | ||
const AxesFold = connectAxesToLayout(Fold); | ||
|
||
|
@@ -145,6 +147,21 @@ class DefaultEditor extends Component { | |
<ColorPicker label={_('Border Color')} attr="marker.line.color" /> | ||
</TraceMarkerSection> | ||
|
||
<Section name={_('Size and Spacing')}> | ||
<BoxGap label={_('Bar Width')} attr="bargap" /> | ||
<BoxGap label={_('Box Width')} attr="boxgap" /> | ||
<BoxGap | ||
label={_('Bar Padding')} | ||
attr="bargroupgap" | ||
showArrows={false} | ||
/> | ||
<BoxGap | ||
label={_('Box Padding')} | ||
attr="boxgroupgap" | ||
showArrows={false} | ||
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. Do you need this? It's already a 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. Nope! Forgot to take it out |
||
/> | ||
</Section> | ||
|
||
<Section name={_('Lines')}> | ||
<Numeric label={_('Width')} step={1.0} attr="line.width" /> | ||
<ColorPicker label={_('Line Color')} attr="line.color" /> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,13 @@ import MenuPanel from './MenuPanel'; | |
import React, {Component, cloneElement} from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import unpackPlotProps from '../../lib/unpackPlotProps'; | ||
import {containerConnectedContextTypes} from '../../lib/connectToContainer'; | ||
|
||
function childIsVisible(child) { | ||
return Boolean((child.props.plotProps || {}).isVisible); | ||
} | ||
|
||
class Section extends Component { | ||
export default class Section extends Component { | ||
constructor(props, context) { | ||
super(props, context); | ||
|
||
|
@@ -49,11 +50,19 @@ class Section extends Component { | |
if (child.plotProps) { | ||
plotProps = child.plotProps; | ||
} else if (isAttr) { | ||
plotProps = unpackPlotProps(child.props, context, child.type); | ||
if (child.type.supplyPlotProps) { | ||
plotProps = child.type.supplyPlotProps(child.props, context); | ||
} else { | ||
plotProps = unpackPlotProps(child.props, context); | ||
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. If you see a use for this |
||
} | ||
} else { | ||
plotProps = {isVisible: true}; | ||
} | ||
|
||
// assign plotProps as a prop of children. If they are connectedToContainer | ||
// it will see plotProps and skip recomputing them. | ||
const childProps = Object.assign({plotProps}, child.props); | ||
|
||
childProps.key = i; | ||
attrChildren.push(cloneElement(child, childProps)); | ||
} | ||
|
@@ -84,12 +93,4 @@ Section.propTypes = { | |
name: PropTypes.string, | ||
}; | ||
|
||
Section.contextTypes = { | ||
container: PropTypes.object, | ||
defaultContainer: PropTypes.object, | ||
fullContainer: PropTypes.object, | ||
getValObject: PropTypes.func, | ||
updateContainer: PropTypes.func, | ||
}; | ||
|
||
export default Section; | ||
Section.contextTypes = containerConnectedContextTypes; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,29 +1,13 @@ | ||
import Numeric from './Numeric'; | ||
import React, {Component} from 'react'; | ||
import {connectToContainer} from '../../lib'; | ||
import {connectToContainer, unpackPlotProps} from '../../lib'; | ||
|
||
class AxesRange extends Component { | ||
static modifyPlotProps(props, context, plotProps) { | ||
if (!plotProps.isVisible) { | ||
return; | ||
} | ||
const {fullContainer} = plotProps; | ||
if (fullContainer && fullContainer.autorange) { | ||
plotProps.isVisible = false; | ||
} | ||
} | ||
|
||
render() { | ||
return <Numeric {...this.props} />; | ||
function supplyPlotProps(props, context) { | ||
const plotProps = unpackPlotProps(props, context); | ||
const {fullContainer} = plotProps; | ||
if (plotProps.isVisible && fullContainer && fullContainer.autorange) { | ||
plotProps.isVisible = false; | ||
} | ||
return plotProps; | ||
} | ||
|
||
AxesRange.propTypes = { | ||
...Numeric.propTypes, | ||
}; | ||
|
||
AxesRange.defaultProps = { | ||
showArrows: false, | ||
}; | ||
|
||
export default connectToContainer(AxesRange); | ||
export default connectToContainer(Numeric, {supplyPlotProps}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,13 @@ | ||
import Numeric from './Numeric'; | ||
import React, {Component} from 'react'; | ||
import {connectToContainer} from '../../lib'; | ||
import {connectToContainer, unpackPlotProps} from '../../lib'; | ||
|
||
class CanvasSize extends Component { | ||
static modifyPlotProps(props, context, plotProps) { | ||
if (!plotProps.isVisible) { | ||
return; | ||
} | ||
const {fullContainer} = plotProps; | ||
if (fullContainer && fullContainer.autosize) { | ||
plotProps.isVisible = false; | ||
} | ||
} | ||
|
||
render() { | ||
return <Numeric {...this.props} />; | ||
function supplyPlotProps(props, context) { | ||
const plotProps = unpackPlotProps(props, context); | ||
const {fullContainer} = plotProps; | ||
if (plotProps.isVisible && fullContainer && fullContainer.autosize) { | ||
plotProps.isVisible = false; | ||
} | ||
return plotProps; | ||
} | ||
|
||
CanvasSize.propTypes = { | ||
...Numeric.propTypes, | ||
}; | ||
|
||
export default connectToContainer(CanvasSize); | ||
export default connectToContainer(Numeric, {supplyPlotProps}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import React from 'react'; | ||
import {connectLayoutToPlot} from '../../../lib'; | ||
import {Panel, Fold, Numeric} from '../../'; | ||
import {TestEditor, fixtures} from '../../../lib/test-utils'; | ||
import {mount} from 'enzyme'; | ||
import CanvasSize from '../CanvasSize'; | ||
|
||
describe('CanvasSize', () => { | ||
const LayoutPanel = connectLayoutToPlot(Panel); | ||
|
||
it('is hidden when autosize is true', () => { | ||
const fixtureProps = fixtures.scatter({layout: {autosize: true}}); | ||
const wrapper = mount( | ||
<TestEditor {...{...fixtureProps}}> | ||
<LayoutPanel name="Layout"> | ||
<Fold name="Canvas"> | ||
<CanvasSize attr="width" /> | ||
</Fold> | ||
</LayoutPanel> | ||
</TestEditor> | ||
).find(Numeric); | ||
|
||
expect(wrapper.length).toBe(0); | ||
}); | ||
|
||
it('is visible when autosize is false', () => { | ||
const fixtureProps = fixtures.scatter({layout: {autosize: false}}); | ||
const wrapper = mount( | ||
<TestEditor {...{...fixtureProps}}> | ||
<LayoutPanel name="Layout"> | ||
<Fold name="Canvas"> | ||
<CanvasSize attr="width" /> | ||
</Fold> | ||
</LayoutPanel> | ||
</TestEditor> | ||
).find(Numeric); | ||
|
||
expect(wrapper.length).toBe(1); | ||
}); | ||
}); |
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.
Hmm, what do we call this component? I feel like its name should indicate that it's numeric and that the
attr
you specify is inlayout
, not in a trace. But it's not specific to boxes or gaps.Also, the label 'Bar Width' isn't quite right, is it? ie an increased
bargap
decreases the width, right?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.
TraceLayoutNumericSansArrows.
I copied the workspace descriptions but looks like it is doing a transformation on the input. Maybe (1-x)*100. I'll implement this transform (and the reverse onUpdate) in this PR and tag you on the commit.
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.
With the transformation it will again be a hyper specific component so perhaps
BoxGap
is more applicable.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.
Though
BoxWidth
is probably clearer