Fix errors when a grouped table is shown

[janhaertel]

If a table has groups which are all collapsed and the VirtualDomVertical renderer is used, there are some issues scrolling the table:

Example
https://jsfiddle.net/isaraystanz/tpgh5vze/11/

• On initial display there is a horizontal scrollbar even though fitColumns is enabled and the colums do fit the view without a scrollbar being needed.

I noticed that RowManager.renderTable() will call VirtualDomVertical.renderRows() => _virtualRenderFill() which triggers GroupRows.virtualRenderFill().
In this function the minWidth is adjusted if there are only group rows causing a horizontal scrollbar to be displayed.
Later in RowManager.renderTable(), the call to RowManager.layoutRefresh() will lead to filtColumns.scaleColumns() to be invoked, which changes the width of the columns.
After this, the columns are thin enough to fit the view but the horizontal scrollbar is still being shown, due to the minWidth that was set.
I added a call to GroupRows.virtualRenderFill() after this, so the minWidth will be adjusted and the horizontal scrollbar disappears.

• The table cannot be scrolled to the bottom, because the view always jumps back to the top.

RowManager._virtualRenderFill() will call row.calcHeight() after rows have been rendered causing Group.outerHeight to be set internally. This is always 0 because the implementation for Group.calcHeight() is empty.
The rowsHeight calculated later will then also be 0 since all groups are collapsed.
When the view is scrolled so far, that the next rows have to be added at the bottom and some have to be removed from the top,
VirtualDomVertical._removeTopRow() tries to sum up row heights until none have to be removed anymore.
Since the height is always 0, all rows are added to a list of removable rows then all are removed.
I added a minimal implementation to Group.calcHeight(), so height will not be 0.

• The table scrolls to the top every time a group is collapsed/expanded.

Collapsing/Expanding a group will lead to_virtualRenderFill(), (GroupRows.updateGroupRows () ... => RowManager.reRenderInPosition() ...=> _virtualRenderFill()) which sets scrollTop after rendering. Here the position is not correct due to the groupRow height being empty.
This is fixed by the code from the previous issue.

Is the implementation for GroupRow.calcHeight() empty on purpose?
If so the height may have to be calculated differently.

These problems are also addressed in the github issues #4219 and #4525.

fixes #4219
fixes #4525

[pspsdev]

I have tried these fixes and I no longer have scroll problems on my closed grouped table. Thanks @janhaertel and looking forward to seeing this merged!

[Kallius74]

This pull request fixes many of my scrolling issues with the vertical virtual renderer and row groups, but not all. Scrolling down with these code changes now works smoothly: if you hold down the thumb of the scrollbar and move it down, it follows the mouse pointer and maintains its vertical distance from the mouse pointer.
However, if you reach the bottom of the scroll area, hold down the scroll bar thumb and move it up again, you will notice jumps and an increasing distance between the scroll bar thumb and the mouse pointer.

The issue seems to arise from the line...
table.insertBefore(row.getElement(), table.firstChild);
... in VirtualDomVertical.js in _addTopRow:

Adding an element to the top changes the position of the vertical scrollbar, and this triggers a scroll event with an increased scroll position value compared to the last scroll action.

The event listener attached in initialize() in RowManager.js recognizes this as a “scrolldown” action (even if the user has scrolled up).

This leads to a "_removeTopRow" and "_addBottomRow" call in VirtualDomVertical.js.

[azmy60]

This is great! I'm trying to understand the whole situation carefully as well as looking into the issue @Kallius74 mentioned. Hopefully we can merge this soon!

[azmy60]

Thanks for submitting the PR - awesome work! Just one small change request.

[azmy60]

Could you do this inside the GroupRows module? Just making sure that we are isolating group rows related codes within its own module.

You can probably subscribe to a table-layout event from the GroupRows module and see if that works. If not, please let me know, and we can figure something out.

[janhaertel]

Hi I have been on parental leave for a while and have only now been able to look into this.
I rebased the master and moved the virtualRender code to group rows.

[rathboma]

@janhaertel Just checking in to see if you can make the change Azmi requested above? If so, we can merge once the change is made