6pac / SlickGrid

A lightning fast JavaScript grid/spreadsheet
https://stackblitz.com/github/6pac/SlickGrid/tree/master/vite-demo
MIT License
1.85k stars 424 forks source link

DataView getFilteredItems seems to be not respecting sorting #1065

Closed legezam closed 2 months ago

legezam commented 2 months ago

Describe the bug

After calling sort and setFilter on DataView, calling getFilteredItems returns the items not reflecting the sort order.

Reproduction

I can create a simple one if necessary but this is somewhat apparent even in the codebase of SlickGrid: the method sort in DataView doesn't update the filteredItems collection, only the items collection.

Which Framework are you using?

Other

Environment Info

System:
    OS: Linux 6.10 Fedora Linux 40 (Workstation Edition)
    CPU: (22) x64 Intel(R) Core(TM) Ultra 9 185H
    Memory: 43.73 GB / 62.23 GB
    Container: Yes
    Shell: 5.9 - /usr/bin/zsh
  Binaries:
    Node: 20.12.2 - /usr/bin/node
    npm: 10.5.0 - /usr/bin/npm

Validations

ghiscoding commented 2 months ago

I think that is normal behavior and should be handled on userland because the filtered data is a separate array and sorting both arrays would slow down the process which is why it's better handled on userland. Or maybe the only acceptable solution would be to add an extra argument that would be optional to also sort the filteredItems array as well (would default to false because of perf consideration that I mentioned above).

https://github.com/6pac/SlickGrid/blob/54bb8d5d9a18689d671e5b43b454ae49f3c2d972/src/slick.dataview.ts#L335-L349

However I took a quick look at the code and I thought that calling dv.refresh() might help because it's calling the internal .recalc() which is then calling

https://github.com/6pac/SlickGrid/blob/54bb8d5d9a18689d671e5b43b454ae49f3c2d972/src/slick.dataview.ts#L1335-L1345

which is then calling .getFilteredAndPagedItems()

https://github.com/6pac/SlickGrid/blob/54bb8d5d9a18689d671e5b43b454ae49f3c2d972/src/slick.dataview.ts#L1308-L1316

and finally inside that protected method, it seems to be where the filteredItems array is being assigned, which is why I'm saying that calling dv.refresh() might work but I haven't tested it.

https://github.com/6pac/SlickGrid/blob/54bb8d5d9a18689d671e5b43b454ae49f3c2d972/src/slick.dataview.ts#L1223-L1240

..but then I realized that .refresh() is actually called directly by the dv.sort() on the last line, so it's a bit surprising that it's not sorting the filtered items, perhaps it only reassigns when any of the "hints" are enabled (it might be that since these hints seems to be only useful for filtering)?

legezam commented 2 months ago

@ghiscoding , thanks for the detailed analysis, could you give me directions to setting those hints you mentioned here, please:

when any of the "hints" are enabled (it might be since that seems to be only for filtering)?

ghiscoding commented 2 months ago

I've never used them myself because I think it requires inlineFilters to be enabled but I guess it's mostly meant for filtering like I said earlier (but again I don't really know or understand how it works)

Example https://6pac.github.io/SlickGrid/examples/example-optimizing-dataview.html

https://github.com/6pac/SlickGrid/blob/54bb8d5d9a18689d671e5b43b454ae49f3c2d972/examples/example-optimizing-dataview.html#L156-L171

and from my previous post, you can see them being used as a condition in the SlickDataView starting on line 1234

ghiscoding commented 2 months ago

@legezam Just adding console log of dataView.getFilteredItems() into the Example 4 - ESM, I just can't replicate your issue since I seem to see the same order in the UI and in the logs as can be seen in the animated gif below. The console logs that I added where in the onSort and onPagingInfoChanged events

EfaO9iWIcq

legezam commented 2 months ago

@ghiscoding , thanks for your help, with your help, i realized i don't have a slickgrid problem. i have a react problem!

ghiscoding commented 2 months ago

in that case you could also take a look at my other repo Slickgrid-React