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 1 commit
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
4 changes: 4 additions & 0 deletions src/components/containers/Section.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ export default class Section extends Component {
} 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
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);
33 changes: 32 additions & 1 deletion src/shame.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
/*
* DELETE THIS FILE AND EVERYTHING IN IT.
* DELETE THIS FILE. EVERYTHING NEEDS TO FIND A HOME.
*/
import {list} from 'plotly.js/src/plots/cartesian/axis_ids';
import {UnconnectedNumeric} from './components/fields/Numeric';
import {
connectLayoutToPlot,
connectToContainer,
getLayoutContext,
unpackPlotProps,
} from './lib';

export function noShame(opts) {
if (opts.plotly && !opts.plotly.Axes) {
Expand All @@ -10,3 +17,27 @@ export function noShame(opts) {
};
}
}

class NumericNoArrows extends UnconnectedNumeric {}
NumericNoArrows.propTypes = UnconnectedNumeric.propTypes;
NumericNoArrows.defaultProps = {
showArrows: false,
};

export const BoxGap = connectLayoutToPlot(
connectToContainer(NumericNoArrows, {
supplyPlotProps: (props, context) => {
// workaround the issue with nested layouts inside trace component.
// See
// https://github.com/plotly/react-plotly.js-editor/issues/58#issuecomment-345492794
const plotProps = unpackPlotProps(props, {
...context,
...getLayoutContext(context),
});

// Full value should multiply by percentage if number.

return plotProps;
},
})
);