Skip to content

Conversation

@janhaertel
Copy link

@janhaertel janhaertel commented Jan 23, 2025

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

…oups

- fix tableholder not scrolling to bottom if there are groups
- fix tableholder jumping to top every time a group is collapsed/expanded

refs olifolkerd#4219
refs olifolkerd#4525
- fix tableholder jumping to top every time a group is collapsed/expanded

refs olifolkerd#4219
refs olifolkerd#4525
@pspsdev
Copy link

pspsdev commented Feb 12, 2025

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
Copy link

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
Copy link
Collaborator

azmy60 commented Mar 24, 2025

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!

Copy link
Collaborator

@azmy60 azmy60 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


// handle resized columns
if(this.table.modExists("groupRows")) {
this.table.modules.groupRows.virtualRenderFill();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@rathboma
Copy link
Collaborator

@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

broniec pushed a commit to broniec/tabulator that referenced this pull request Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

5 participants