Skip to content

Revert the fit transforms revert (f1868693f889...) #542

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 1 commit into from
Jul 4, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
61 changes: 31 additions & 30 deletions scripts/translationKeys/combined-translation-keys.txt

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions scripts/translationKeys/translation-keys.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Aggregations
Aitoff // /default_panels/GraphCreatePanel.js:202
Albers USA // /default_panels/GraphCreatePanel.js:183
All // /default_panels/StyleAxesPanel.js:406
All Traces // /components/containers/TraceAccordion.js:75
All Traces // /components/containers/TraceAccordion.js:89
Ambient // /default_panels/StyleTracesPanel.js:349
Anchor // /default_panels/StyleAxesPanel.js:82
Anchor Point // /default_panels/StyleAxesPanel.js:455
Expand Down Expand Up @@ -232,7 +232,7 @@ Images
Include // /default_panels/StyleTracesPanel.js:76
Increasing // /default_panels/StyleTracesPanel.js:68
Increasing Trace Styles // /default_panels/StyleTracesPanel.js:368
Individual // /components/containers/TraceAccordion.js:76
Individual // /components/containers/TraceAccordion.js:90
Inside // /components/fields/derived.js:473
Intensity // /default_panels/GraphCreatePanel.js:143
Interpolate Gaps // /default_panels/StyleTracesPanel.js:308
Expand Down Expand Up @@ -443,7 +443,7 @@ Top
Top Center // /components/fields/derived.js:462
Top Left // /components/fields/derived.js:461
Top Right // /components/fields/derived.js:463
Trace // /components/containers/TraceAccordion.js:34
Trace // /components/containers/TraceAccordion.js:48
Trace Order // /default_panels/StyleLegendPanel.js:75
Traces // /DefaultEditor.js:30
Transform // /components/containers/TransformAccordionDev.js:84
Expand Down
18 changes: 17 additions & 1 deletion src/components/containers/PlotlyFold.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ export class Fold extends Component {
this.foldVisible = true;
}

getChildContext() {
return {
foldInfo: this.props.foldInfo ? this.props.foldInfo : null,
};
}

