search

trading-peter / chart-elements

Chart.js as Polymer Elements
https://robdodson.github.io/chart-elements
267 stars 70 forks source link

Charts are not responsive #74

Closed frasun closed 6 years ago

frasun commented 6 years ago

I was unable to make the charts responsive out of the box as the Chart.js documentation suggests. After investigating the code I found out that there is only update function used and it doesn't handle the scenario where only the dimensions of the chart have changed, not data.

Currently I am using the extension of this component in order to have it working properly. I am talking precisely about this chunk of code:

context-behavior.html

...
if (this.chart) {
  this.chart.stop();
  this.mixin(this.chart.data, this.data);
  this.chart.update();
  this.chart.resize(); * insert this *
} else {
...

Am I missing something here or is there a bug in the implementation?

robdodson commented 6 years ago

It's probably a bug. But I don't know how actively we're maintaining this project so we're open to any pull requests to try to fix it.

trading-peter commented 6 years ago

I'll try to look at this soon. Maybe tomorrow, but PR are of course very welcome :)

trading-peter commented 6 years ago

From my pov your fix is what was missing, @frasun. I merged an pr. @robdodson Can you tag v4.0.1 ? Seems like I'm not allowed to.

robdodson commented 6 years ago

Thank you @pkaske! I pushed a new tag (it looked like it was at 3.x so I revved it to 4.x)

What error do you get when you try to do a tag?

trading-peter commented 6 years ago

Solved it. Apparently the upstream repo has to be connected with https instead of ssh... Makes sense... No I can push tags.

I think I know were the version confusion comes from. Some time ago I ported the element to Polymer 2.0 (class based syntax). You will find it in the polymer-2.0 branch. I merged the change with that branch. I guess we need to make another pr for the master branch (aka Polymer 1.x). Anyways I think the tag is currently on the version pointing to commit on master instead of polymer-2.0.

robdodson commented 6 years ago

oh yeah. We could do a PR for master and put a 4.0.1 tag. My hunch is it's not a major breaking change for folks if we do that.

Then you could PR your 2.0 version to master and we could merge that and tag it at 5.0.

4.0.1 would be end of the line for Polymer 1.x