RowDetailView Quick Fix

[carlosdelros]

Hi @ghiscoding et all,

First, thank you for all the hard work building this all-batteries-included library.

I've recently started working with Slickgrid-vue and have been having great results for the most part. Most recently I had some issues with the row-detail-view plugin. I later found this note in the docs:

NOTE Please note that because of the complexity behind Row Detail, the following features cannot be mixed with Row Detail because they will cause UI problems: Grouping, Pagination, Tree Data, RowSpan

I have also seem similar comments in some closed issues, BUT I'd like to suggest a quick fix that should help in some cases.

I ran into an issue when using both server side pagination and the rowDetailView plugin. The source of the issue seems to be this line: const rowId = item[this.dataViewIdProperty];
It seems that because of the way rows are removed before adding new rows it sometimes can't find the dataViewIdProperty in the dataset.

I cloned the project and added a check for the existence of items right before this line: if(!item) return; and this seems to fix most problems (at least for my current usecase) I was able to switch pages, filter, change the page sizes, sort, etc. I ran all the unit tests and they ALL PASSED. So:

    const item = this.dataView.getItemByIdx(rowIndex);

    if(!item) return; // <---------------- This is the change

    const rowId = item[this.dataViewIdProperty];

    if (this._expandedRowIds.has(rowId)) {
      this.onBeforeRowOutOfViewportRange.notify(
        {
          grid: this._grid,
          item,
          rowId,
          rowIndex,
          expandedRows: Array.from(this._expandedRowIds).map((id) => this.dataView.getItemById(id)),
          rowIdsOutOfViewport: Array.from(this.syncOutOfViewportArray(rowId, true)),
        },
        null,
        this
      );
      this._disposedRows.add(rowIndex);
    }
  }

So even if, as you mentioned in the comment, the complexity of the plugin makes so that we cant mix these features, it seems that this simple fix can at least help in some cases that would be very useful. So a good reward/effort ratio in my opinion.

Please consider this. If you think it's an easy enough fix, I can create a PR from a fork and have it merged.

Thank you

[ghiscoding]

Yeah I've put these errors in place because I was getting annoyed of repeating myself when few people kept opening issues without searching for old issues first. The funny thing is that even with these errors in place, some users still think that someone can help them by just opening a Stack Overflow question and hope for the best... which of course nobody answered (not even me) 😆

But anyway, if you do get something that can help by simply changing a couple of lines, then yeah a PR would be welcome for sure and the errors could be removed (or simply turned into simple console warnings).

Just a couple of things to note though, the way that the Row Detail works is that it calculates an offset depending if it's expanded or not. When it's expanded, it just kind pushes the next rows further down by an offset of X rows (which is why the Row Detail height is calculated by rows instead of px). So all the rows are actually still exists behind the Row Detail which itself has a higher z-index, so you don't see the rows but they are still there behind and at some points in time, we had an issues that some users saw undefined showing in the Row Detail and it was because of that reason (the rows weren't reset properly).... anyway, the Row Detail plugin kinda got started from this Stack Overflow answer: https://stackoverflow.com/a/29399927/1212166

The fact is that because of the row offset calculation and all, it becomes complex when dealing with Pagination and others

[carlosdelros]

Created a PR

#2026

[carlosdelros]

Haha yeah I had noticed that, it's just a container with display: absolute on top of X number of empty rows based on the panelRows property empty/dummy rows, which is an interesting way of doing it.

But anyway, yeah I don't pretend that this simple fix will fix all issues with this plugin, like the one you are mentioning with the undefined. And maybe later down the line I will find other issues, but at least initially it fixes issues I was having when changing pages, filtering, changing the page size, sorting, etc. So that's an easy win.

I'll fork the project and submit a PR. Thank you for the quick reply.