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

pagination with editable cells #167

Closed
oyaaraas opened this issue May 18, 2012 · 10 comments
Closed

pagination with editable cells #167

oyaaraas opened this issue May 18, 2012 · 10 comments

Comments

@oyaaraas
Copy link

Hi,

I have a grid with pagination and editable cells (NumberTextBoxes). The grid also use a JsonRest store wrapped in a Observable store to communicate with the backend.

On the first page everything works correct whenever I edit one of the cells (with autoSave: true). However, when I go to the second page and edit a cell, the row simply disappears from the grid.
There is also strange behaviour on the last row of the grid if it is on page 2 or later. It doesn't dissappear in the same way, but the grid event handling doesn't update the row / reset the row (as it does on page 1).

@yassam
Copy link

yassam commented Jun 14, 2012

I'm also experiencing the first problem reported above, namely that only editing on the first page works.

Please let me know if you intend to fix this soon, otherwise I'll fork and fix.

Update: Could this be linked to #194 ?

@oyaaraas
Copy link
Author

After upgrading to dojo 1.8 pagination now works fine in all but one scenario.

The bug is on the last element of each page. Seems like the logic in renderArray function in List.js expects a row.nextSibling to do its magic. However on any page the last element doesn't have a nextSibling, and is therefore not updated with the latest data.

Is this something you will look into anytime soon?

@kriszyp
Copy link
Member

kriszyp commented Sep 10, 2012

With Dojo 1.8, I don't see any issues with dropped rows. For editing the last element of the page, is the issue row dropping, or some other behavior? Are there any other details on how to reproduce this?

@oyaaraas
Copy link
Author

JsonRestStore and dgrid with Pagination have row-dropping on the last element.

I've created a node demo app on cloud9: http://c9.io/oyaaraas/dgridpaging
Just run app.js after logging in and point your browser to http://dgridpaging.oyaaraas.c9.io/
If it is more convenient, I can push the demo to github.

Try to edit the text fields and you'll see that it works fine on all rows except on the last which will drop.

If you refresh the page a couple of times you'll also see an example of #257, especially in FF and IE.

@oyaaraas
Copy link
Author

I've put it on github as well: https://github.com/oyaaraas/DgridPaginationDemo

@kriszyp
Copy link
Member

kriszyp commented Sep 17, 2012

With the provided test case I was able to reproduce and fix this issue. However, this issue only appears with the combination of the Pagination extension and OnDemandGrid, which is considered to be an invalid combination, since they provide conflicting querying mechanisms (I'm surprised it works at all). When I alter his test case to properly use the dgrid/Grid instead of dgrid/OnDemandGrid the last row can be properly edited without issue.

I did put commit the fix for pagination in the test case, but I'd certainly recommend using a valid combination of Grid+Pagination instead:
https://github.com/kriszyp/dgrid/tree/fix-167

@oyaaraas
Copy link
Author

The use of OnDemandGrid was a mistake by me, it should have been Grid instead. So that solves the line dropping issue.

However, the problem of updating the last row with the new data from the put response still remain.

You can see this quite clearly in the demo - if you edit on the last element, the "Update status" field is not updated as with the other rows.

I believe this issue is related to line 440 in List.js, where it checks for a row.nextSibling before deleting the current row, and later inserts it again at the stored position.

@ghost
Copy link

ghost commented Sep 25, 2012

Can you retry against current master? It's possible the remaining issue here was resolved by a recent fix to another issue. Thanks!

@oyaaraas
Copy link
Author

Latest master introduces line-dropping again.

@ghost
Copy link

ghost commented Oct 5, 2012

Sorry for the cross-up here. The first attempt we had merged had specifically left out part of Kris' original fix, because we were thinking that part was related to mixing in Pagination and OnDemandList at the same time, but it turns out that was not the case.

We've verified and merged the full set of changes now, and it appears to resolve the issue in your example application. Let us know if you still have problems. Thanks!

FYI, here's the merge commit: 8ab44ba

This issue was closed.
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

3 participants