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

Sorting Causes Data To Disappear #70

Open
jmjf opened this issue Nov 16, 2015 · 1 comment
Open

Sorting Causes Data To Disappear #70

jmjf opened this issue Nov 16, 2015 · 1 comment

Comments

@jmjf
Copy link
Contributor

jmjf commented Nov 16, 2015

NOTE: I'm running on the latest (beta-1) skeleton with the skeleton-0.20.0 modifications discussed in Issue #67 and PR #69. I don't think this is related to any of that because this looks like a fairly straightforward issue.

If pageable == false, and we aren't using server paging, sorting, or filtering, sorting causes data to disappear.

In handleResult(), this.cache is loaded only if this.pageable == false.

if(this.pageable && !this.serverPaging && !this.serverSorting && !this.serverFiltering) {
  // Cache the data
  this.cache = result.data;
  this.filterSortPage(this.cache);
} else {
  this.data = result.data;
  this.filterSortPage(this.data);
}

In refresh(), if we aren't using server paging, sorting, or filtering (this.initialLoad is always true, see getData() for the only place it's set), we sort this.cache. So a non-pageable that isn't using server paging, sorting, or filtering sorts nothing. (Actually this.cache seems to be a ModifyArrayObserver.)

if(this.serverPaging || this.serverSorting || this.serverFiltering || !this.initialLoad)
  this.getData();
else
  this.filterSortPage(this.cache);

Possible solutions:

  • in handleResult(): this.data = this.cache = result.data OR
  • in refresh(): both cases set this.data and use this.filterSortPage(this.data) OR
  • in handleResult(): replace this.pageable with this.initialLoad (don't know if this is a legitimate solution, haven't worked through the implications) OR
  • in an appropriate place (unknown): set this.initialLoad = false (not sure where or how many wheres this might be involved) OR
  • some option I haven't thought of

The first two options seem to be the easiest. The first option seems to be very straightforward and almost certain to work in most (maybe even all) scenarios.

If you have a preference, let me know and I can set up a PR.

@charlespockert
Copy link
Owner

@jmjf thanks for all the interest and yes the first seems like to most straightforward.

The plan is to do a fair bit of refactoring once the Aurelia beta has bedded down a bit - I'd like to make this a half decent grid solution and try to get a decent feature set implemented.

Some of the internals of the templating engine around ViewSlot and the like have changed so there will likely be some hefty redesigning (I'm flirting with a pluggable model)

Feel free to throw PRs my way if it helps you but bear in mind that I'll probably re-design a chunk of this (I'm also thinking about moving it all to TypeScript)

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