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

BasicVertical renderer restores scroll position in rerenderRows #4481

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Bobcat64
Copy link

Makes the BasicVertical renderer remember the scroll position when rerenderRows is called so it can be restored after clearing and rendering the rows.

This fixes #4469 when Progressive Scrolling is used with the Basic renderer, the table scrolls back to the top when a new page is loaded. This is because all rows are cleared and re-added.

Side note/Suggestion: The documentation at https://tabulator.info/docs/6.2/render#vertical for rerenderRows says that the rows should be cleared AND rendered before the callback is called. However the current code for both the Basic and Virtual renderers clear then call the callback and then render the rows. Might be prudent to update the documentation to reflect this.

@olifolkerd
Copy link
Owner

How does this resolve the issue?

In basic render mode the table generally doesn't have a height set and so the scroll is not on the table element but usually on the page body and it is the redrawing of the content that results in the loss of scroll position. Which changing the internal scroll position of the table will not resolve?

Cheers

Oli

@olifolkerd olifolkerd added the PR Needs Work The PR is a good idea, but needs some updates before it can be merged label Apr 28, 2024
@Bobcat64
Copy link
Author

Bobcat64 commented May 1, 2024

It resolves it when the table element does have a scroll position. Such as when the height is explicitly set like in the example in the linked issue.

Only way I can think to resolve your scenario with the page itself scrolling would be to temporarily add some padding or something before the rows are removed so the total height doesn't change. I'm however unsure if that would just cause more performance issues with the browser redrawing. (I am not well versed in when/what causes browser re-renders)

I suppose I'm also a bit at a loss why adding new rows to the end should end up with the basic renderer clearing and recreating all the rows. Perhaps there should be a method on renderers to update/render specific rows (or just new ones)? This would allow the basic and/or virtual renderer to determine if it needs to draw/add elements as necessary.

Is there an issue with setting the scroll position to its previous value after rendering the rows?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR Needs Work The PR is a good idea, but needs some updates before it can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Progressive Load: Scroll on Basic Renderer jumps to top while scrolling
2 participants