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
10 changes: 5 additions & 5 deletions src/DefaultEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
import {DEFAULT_FONTS} from './constants';
import {localize, connectAxesToLayout, connectLayoutToPlot} from './lib';

import {BoxGap} from './shame';
import {BoxWidth, BoxPad} from './shame';

const LayoutPanel = connectLayoutToPlot(Panel);
const AxesFold = connectAxesToLayout(Fold);
Expand Down Expand Up @@ -148,10 +148,10 @@ class DefaultEditor extends Component {
</TraceMarkerSection>

<Section name={_('Size and Spacing')}>
<BoxGap label={_('Bar Width')} attr="bargap" />
<BoxGap label={_('Box Width')} attr="boxgap" />
<BoxGap label={_('Bar Padding')} attr="bargroupgap" />
<BoxGap label={_('Box Padding')} attr="boxgroupgap" />
<BoxWidth label={_('Bar Width')} attr="bargap" />
<BoxWidth label={_('Box Width')} attr="boxgap" />
<BoxPad label={_('Bar Padding')} attr="bargroupgap" />
<BoxPad label={_('Box Padding')} attr="boxgroupgap" />
</Section>

<Section name={_('Lines')}>
Expand Down
72 changes: 59 additions & 13 deletions src/shame.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
* DELETE THIS FILE. EVERYTHING NEEDS TO FIND A HOME.
*/
import isNumeric from 'fast-isnumeric';
import {list} from 'plotly.js/src/plots/cartesian/axis_ids';
import {UnconnectedNumeric} from './components/fields/Numeric';
import {
Expand All @@ -22,22 +23,67 @@ class NumericNoArrows extends UnconnectedNumeric {}
NumericNoArrows.propTypes = UnconnectedNumeric.propTypes;
NumericNoArrows.defaultProps = {
showArrows: false,
postfix: '%',
};

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

export const BoxWidth = connectLayoutToPlot(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps FractionalWidth? Or LayoutWidthFraction? Violin is going to use this too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep works for me 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

something like 1bebf70 ?

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;
supplyPlotProps: supplyLayoutPlotProps,
modifyPlotProps: (props, context, plotProps) => {
const {fullValue, updatePlot} = plotProps;
plotProps.fullValue = () => {
let fv = fullValue();
if (isNumeric(fv)) {
fv = Math.round((1 - fv) * 100);
}
return fv;
};

plotProps.updatePlot = v => {
if (isNumeric(v)) {
updatePlot(1 - v / 100);
} else {
updatePlot(v);
}
};

plotProps.max = 100;
Copy link
Collaborator

Choose a reason for hiding this comment

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

right now all of these have min: 0, max: 1, so setting just max = 100 works. But I'd feel more comfortable about future-proofing if we at least explicitly supply min = 0, and maybe even better do the same (1-v) * 100 transform (which would actually switch min / max)

Copy link
Member Author

Choose a reason for hiding this comment

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

good point will do.

Copy link
Member Author

Choose a reason for hiding this comment

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

},
})
);

export const BoxPad = connectLayoutToPlot(
connectToContainer(NumericNoArrows, {
supplyPlotProps: supplyLayoutPlotProps,
modifyPlotProps: (props, context, plotProps) => {
const {fullValue, updatePlot} = plotProps;
plotProps.fullValue = () => {
let fv = fullValue();
if (isNumeric(fv)) {
fv = fv * 100;
}
return fv;
};

plotProps.updatePlot = v => {
if (isNumeric(v)) {
updatePlot(v / 100);
} else {
updatePlot(v);
}
};

plotProps.max = 100;
},
})
);