render() {
if (!this.foldVisible && !this.props.messageIfEmpty) {
return null;
Expand All @@ -21,6 +27,7 @@ export class Fold extends Component {
children,
className,
folded,
foldInfo,
toggleFold,
hideHeader,
icon: Icon,
Expand Down Expand Up @@ -56,7 +63,7 @@ export class Fold extends Component {
className="fold__top__delete js-fold__delete"
onClick={e => {
e.stopPropagation();
deleteContainer(e);
deleteContainer(foldInfo);
}}
>
<CloseIcon />
Expand Down Expand Up @@ -105,13 +112,22 @@ Fold.propTypes = {
children: PropTypes.node,
className: PropTypes.string,
folded: PropTypes.bool,
foldInfo: PropTypes.object,
toggleFold: PropTypes.func,
hideHeader: PropTypes.bool,
icon: PropTypes.oneOfType([PropTypes.node, PropTypes.func]),
messageIfEmpty: PropTypes.string,
name: PropTypes.string,
};

Fold.contextTypes = {
deleteContainer: PropTypes.func,
};

Fold.childContextTypes = {
foldInfo: PropTypes.object,
};

class PlotlyFold extends Fold {
constructor(props, context) {
super(props, context);
Expand Down
15 changes: 13 additions & 2 deletions src/components/containers/PlotlyPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ export class Panel extends Component {
this.toggleFold = this.toggleFold.bind(this);
}

getChildContext() {
return {
deleteContainer: this.props.deleteAction ? this.props.deleteAction : null,
};
}

componentDidCatch() {
this.setState({hasError: true});
}
Expand Down Expand Up @@ -124,10 +130,11 @@ export class Panel extends Component {
}

Panel.propTypes = {
children: PropTypes.node,
addAction: PropTypes.object,
showExpandCollapse: PropTypes.bool,
children: PropTypes.node,
deleteAction: PropTypes.func,
noPadding: PropTypes.bool,
showExpandCollapse: PropTypes.bool,
};

Panel.defaultProps = {
Expand All @@ -138,6 +145,10 @@ Panel.contextTypes = {
localize: PropTypes.func,
};

Panel.childContextTypes = {
deleteContainer: PropTypes.func,
};

class PlotlyPanel extends Panel {}

PlotlyPanel.plotly_editor_traits = {
Expand Down
27 changes: 21 additions & 6 deletions src/components/containers/TraceAccordion.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,25 @@ const TraceFold = connectTraceToPlot(PlotlyFold);
class TraceAccordion extends Component {
render() {
const {data = [], localize: _} = this.context;
const {canAdd, canGroup, children, messageIfEmptyFold} = this.props;
const {
canAdd,
canGroup,
children,
messageIfEmptyFold,
excludeFits,
} = this.props;

// we don't want to include analysis transforms when we're in the create panel
const filteredData = data.filter(t => {
if (excludeFits) {
return !(t.transforms && t.transforms.every(tr => tr.type === 'fit'));
}
return true;
});

const individualTraces =
data.length &&
data.map((d, i) => {
filteredData.length &&
filteredData.map((d, i) => {
return (
<TraceFold
key={i}
Expand Down Expand Up @@ -46,7 +60,7 @@ class TraceAccordion extends Component {
</PlotlyPanel>
);
}
const tracesByGroup = data.reduce((allTraces, nextTrace, index) => {
const tracesByGroup = filteredData.reduce((allTraces, nextTrace, index) => {
const traceType = plotlyTraceToCustomTrace(nextTrace);
if (!allTraces[traceType]) {
allTraces[traceType] = [];
Expand All @@ -67,7 +81,7 @@ class TraceAccordion extends Component {
);
});

if (canGroup && data.length > 1 && groupedTraces.length > 0) {
if (canGroup && filteredData.length > 1 && groupedTraces.length > 0) {
return (
<TraceRequiredPanel noPadding>
<Tabs>
Expand Down Expand Up @@ -102,9 +116,10 @@ TraceAccordion.contextTypes = {
};

TraceAccordion.propTypes = {
children: PropTypes.node,
canAdd: PropTypes.bool,
canGroup: PropTypes.bool,
children: PropTypes.node,
excludeFits: PropTypes.bool,
messageIfEmptyFold: PropTypes.string,
};

Expand Down
2 changes: 1 addition & 1 deletion src/default_panels/GraphCreatePanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {

const GraphCreatePanel = (props, {localize: _}) => {
return (
<TraceAccordion canAdd>
<TraceAccordion canAdd excludeFits>
<TextEditor label={_('Name')} attr="name" richTextOnly />
<TraceSelector label={_('Type')} attr="type" show />

Expand Down
23 changes: 23 additions & 0 deletions src/lib/connectTraceToPlot.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,30 @@ export default function connectTraceToPlot(WrappedComponent) {
let fullTrace = {};
for (let i = 0; i < fullData.length; i++) {
if (trace.uid === fullData[i]._fullInput._input.uid) {
/*
* Fit transforms are custom transforms in our custom plotly.js bundle,
* they are different from others as they create an extra trace in the
* data array. When plotly.js runs supplyTraceDefaults (before the
* transforms code executes) it stores the result in _fullInput,
* so that we have a reference to what the original, corrected input was.
* Then the transform code runs, our figure changes accordingly, but
* we're still able to use the original input as it's in _fullInput.
* This is the desired behaviour for our transforms usually,
* but it is not useful for fits, as the transform code adds some styles
* that are useful for the trace, so really for fits we'd like to read
* from _fullData, not _fullInput. Here we're setting _fullInput to
* _fullData as that is where the rest of our code expects to find its
* values.
*/
if (
trace.transforms &&
trace.transforms.every(t => t.type === 'fit')
) {
fullData[i]._fullInput = fullData[i];
}

fullTrace = fullData[i]._fullInput;

break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/styles/components/widgets/_numeric-input.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
.numeric-input__wrapper {
line-height: 20px;
max-width: 100%;
width: 100%;
flex: 1;
display: flex;
align-items: center;
color: var(--color-text-base);
Expand Down