fin-hypergrid / core

A canvas-based super high performant grid renderer API
MIT License
897 stars 144 forks source link

Sorting problem #28

Closed eduardotang closed 9 years ago

eduardotang commented 9 years ago

All data disappears after you go back to the non sort state, asc -> desc -> no sort (data gone)

while if you sort by another column before you go back to no sort on the same column, it is ok.

stevewirts commented 9 years ago

I can't reproduce this, is this the JSON behavior?

eduardotang commented 9 years ago

yes the JSON behavior and in the console i noticed Uncaught TypeError: Cannot set property '__i' of undefined

stevewirts commented 9 years ago

that means one of the json objects you've provided was replaced with undefined somehow. in order to get back to unsorted state I put __i index on the objects in order to have the native sort be stable

stevewirts commented 9 years ago

I'm going to close this for now as I can't seem to reproduce it. Let me know if it still is happening for you and we can dig in a little deeper.

eduardotang commented 9 years ago

i ran it through debugger, found that inside the toggleSort function this copying makes use of __si , but this is undefined

    if (sortStateIndex === 0) {
                var newData = new Array(this.data.length);
                for (i = 0; i < this.data.length; i++) {
                    var each = this.data[i];
                    newData[each.__si] = each;
                }
                this.data = newData;
    }

since i do setData with empty array and push object into it later on, so initDataIndexes has no effect.

stevewirts commented 9 years ago

are you inserting new objects into the collection after things are running? if so things need to be initialized.

On Mon, May 4, 2015 at 11:35 PM, eduardotang notifications@github.com wrote:

i ran it through debugger, found that inside the toggleSort function this copying makes use of __si , but this is undefined

     if (sortStateIndex === 0) {
            var newData = new Array(this.data.length);
            for (i = 0; i < this.data.length; i++) {
                var each = this.data[i];
                newData[each.__si] = each;
     }
            this.data = newData;
        }

— Reply to this email directly or view it on GitHub https://github.com/openfin/fin-hypergrid/issues/28#issuecomment-98929609 .

eduardotang commented 9 years ago

so everytime after i insert new objects , i have to call initDataIndexes?

stevewirts commented 9 years ago

yes, unless you don't care about stable sorting and being able to go back to the initial unsorted state

On Tue, May 5, 2015 at 12:44 PM, eduardotang notifications@github.com wrote:

so everytime after i insert new objects , i have to call initDataIndexes?

— Reply to this email directly or view it on GitHub https://github.com/openfin/fin-hypergrid/issues/28#issuecomment-99136159 .

stevewirts commented 9 years ago

I've just done a small refactoring, so if you change the underlying data call "dataChanged()" on the behavior object...

    setData: function(jsonData) {
        this.data = jsonData;
        this.dataChanged();
    },

    dataChanged: function() {
        this.initDataIndexes();
        this.initColumnIndexes();
        this.changed();
    },

let me know how things work out....

On Tue, May 5, 2015 at 12:44 PM, eduardotang notifications@github.com wrote:

so everytime after i insert new objects , i have to call initDataIndexes?

— Reply to this email directly or view it on GitHub https://github.com/openfin/fin-hypergrid/issues/28#issuecomment-99136159 .

eduardotang commented 9 years ago

now the data wont be gone, and it stays at the descending state. it's good enough. thx

stevewirts commented 9 years ago

I'll create an issue and implement a more robust solution, Hypergrid should do something reasonable after inserting new data.

On Wed, May 6, 2015 at 4:38 AM, eduardotang notifications@github.com wrote:

now the data wont be gone, and it stays at the descending state. it's good enough. thx

— Reply to this email directly or view it on GitHub https://github.com/openfin/fin-hypergrid/issues/28#issuecomment-99383444 .

eduardotang commented 9 years ago

your last commit move the call to initDataIndexes to setData, cause problem adding data to original array again

stevewirts commented 9 years ago

reverted, thanks :)

On Sun, May 17, 2015 at 6:55 AM, eduardotang notifications@github.com wrote:

your last commit move the call to initDataIndexes to setData, cause problem adding data to original array again

— Reply to this email directly or view it on GitHub https://github.com/openfin/fin-hypergrid/issues/28#issuecomment-102783172 .