Skip to content

DM-5760 Charts need to be expandable#58

Merged
tgoldina merged 2 commits intodevfrom
DM-5760_plotexpand
Apr 15, 2016
Merged

DM-5760 Charts need to be expandable#58
tgoldina merged 2 commits intodevfrom
DM-5760_plotexpand

Conversation

@tgoldina
Copy link
Copy Markdown
Contributor

Support expanded view for charts (histogram and plot)
(expand button in the right upper corner of the chart area)

}

renderToolbar() {
const {expandable, expandedMode} = this.props;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMO and without full knowledge of this component, I think breaking the many render functions into private functional components will make the main/public component looks cleaner. Also from a performance perspective, it may not have to re-render.
i.e.
function Toolbar({expandable, expandedMode}) {...}
then use in place of renderToolbar

@loitly
Copy link
Copy Markdown
Contributor

loitly commented Apr 14, 2016

In Safari, xy-options panel is acting strangely. Maybe it's the suggestion box growing too tall pushing everything else off the 'hidden' container. This is problematic. I think you should fix it before merging. If you can't duplicate, call me and we can try to make it happen.

Also, I noticed here: visualize/ChartsTableViewPanel.jsx:511, where autoflow: 'auto' is causing the scrollbar to flash(show/hide) in expanded mode when resizing the whole browser. Flashing probably means the component had to render twice.

@tgoldina
Copy link
Copy Markdown
Contributor Author

I have fixed suggest box display and expanded view in Safari. I can not change visualize/ChartsTableViewPanel.jsx:511 autoflow: 'auto' to 'hidden', because it will prevent scrolling down the plot when stretch is 'fill'.

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.

2 participants