Skip to content

Conversation

@VeraZab
Copy link
Contributor

@VeraZab VeraZab commented Mar 23, 2018

fixes: #403

@VeraZab VeraZab force-pushed the plotly-containers branch from 94fda9b to 3b47567 Compare March 27, 2018 02:43
@VeraZab VeraZab changed the title Rename Section -> PlotlySection || Remove SectionHeader Rename Panel/Fold/Section -> PlotlyPanel/Fold/Section/ Mar 27, 2018
@VeraZab VeraZab force-pushed the plotly-containers branch 4 times, most recently from 0a25da3 to 93d9bac Compare March 28, 2018 17:02
className: PropTypes.string,
folded: PropTypes.bool,
toggleFold: PropTypes.func,
hideHeader: PropTypes.bool,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the isRequired here...
BUT, I am testing in my tests that those props are there for <Fold/>s
I don't know why those console.errors were occurring..

Panel.propTypes = {
PlotlyPanel.propTypes = {
children: PropTypes.node,
addAction: PropTypes.object,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left the addAction on <Panel/> maybe devs would like to add some non context related addActions..
Doesn't have to be disabled no?


SectionHeader.propTypes = {
name: PropTypes.string,
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't need this anymore.. also Ben didn't use it in his code :)

return (
<Field {...this.props}>
<div className="js-test-info">{this.props.children}</div>
</Field>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I add the class on Field, it gives me 2 js-test-info classes, one on the localized wrapper one on Field

@VeraZab VeraZab force-pushed the plotly-containers branch from 93d9bac to 473351a Compare March 28, 2018 17:13
@VeraZab
Copy link
Contributor Author

VeraZab commented Mar 28, 2018

@nicolaskruchten would you take a 👀

@VeraZab VeraZab force-pushed the plotly-containers branch from 4c4785f to 52109c5 Compare March 28, 2018 19:42
export default class PlotlyPanel extends Panel {}
export const Panel = localize(UnlocalizedPanel);

export default class PlotlyPanel extends UnlocalizedPanel {}
Copy link
Contributor

Choose a reason for hiding this comment

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

that's weird... the Panel and Section were not localized but the Fold was ???

@nicolaskruchten
Copy link
Contributor

💃 thanks for putting up with these requirements and changes :)

@VeraZab VeraZab merged commit e78c77a into master Mar 28, 2018
@VeraZab VeraZab deleted the plotly-containers branch March 28, 2018 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plotly figure connected containers should be prefixed with Plotly

3 participants