Skip to content

Conversation

dmt0
Copy link
Contributor

@dmt0 dmt0 commented Aug 14, 2018

closes #688
closes #689

@nicolaskruchten
Copy link
Contributor

Behaviour looks good! checking code now.

Two mods:

  1. The TraceRequired should say "panel" instead of "tab"
  2. We should reuse this linking ability in the AxesCreator thing so people can click on Subplots to get to the subplots tab

heading: _("Looks like there aren't any traces defined yet."),
message: _("Go to the 'Create' tab to define traces."),
};
if (!this.props.visible || !showPanel) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice refactoring! Please do one more pass here though... you can get rid of showPanel I think, and inline the "Looks like there aren't any traces defined yet" heading and so on :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, that was a bit uncooked

TraceRequiredPanel.propTypes = {
children: PropTypes.node,
visible: PropTypes.bool,
extraConditions: PropTypes.array,
Copy link
Contributor

Choose a reason for hiding this comment

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

ah nice.

{content ? content : null}
</TraceRequiredPanel>
);
return <TraceRequiredPanel>{content ? content : null}</TraceRequiredPanel>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm ok once we tackle #562 this can just be a Panel :)

} from '../components';

class StyleAxesPanel extends Component {
constructor(props, context) {
Copy link
Contributor

Choose a reason for hiding this comment

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

very nice

return this.context.fullData.some(
d =>
(d.marker && d.marker.showscale !== undefined) || // eslint-disable-line no-undefined
d.showscale !== undefined // eslint-disable-line no-undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good. I know the panel still appears even when all traces are in constant-color mode, but that's ok. at least this panel never appears when there's nothing you can do in it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

once you saw that panel appear, you don't wanna loose it again right? :)

@nicolaskruchten
Copy link
Contributor

I'm loving this setPanel thing. we can use it judiciously in a couple of places to help the user navigate, like "go to the Color Bar panel to style it" or "go to the legend panel" or whatever

@nicolaskruchten
Copy link
Contributor

💃 nice one!

@dmt0 dmt0 merged commit 4166fb4 into master Aug 14, 2018
@dmt0 dmt0 deleted the empty-panels branch August 14, 2018 22:57
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.

Sliders/Menus panel shouldn't show when none present TraceRequiredPanel should have link to create
2 participants