-
Notifications
You must be signed in to change notification settings - Fork 35
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
added overview chart initialisation #301
Conversation
…ew-chart Conflicts: client/charts/charts.js
Tested this one again. And it turns out that the code refactoring that we did (dividing code for rendering charts to separate functions like @brylie Any thoughts at his point? I think we can leave it like this or return to one-function view to have this functionality present. |
@apinf/developers This PR is ready to be merged. |
@@ -136,19 +136,36 @@ Template.chartsLayout.created = function () { | |||
|
|||
var chart = dc.lineChart("#line-chart"); | |||
var countryChart = dc.barChart("#bar-chart"); | |||
var overview = dc.barChart("#overview-chart"); | |||
|
|||
chart | |||
.width(1140) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if you omit the chart width? Does the chart become responsive with regards to the parent container width?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brylie Fixed this one. Chart's width is dynamic and fits the parent container on load. Still not responsive though.
@brylie When I remove |
added overview chart initialisation
@apinf/developers Here is the hot fix regarding overview chart. Chart initialisation function was somehow removed from develop branch after merging. Here is the fix, tested it and generally it works but area selection does not force chart refresh now, still zooming works.