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.

Sign up for GitHub

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

Jump to bottom

OnDemandGrid rows don't get displayed when inserted at end #363

Closed
jdawsongit opened this issue Dec 7, 2012 · 7 comments
Closed

OnDemandGrid rows don't get displayed when inserted at end #363

jdawsongit opened this issue Dec 7, 2012 · 7 comments

Comments

@jdawsongit
Copy link

OnDemandGrid has a problem when rows are inserted into the data store and the sort order places them at the end. If the end of the grid visible when this happens, and there are enough rows for pagination to be taking place, the row doesn't get rendered.

I made a standalone test file that you can copy to the dgrid/test directory and run like the other tests there.

<!DOCTYPE html>
<html>
    <head>
        <title>Bug Inserting To End Of OnDemandGrid</title>
        <meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
        <meta name="viewport" content="width=570" />
        <style type="text/css">
            @import "../../dojo/resources/dojo.css";
            @import "../css/dgrid.css";
            @import "../css/skins/claro.css";

            body {
                margin: 5px;
            }

            #grid {
                height: 400px;
                width: 600px;
            }
        </style>
        <script type="text/javascript" src="../../dojo/dojo.js" 
            data-dojo-config="async: true"></script>
        <script type="text/javascript">
            require([
                "dojo/on",
                "dojo/when",
                "dojo/store/Memory",
                "dojo/store/Observable",
                "dgrid/OnDemandGrid"
            ], function (on, when, Memory, Observable, OnDemandGrid) {
                var lastId = 0;
                function nextId() {
                    return ++lastId;
                }
                function makeItem() {
                    var id = nextId();
                    return {
                        id: id,
                        msg: "Message " + id
                    };
                }
                var data = [];
                for (var i = 0; i < 40; i++) {
                    data.push(makeItem());
                }
                var memory = new Memory({ data: data });
                var store = new Observable(memory);
                var sort = [ { attribute: 'id', descending: false } ];
                var grid = new OnDemandGrid({
                    store: store,
                    columns: [
                        { label: 'id', field: 'id' },
                        { label: 'msg', field: 'msg' }
                    ],
                    sort: sort
                }, 'grid');
                on(document.getElementById('addRowButton'), 'click', function() {
                    store.put(makeItem());
                });
                on(document.getElementById('removeRowButton'), 'click', function() {
                    when(store.query(function () { return true; }), function (results) {
                    if (results.length) {
                        store.remove(results[0].id);
                    }
                    });
                });
            });
        </script>
    </head>
    <body class="claro">
        <h1>How to reproduce the bug:</h1>
        <ol>
            <li>Scroll to the bottom</li>
            <li>Click "Add Row"</li>
        </ol>
        <p>No row appears.</p>
        <p>If you re-sort, the row becomes visible.</p>
        <h1>Test apparatus</h1>
        <form>
            <div style="padding: 5px">
                <input type="button" id="addRowButton" value="Add Row" />
                <input type="button" id="removeRowButton" value="Remove Row" />
            </div>
        </form>
        <div id="grid"></div>
    </body>
</html>
@jdawsongit
Copy link
Author

The problem seems to be a disagreement between dojo/store/Observable and dgrid/OnDemandList regarding how to tell if we're at the end of the data store.

In Observable.js, the query updater does this:

var atEnd = resultsArray.length != options.count;

This looks reasonable at first glance. It makes sense if the expectation is that we queried the store to get 25 rows but got back only 15 rows, so options.count might be smaller than resultsArray.length.

However, over in OnDemandList.js, in _processScroll, one finds this line of code:

options.count = Math.min(count + queryRowsOverlap, grid.maxRowsPerPage);

That sets options.count to the actual number of rows in the paginated chunk. Thus, atEnd in Observable will always end up false. This ends up making the row not get rendered.

It's not clear to me which party needs fixing. Perhaps both. I think dojo/store/JsonRest is also affected by this issue.

@jdawsongit
Copy link
Author

I made a jsfiddle of the above code that demonstrates the bug. To get the dgrid libraries, I copied a Javascript include file used by ejellard in #358. I believe it's using Dgrid 0.3.1. The problem is reproducible on the master, though.

http://jsfiddle.net/4U9tC/

@ghost
Copy link

ghost commented Aug 7, 2013

Is there any update on this issue? It's particularily nasty for our grid which can hold thousands of records and our default desired behaviour is to add records at the end.

@mhils
Copy link

mhils commented Aug 7, 2013

IMO dgird lacks a total property that adjusts everything accordingly (just as SlickGrid does it).

@ghost
Copy link

ghost commented Aug 7, 2013

I agree, is there any way to escalate this issue? This is effectively making our infinite scrolling grids read-only.

@jdawsongit jdawsongit reopened this Aug 9, 2013
@kfranqueiro
Copy link
Member

This has been resolved by recent work dealing with observable query results in 105e38b. There is still an issue where if you are already scrolled to the bottom and an item is added, the grid doesn't re-scroll to the bottom, but I believe that is more relevant to #341, so I am going to close this.

@mhils
Copy link

mhils commented Jan 11, 2014

@kfranqueiro and @kriszyp, super happy to see this. I obviously haven't tested it properly in the last three minutes, but I can't wait to do so tomorrow.

Thank you both! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kfranqueiro @jdawsongit @mhils