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

DobyGrids in nested rows #144

Open
ckosmowski opened this issue Oct 20, 2014 · 12 comments
Open

DobyGrids in nested rows #144

ckosmowski opened this issue Oct 20, 2014 · 12 comments
Labels

Comments

@ckosmowski
Copy link
Collaborator

I'm trying to have nested doby grids in the nested rows. (Like in the nested rows example).

I can't manage to cache a doby grid in a jquery element in the background, so i could just put it back into the dom when postprocess is called. In the example everytime postprocess is called a new dobygrid is instatiaded and populated with data again.

I'd really like to avoid this approach, because i have the table, i have the data, why do i need to reload all that stuff when i can be sure that nothing changed? Why is postprocess called everytime i scroll at all? Even if the row has not been moved out of the viewport.

Can you think of an alternate solution for this?

@EvHaus
Copy link
Owner

EvHaus commented Oct 20, 2014

In order to keep DobyGrid scrolling fast and performant, rows which the user has scrolled past are removed from DOM. This means that your nested DobyGrids are completely destroyed and need to be re-initialized.

I can add an option to disable dynamic row creation, but you will pay a massive performance penalty. It will basically be a regular <table> element at that point.

@ckosmowski
Copy link
Collaborator Author

I totally understand, why the rows need to be destroyed as soon as they leave the viewport. But (at least that is what i am experiencing) postprocess is called on every scroll event no matter if the rows are in the viewport or not.

I would expect the following after a scroll:

1.) Rows visible before and after the scroll: don't call postprocess, leave them as they are
2.) Rows visible before and not visible after the scroll: destroy them
3.) Rows not visible before and visible after the scroll: call postprocess

that would save a lot of performance if you have nested doby grids that otherwise would need to be instantiated and populated with data on each scroll even if they are visible all the time.

The same happens when i do setItem() on any item in the grid. No matter what i try, everytime setItem is called i seems that all the nested rows will be completely emptied and postprocess gets called again. So when i expand one row (i need setItem for this) all the visible nested rows in the table will get rerendered.

This problem can be best described by your comment in the code:

// If the rows were changed we need to invalidate the child rows

My question would be: Why? In our case at least, the nested rows do not need to be invalidated when the parent row changed. And reading the code "If the rows were changed" means "if setItem has been called, even if nothing really changed".

@ckosmowski
Copy link
Collaborator Author

Digging into the setItem problem a bit further, i found out that until reaching the refresh function everythign seems fine and only the rows affected will be updated. Than at the end of refresh() there is this code block:

if (this.length === 0) {
   if (!grid.fetcher && grid.options.emptyNotice) insertEmptyOverlay();
} else {
  grid.hideOverlay();
}

which causes "hideOverlay()" to be called on each refresh.

Looking into hideOverlay():

this.hideOverlay = function () {
  if ($overlay && $overlay.length) {
    removeElement($overlay[0]);
    $overlay = null;
}

// Redraw grid
invalidate();

return this;
};

So it seems to me that all the selective refreshing that is done in the methods before (find out what changed -> only update the changed things) will have no effect, because everytime hideOverlay() is called no matter if there is an overlay or not, all rows will be invalidated and redrawn and therefore every row will be postprocessed again when using setItem();

Is this a desired behaviour?

What i do is, like you suggested in some issues before:

  • Generating content inside preprocess
  • Using setItem() to change the height of the nested row to the height of the content

which causes all rows to be postprocessed again when i expand one new row.

@ckosmowski
Copy link
Collaborator Author

Also a part of this is the fact that setting "cache:true" on the columns of the nested rows won't have an effect.

@EvHaus
Copy link
Owner

EvHaus commented Oct 21, 2014

Thanks for the highly detailed analysis. It sounds like there might be a flaw in the logic for handling overlay hiding. I'll investigate.

@EvHaus
Copy link
Owner

EvHaus commented Oct 22, 2014

Can you try the latest master build?

@ckosmowski
Copy link
Collaborator Author

Thanks for the effort so far. Now i get the following error before anything shows up at all:

Uncaught TypeError: Cannot read property '0' of undefined

at the following line:

if ((sheets[i].ownerNode || sheets[i].owningElement) == $style[0]) { ($style is the one that is undefined)

int the getColumnCssRules() method.

It seems like the method is now called before $style has been initialized via createCssRules(). This is because hideOverlay() and therefor updateCanvasWidth(); is called before createCssRules().

Everthing i tried myself to fix this (creating the css rules before) leads to more errors. (Only a few rows are displayed or completely messed up rows).

@EvHaus
Copy link
Owner

EvHaus commented Oct 23, 2014

Any chance you can publish your grid for me to view somewhere? None of the tests on my end seem to yield any errors.

@ckosmowski
Copy link
Collaborator Author

The only chance i see is to make an appointment for a remote session, so you can view my screen. I don't currently have a change of publishing the software anywhere public because it is not released yet and i don't have the permission to do so.

Or i could try to make a screencast for you to view.

The tests work fine for me too. The error only happens in the real table.

ckosmowski added a commit to ckosmowski/doby-grid that referenced this issue Nov 21, 2014
@ckosmowski
Copy link
Collaborator Author

Finally i got this issue isolated and reproduced. One of the insights so far: it seems to have nothing to do with nested tables but with nested rows in general.

You can find my work at:

http://jsbin.com/vevudiyune/1/edit?js,output

  • The linked version of doby-grid.js is built from your master branch
  • To reproduce click the "+" button of a cell. The nested row will overlay some cells below
  • Click the same "+" button again to hide the nested row
  • Open it up again (now the row is inserted and not overlayed as wanted)

i hope this helps to investigate the problems we have. FYI: With the builds in your repository (latest build for example) there is no such problem. It seems to be connected with the 041851f commit.

In addition to that, if you open / close some nested rows in a faster sequence the grid will get totally confused and some of the normal rows will get different heights.

doby2

@EvHaus
Copy link
Owner

EvHaus commented Nov 25, 2014

Thanks for that investigation. I will look into it when I can and see if there's a fix.

@ckosmowski
Copy link
Collaborator Author

With the master branch, this got worse again. Postprocess is called for each and every custom row, caching doesn't work here. It can also be shown in the above example.

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

No branches or pull requests

2 participants