SitePen / dgrid

A lightweight, mobile-ready, data-driven, modular grid widget designed for use with dstore
http://dgrid.io/
Other
628 stars 295 forks source link

Tree: Column sorting breaks OnDemandGrid paging #1341

Open AndrewT-TitaniumSolutions opened 7 years ago

AndrewT-TitaniumSolutions commented 7 years ago

If you have at least three top level entries in your tree and the first and last entries have more than minRowsPerPage children but one of the other entries has less than minRowsPerPage children then when a column is sorted not all entries are displayed. If all entries have either more or less than minRowsPerPage children then things work as expected.

We are using dgrid 0.3.12 so I initially though this was related to #701 or #714 as we are backing the grid with an Observable store, but upgrading to 0.3.17 has not fixed the issue and there are no errors or warnings in the console.

What appears to be happening is that the grid queries the store for minRowsPerPage items for each parent element and once those are drawn the _processScroll function tries to fill in the remaining values from the bottom of the grid upwards. After the smaller parent element has had it's children rendered the remaining items have preload.count set to 0 which is preventing more items being rendered.

I have managed to reproduce the issue by altering the tree test page, javascript inline below, though it has slightly different behavior to what we are seeing in production (the rendering is broken from the initial load rather than only after sorting a column).

require(["dojo/_base/declare", "dojo/_base/lang", "dgrid/tree", "dgrid/OnDemandGrid",  "dojo/store/Memory", "dojo/store/Observable"], 
function(declare, lang, Tree, OnDemandGrid, Memory,Observable){

    function prepareExampleData(){
    var data = [{ id: 'AF', name:'FAfrica'},
            { id: 'EG', name:'Egypt', type:'country', parent: 'AF' },
            { id: 'AS', name:'ZAsia', type:'continent', population: '3.2 billion'},
            { id: 'CN', name:'China', type:'country', parent: 'AS' },
                { id: 'IN', name:'India', type:'country', parent: 'AS' },
                { id: 'RU', name:'Russia', type:'country', parent: 'AS' },
                { id: 'MN', name:'Mongolia', type:'country', parent: 'AS' },
            { id: 'EU', name:'Europe', type:'continent', population: '774 million' },
            { id: 'DE', name:'Germany', type:'country', parent: 'EU' },
            { id: 'FR', name:'France', type:'country', parent: 'EU' },
            { id: 'ES', name:'Spain', type:'country', parent: 'EU' },
            { id: 'IT', name:'Italy', type:'country', parent: 'EU' }];

    return data;
  }

  var TreeGridCls = declare([OnDemandGrid]);

  var data = prepareExampleData();
  var store = new Observable(new Memory({
  data: lang.clone(data),

    getChildren: function (parent, options) {
      return this.query(
                lang.mixin({}, options && options.originalQuery || null,
                    { parent: parent.id }),
                options);
    },

    mayHaveChildren: function (parent) {
      return parent.parent === undefined;
    },
    query: function (query, options){
            query = query || {};
            options = options || {};

            if (!query.parent && !options.deep) {
                // Default to a single-level query for root items (no parent)
                query.parent = undefined;
            }
            return this.queryEngine(query, options)(this.data);
        }
  }));

  var treeGrid = new TreeGridCls({
    columns: [
             Tree({label: "Name", field:"name",
                       shouldExpand: function (row, level, previouslyExpanded) {
                            return true;
                        }}),
                        {label:"Type", field:"type"},
                        {label:"Population", field:"population"},
                        {label:"Timezone", field:"timezone"}
                        ],
    sort: "name",
    minRowsPerPage: 2,
    store: store
  }, "treeGrid");
  treeGrid.startup();
});
dylans commented 7 years ago

I would guess some combination of 3cd8120 and 3cd8120 fix this, but not all the way back to 0.3.x unfortunately. It would be worth updating to master and seeing if that fixes the issue for you.

AndrewT-TitaniumSolutions commented 7 years ago

Unfortunately we have some heavily customized object stores we use with dgrid so migrating to dstore to use a newer dgrid is a non-trivial exercise. I have worked around the issue (by forcing a grid refresh to collapse the tree) which is sufficient for our purposes at the moment.

I did give the current master a try and there is still an issue. Adding minRowsPerPage: 2, into one of the grid definitions on the included tree.html test page still triggers the issue, though the placeholder rows will fill in once you expand enough nodes.

dylans commented 7 years ago

Sure, though dstore does provide an adapter that makes it simply a wrapper around a dojo/store which might help. Regardless, understood. And we will look into the use case you've provided against master as well...