Skip to content
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

Animation with reactivity #34

Closed
hotrush opened this issue Feb 14, 2017 · 14 comments
Closed

Animation with reactivity #34

hotrush opened this issue Feb 14, 2017 · 14 comments

Comments

@hotrush
Copy link

hotrush commented Feb 14, 2017

If chart has empty initial labels and data and they are updated dynamically - appear animation not working. Do you know how i can fix this?

Environment

  • OS: linux mint
  • NPM Version: 7+
@hotrush
Copy link
Author

hotrush commented Feb 14, 2017

Solved for me with adding v-if="chartData.labels.length > 0" to the chart element, but i think low priority bug still there)

@apertureless
Copy link
Owner

So the steps to reproduce would be,
pass in a dataset that has data but no labels and then after some time, add labels?

@hotrush
Copy link
Author

hotrush commented Feb 14, 2017

No, no data and no labels

<template>
    <div class="container">
        <div class="row">
            <div class="col-md-12">
                <prices-chart :chartData="pricesChart" :options="options" v-if="pricesChart.labels.length > 0"></prices-chart>
            </div>
        </div>
    </div>
</template>

<script>

    export default{
        data() {
            return {
                pricesChart: {
                    labels: [],
                    datasets: [
                        {
                            type: 'line',
                            label: 'Min',
                            data: []
                        },
                        {
                            type: 'line',
                            label: 'Max',
                            data: []
                        },
                        {
                            type: 'line',
                            label: 'Avg',
                            data: []
                        }
                    ]
                }
                options: {
                    responsive: true,
                    maintainAspectRatio: false
                }
            }

        },
        mounted() {
            this.loadData();
        },
        methods: {
            loadData() {
                this.$http.post('/api/database, {})
                    .then(function(response) {

                        var pricesChartData = _.merge({}, this.pricesChart);
                        pricesChartData.labels =  _.keys(response.body.prices_chart);
                        pricesChartData.datasets[0].data = _.map(response.body.prices_chart, 'min_price_per_day');
                        pricesChartData.datasets[1].data = _.map(response.body.prices_chart, 'max_price_per_day');
                        pricesChartData.datasets[2].data = _.map(response.body.prices_chart, 'avg_price_per_day');
                        this.pricesChart = pricesChartData;

                    }, function(error) {
                        // @todo log error
                        console.log(error);
                    });
            }
        }
    }
</script>

With v-if="pricesChart.labels.length > 0" works ok, without works, but no appear animation

@apertureless
Copy link
Owner

Ok thanks. I will check that.

@apertureless
Copy link
Owner

Are you using the reactivePropMixin ?

@hotrush
Copy link
Author

hotrush commented Feb 14, 2017

yes

<script>
    import Chart from 'vue-chartjs';
    export default Chart.Bar.extend({
        mixins: [Chart.mixins.reactiveProp],
        props: ['chartData', 'options'],
        mounted () {
            this.renderChart(this.chartData, this.options)
        }
    });
</script>

@hotrush
Copy link
Author

hotrush commented Feb 14, 2017

I am also noticed an issue that charts and x-axis do not match
5684136192

@apertureless
Copy link
Owner

apertureless commented Feb 14, 2017

Hm I can't reproduce it. So I think it could be caused by the API call, because of the async data.
And how the mixin and chartjs works.

Right now, the mixin is pretty simple, it grabs all labels of the old data and new data. In your case Min, Max, Avg. And compares both. With the assumtion, that if a new label appears, it means a new dataset appears. So if they mismatch it simply rerenders the whole chart instance.

If they match, it assumes that the data inside the datasets changes, and sets them on the chartjs instance and call update().

However I found another bug #35 where the chart instance does not get destroyed before a new render. Which causes multiple instances. I will investigate further. 🔍

The screenshot however looks like your data[] count missmatches your labels count. You should check the data.

@apertureless
Copy link
Owner

@hotrush can you update vue-chartjs to 2.3.5 and check if it still buggy?

@hotrush
Copy link
Author

hotrush commented Feb 14, 2017

2.3.5 is still buggy. i made a test repo to demonstrate this problem https://hotrush.github.io/vue-charts-bug/

as you can see here https://hotrush.github.io/vue-charts-bug/data.json, data is clean and there is no missmatches in labels or values.

@hotrush
Copy link
Author

hotrush commented Feb 14, 2017

also added a second chart with v-if where animation works

@apertureless
Copy link
Owner

apertureless commented Feb 15, 2017

Okay, I think I found the issue with the initial animation.
Well the problem is, with your predefinition of the data object.

What the mixin is doing

The recativity mixin checks two things:

  1. It checks the labels prodived for the datasets.
  2. It checks the dataset length.

And based on that information it either

  • Call update()or
  • Call renderChart()

If you datasets arrive it calls renderChart and if data changes it calls update.

The check

So the assumtion is, if a new dataset arrive either a new label is set or the dataset array is bigger / smaller. Then we need a render. If only the data changes, an update() is enough.

The Problem

The problem is, that if you predefine your labels for the datasets and the datasets, the mixin will only update(). Now you set your data with the api call.

And we're getting a race condition there. Because the watcher gets triggered by any changes of the chartData prop. However setting your data this way, a race condition appears, because the watcher get triggered but meanwhile your newData is set.

If you output the oldData and newData in the watcher mixin, you'll see that they are equal. Because the oldData is equal with the newData no animation appears.

The v-if check prevents this. Because it waits for the data to be set properly.

However I am not entirely sure, if its a bug. It's a bit of a weird edge case, because you set your datasets and labels before.

Right now I can't think of a clean way to change the watcher mixins, to work with this edgecase.

Workaround

Well, one workaround would be the v-if statement.

Another would be to set the labels in your api request. This way, the mixin would see the changed datasets and do a render.

....
pricesChartData.datasets[0].data = _.map(response.data.prices_chart, 'min_price_per_day')
pricesChartData.datasets[0].label = 'Max'
pricesChartData.datasets[1].data = _.map(response.data.prices_chart, 'max_price_per_day')
pricesChartData.datasets[1].label = 'Min'
pricesChartData.datasets[2].data = _.map(response.data.prices_chart, 'avg_price_per_day')
...

Alternative

Another alternative would be to implement an small event system and call update and render yourself. I think, chart.js is not really made for async data flow, tho.

@apertureless
Copy link
Owner

For the issue with the x-axis, I could not really reproduce it. Do you have any additional custom settings?

Can you try to set the label in your api call, while mapping your data? And see if it changes?

You could also enable grid lines on the axis to see, if maybe only the labels got an offset.

 options: {
          responsive: true,
          maintainAspectRatio: true,
          scales: {
            xAxes: [{
              stacked: true,
              ticks: {
                beginAtZero: false
              },
              gridLines: {
                display: true
              }
            }],
            yAxes: [{
              stacked: true,
              ticks: {
                beginAtZero: false
              },
              gridLines: {
                display: true
              }
            }]
          }
        }

@hotrush
Copy link
Author

hotrush commented Feb 16, 2017

So, finally for axes: according this issue #29 Chart.Bar.extend() was used and when switched to Chart.Line.extend() problem disappeared, no more axes missmatching.

But for animation i am still using v-if, nothing other helps in my case.

Thanks a lot for your help! Can close this issue i think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants