Skip to content

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

Merged
merged 12 commits into from
Nov 21, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/DefaultEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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" />
Copy link
Collaborator

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 in layout, 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?

Copy link
Member Author

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.

Copy link
Member Author

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.

Copy link
Member Author

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

<BoxGap label={_('Box Width')} attr="boxgap" />
<BoxGap
label={_('Bar Padding')}
attr="bargroupgap"
showArrows={false}
/>
<BoxGap
label={_('Box Padding')}
attr="boxgroupgap"
showArrows={false}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this? It's already a NumericNoArrows component...

Copy link
Member Author

Choose a reason for hiding this comment

The 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" />
Expand Down
23 changes: 12 additions & 11 deletions src/components/containers/Section.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you see a use for this unpack, then wouldn't the if (child.type.modifyPlotProps) block possibly be relevant in that case too? ie should it be moved outside that if/else block, so the existences of supply and modify are independent?

}
} 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));
}
Expand Down Expand Up @@ -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;
8 changes: 4 additions & 4 deletions src/components/containers/__tests__/Section-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe('Section', () => {
it('is visible if it contains any visible children', () => {
// mode is visible with scatter. Hole is not visible. Section should show.
const wrapper = mount(
<TestEditor onUpdate={jest.fn()} {...fixtures.scatter({deref: true})}>
<TestEditor onUpdate={jest.fn()} {...fixtures.scatter()}>
<TraceSection traceIndex={0} name="test-section">
<Flaglist
attr="mode"
Expand Down Expand Up @@ -46,7 +46,7 @@ describe('Section', () => {

it('is visible if it contains any non attr children', () => {
const wrapper = mount(
<TestEditor onUpdate={jest.fn()} {...fixtures.scatter({deref: true})}>
<TestEditor onUpdate={jest.fn()} {...fixtures.scatter()}>
<Section name="test-section">
<Info>INFO</Info>
</Section>
Expand All @@ -61,7 +61,7 @@ describe('Section', () => {
it('is not visible if it contains no visible children', () => {
// pull and hole are not scatter attrs. Section should not show.
const wrapper = mount(
<TestEditor onUpdate={jest.fn()} {...fixtures.scatter({deref: true})}>
<TestEditor onUpdate={jest.fn()} {...fixtures.scatter()}>
<TraceSection traceIndex={0} name="test-section">
<Numeric attr="pull" min={0} max={1} step={0.1} traceIndex={0} />
<Numeric attr="hole" min={0} max={1} step={0.1} traceIndex={0} />
Expand All @@ -81,7 +81,7 @@ describe('Section', () => {

it('will render first menuPanel even with no visible attrs', () => {
const wrapper = mount(
<TestEditor onUpdate={jest.fn()} {...fixtures.scatter({deref: true})}>
<TestEditor onUpdate={jest.fn()} {...fixtures.scatter()}>
<Section name="test-section">
<MenuPanel show>
<Info>INFO</Info>
Expand Down
32 changes: 8 additions & 24 deletions src/components/fields/AxesRange.js
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});
24 changes: 4 additions & 20 deletions src/components/fields/AxesSelector.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,21 @@ import Field from './Field';
import PropTypes from 'prop-types';
import RadioBlocks from '../widgets/RadioBlocks';
import React, {Component} from 'react';
import {connectToContainer} from '../../lib';

export default class AxesSelector extends Component {
constructor(props, context) {
super(props, context);

if (!props.axesTargetHandler && !context.axesTargetHandler) {
if (!context.axesTargetHandler) {
throw new Error(
'AxesSelector must be nested within a connectAxesToPlot component ' +
'or passed axesTargetHandler and axesOptions props'
'AxesSelector must be nested within a connectAxesToPlot component'
);
}
}

render() {
let axesTargetHandler, axesOptions, axesTarget;
if (this.props.axesTargetHandler) {
axesTargetHandler = this.props.axesTargetHandler;
axesOptions = this.props.axesOptions;
axesTarget = this.props.axesTarget;
} else {
axesTargetHandler = this.context.axesTargetHandler;
axesOptions = this.context.axesOptions;
axesTarget = this.context.axesTarget;
}
const {axesTargetHandler, axesOptions, axesTarget} = this.context;

return (
<Field {...this.props}>
<RadioBlocks
Expand All @@ -39,12 +29,6 @@ export default class AxesSelector extends Component {
}
}

AxesSelector.propTypes = {
axesTargetHandler: PropTypes.func,
axesOptions: PropTypes.array,
axesTarget: PropTypes.string,
};

AxesSelector.contextTypes = {
axesTargetHandler: PropTypes.func,
axesOptions: PropTypes.array,
Expand Down
28 changes: 8 additions & 20 deletions src/components/fields/CanvasSize.js
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});
18 changes: 10 additions & 8 deletions src/components/fields/DataSelector.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,13 @@ import PropTypes from 'prop-types';
import React, {Component} from 'react';
import Field from './Field';
import nestedProperty from 'plotly.js/src/lib/nested_property';
import {connectToContainer} from '../../lib';
import {connectToContainer, unpackPlotProps} from '../../lib';

function attributeIsData(meta = {}) {
return meta.valType === 'data_array' || meta.arrayOk;
}

class DataSelector extends Component {
static modifyPlotProps(props, context, plotProps) {
if (attributeIsData(plotProps.attrMeta)) {
plotProps.isVisible = true;
}
}

constructor(props) {
super(props);

Expand Down Expand Up @@ -72,4 +66,12 @@ DataSelector.propTypes = {
...Field.propTypes,
};

export default connectToContainer(DataSelector);
function supplyPlotProps(props, context) {
const plotProps = unpackPlotProps(props, context);
if (attributeIsData(plotProps.attrMeta)) {
plotProps.isVisible = true;
}
return plotProps;
}

export default connectToContainer(DataSelector, {supplyPlotProps});
8 changes: 4 additions & 4 deletions src/components/fields/Numeric.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import PropTypes from 'prop-types';
import React, {Component} from 'react';
import {connectToContainer} from '../../lib';

class Numeric extends Component {
export class UnconnectedNumeric extends Component {
render() {
return (
<Field {...this.props}>
Expand All @@ -23,7 +23,7 @@ class Numeric extends Component {
}
}

Numeric.propTypes = {
UnconnectedNumeric.propTypes = {
defaultValue: PropTypes.number,
fullValue: PropTypes.func,
min: PropTypes.number,
Expand All @@ -34,8 +34,8 @@ Numeric.propTypes = {
...Field.propTypes,
};

Numeric.defaultProps = {
UnconnectedNumeric.defaultProps = {
showArrows: true,
};

export default connectToContainer(Numeric);
export default connectToContainer(UnconnectedNumeric);
40 changes: 40 additions & 0 deletions src/components/fields/__tests__/CanvasSize-test.js
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);
});
});
4 changes: 2 additions & 2 deletions src/components/fields/__tests__/Radio-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const Trace = connectTraceToPlot(Section);
describe('<Radio>', () => {
it('enables <Field> centering by default', () => {
const wrapper = mount(
<TestEditor plotly={plotly} {...fixtures.area({deref: 'true'})}>
<TestEditor plotly={plotly} {...fixtures.area()}>
<Trace traceIndex={0}>
<Radio
label="Connect Gaps"
Expand All @@ -30,7 +30,7 @@ describe('<Radio>', () => {

it('permits <Field> centering to be disabled', () => {
const wrapper = mount(
<TestEditor plotly={plotly} {...fixtures.area({deref: 'true'})}>
<TestEditor plotly={plotly} {...fixtures.area()}>
<Trace traceIndex={0}>
<Radio
center={false}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import {Numeric} from '../components/fields';
import {Fold, Panel, Section} from '../components/containers';
import NumericInput from '../components/widgets/NumericInputStatefulWrapper';
import NumericInput from '../../components/widgets/NumericInputStatefulWrapper';
import React from 'react';
import {EDITOR_ACTIONS} from '../constants';
import {TestEditor, fixtures, plotly} from '../lib/test-utils';
import {connectLayoutToPlot} from '../lib';
import connectLayoutToPlot from '../connectLayoutToPlot';
import {EDITOR_ACTIONS} from '../../constants';
import {Fold, Panel, Section} from '../../components/containers';
import {Numeric} from '../../components/fields';
import {TestEditor, fixtures, plotly} from '../test-utils';
import {mount} from 'enzyme';

const Layouts = [Panel, Fold, Section].map(connectLayoutToPlot);
Expand Down
Loading