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

OnDemandGrid sometimes removes updated rows #305

Closed
dmartin-gh opened this issue Oct 9, 2012 · 6 comments
Closed

OnDemandGrid sometimes removes updated rows #305

dmartin-gh opened this issue Oct 9, 2012 · 6 comments

Comments

@dmartin-gh
Copy link
Contributor

I encountered this issue while using an OnDemandGrid backed by a dojo.store.Observable/dojo.store.Memory combo. When an entry is updated and it needs to jump from one result set to another, it will sometimes get removed from the grid entirely.

I've uploaded a tar.gz archive containing a minimal example which reproduces the error. Load the page and inspect the main.js file. It contains more information about the bug and the exact steps to reproduce it.

http://tinyurl.com/8nyubby (observable_bug.tar.gz 32MB)

@justindoherty
Copy link
Contributor

I believe I've encountered this same issue 6 months ago where items were disappearing from my grid. I had opened two bugs with dojo regarding the issue.

http://trac.dojotoolkit.org/ticket/15227
http://trac.dojotoolkit.org/ticket/15228

@amuraco
Copy link
Contributor

amuraco commented Oct 19, 2012

Were you by any chance using the Pagination extension, I saw similar behavior with Observable(JsonRest) and pagination, and managed to fix it in the Paginator.

@dmartin-gh
Copy link
Contributor Author

I don't have the Paginator mixin enabled in my web app nor the reduced reproduction case that I linked above. With the way our web application is intended to be designed, pagination would not fit our feature requests. I am using a JsonRest store instead of a Memory store in our web app, but the issue is reproducible with both.

Neither of the two tickets above directly address the issue here. What's happening is an item is updated in the second result set and needs to be resorted such that it ends up in the first result set. The resulting observer calls look something like:

// result set 1
observer(obj, -1, 0)
// result set 2
observer(obj, 4, -1)

In this case it should have been moved from the 4th position in the second result set to the front of the first result set. During the first observer callback this happens just fine; if you pause the page at this point the row has been moved in the DOM and everything looks good. Next the second observer is called and at this point dgrid/List fails to recognize that it was moved to another result set. It simply deletes the row's node from the DOM. This can be temporarily fixed (as we have done) by patching dojo.store.Observable() to always do removal callbacks before addition callbacks, but I don't think this is a good long-term fix.

I'm not entirely convinced that the issue here is with dojo.store.Observable(). Is it intended in the design for the result sets to change size over time? Does it matter that it tells the grid that an object was added to one result set and then removed from another (in that order)? The issue may simply be that dgrid/List is removing the item when it should have checked to see that it was moved to another result set already.

kriszyp added a commit to kriszyp/dgrid that referenced this issue Nov 13, 2012
@myklt
Copy link

myklt commented Jun 12, 2013

frostedcheerios, were You able to find a better workaround than patching dojo/store/Observable? As I see, Observable hasn't been changed with Dojo v1.9.

I can also reproduce the same problem with TODO sample ( http://dojofoundation.org/packages/dgrid/demos/todo/ ) by adding some todos, refreshing the page and updating them. If I click on last added todo - it is removed from grid.

@dmartin-gh
Copy link
Contributor Author

I have not had a chance to see if kriszyp's changes in b234c63 have fixed this issue in the master. Internally we're still using the older version of dgrid with the patch to dojo/store/Observable.

@kfranqueiro
Copy link
Member

This should be resolved in dgrid 0.3.13 and later due to fixes relevant to #701 and #714.

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

5 participants