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

SingleQuery and grid.get('total') #1271

Closed
AndrewWSHenry opened this issue Apr 6, 2016 · 4 comments
Closed

SingleQuery and grid.get('total') #1271

AndrewWSHenry opened this issue Apr 6, 2016 · 4 comments
Milestone

Comments

@AndrewWSHenry
Copy link

This is more of an issue on a like to have/request.

First, thanks for the SingleQuery tutorial at the dgrid.io site. It's a useful mixin for the in-memory store I was working with. Would it be possible to add this mixin to dgrid itself? It's usefulness suggests that it is worthy of inclusion.

And the issue I had was that I needed to display the total number of rows after setting a filtered collection to the grid. I came across the issue that pointed me to using grid.get('total'). After discovering that get('total') always returned zero with the SingleQuery mixin, I looked at the code in _StoreMixin and realized that the _total property would never be set.

I'd like to suggest updating the SingleQuery tutorial to include setting total in the renderArray function, so that grid.get('total') functions properly.

renderArray: function () {

    var rows = this.inherited(arguments);

    // Clear _lastCollection which is ordinarily only used for
    // store-less grids

    this._lastCollection = null;

    // Set the total number of rows. _StoreMixin has a property called _total.
    // SingleQuery extends _StoreMixin so it can set _total directly.
    // Can then use get('total') that is made available by the _getTotal of _StoreMixin.

    this._total = rows.length;                                     

    return rows;
}
@kfranqueiro kfranqueiro added this to the 1.0.1 milestone Apr 25, 2016
@kfranqueiro
Copy link
Member

kfranqueiro commented Apr 25, 2016

Thanks for pointing out the omission of setting _total. I've committed updates to the 0.4 and 1.0 versions of the tutorials for that for the next time we update the site.

As for why it's not in dgrid as a first-class mixin, the main reason was to avoid the footgun of people finding it in plain sight, then using it for inappropriately large data sets (or with stores that ordinarily would be used to request a small range at a time from a server), and then wrongly assuming that dgrid is to blame for the ensuing performance nightmare.

However, at this point it sort of is in the repository anyway (tucked under the folder for the laboratory demo which makes use of it), which kind of seems silly. I'm planning to have another discussion with the team as to whether it's time to just promote it after all (maybe with a warning if it detects a large data set), at least for 1.x.

@kfranqueiro
Copy link
Member

kfranqueiro commented Apr 26, 2016

We've decided to go ahead and add SingleQuery as a top-level or extension module in dgrid for the next release, which we're planning to tag as 1.1.0 as a result. (There are no breaking changes.) I'll work on moving it and adding tests/docs sometime this week.

@AndrewWSHenry
Copy link
Author

Cool. Thanks Ken.

@kfranqueiro
Copy link
Member

SingleQuery will be available as dgrid/extensions/SingleQuery in 1.1.0: fe8092c

Adding this (along with support for things like noDataMessage and loadingMessage) also enabled us to add some tests and refactor some code, which led to finding some other stuff going on as well, relevant to #1275 and #1277, which will both be fixed in that release.

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

No branches or pull requests

2 participants