EvHaus / doby-grid

An HTML table element on steroids.
Other
14 stars 5 forks source link

DobyGrids in nested rows #144

Open ckosmowski opened 9 years ago

ckosmowski commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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:

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

ckosmowski commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

Can you try the latest master build?

ckosmowski commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

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 https://github.com/globexdesigns/doby-grid/commit/041851f9be87bc820393bf83742f9c8af99236b9 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 commented 9 years ago

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

ckosmowski commented 9 years ago

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.