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

Incorrect rowHeight calculation when using virtual scrolling #3301

Closed
rlexa opened this issue Jul 5, 2017 · 2 comments
Closed

Incorrect rowHeight calculation when using virtual scrolling #3301

rlexa opened this issue Jul 5, 2017 · 2 comments
Assignees
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@rlexa
Copy link

rlexa commented Jul 5, 2017

I'm submitting a ... (check one with "x")

[x] bug report => Search github for a similar issue or PR before submitting
[ ] feature request => Please check if request is not on the roadmap already https://github.com/primefaces/primeng/wiki/Roadmap
[ ] support request => Please do not submit support request here, instead see http://forum.primefaces.org/viewforum.php?f=35

Current behavior
If a lazy loaded virtually scrolled datatable gets initialized with an empty data in the beginning then the ScrollableView.rowHeight gets calculated from the 'No records found' default row and never gets updated again which leads to incorrect calculation of the lazy requested pages afterwards (in my case the rows are 43px and the 'No records found' row is 22px which logically leads to ca. double page offset size when scrolling).
Looking at the code; the ScrollableView.rowHeight is set in ScrollableView.ngAfterViewChecked. Currently it just gets 'tr.ui-widget-content' element - which is usually the default empty row - and takes the height, which only works if there is already some data there before the view gets rendered (in fact that is how I hackfixed my cases for now i.e. setting a bogus row which is ugly because it is visible before a real lazy load happens).
Suggestion: maybe check the rowHeight every time and take it's max and reset scrolling on change accordingly?

Expected behavior
When scrolling directly to the bottom in the virtually lazy dataTable the last page should be loaded correctly (currently jumps back to beginning due to too big of a page offset).

Minimal reproduction of the problem with instructions
Use dataTable with lazy loading and virtual scrolling, make sure it was rendered once with no data, then set big data and scroll to bottom.

What is the motivation / use case for changing the behavior?
Virtual scrolling in lazy loaded data is very useful.

Please tell us about your environment:
Windows 10, npm, ng console.

  • Angular version: 4.2.5

  • PrimeNG version: 4.1.0-rc.3

  • Browser: Chrome

  • Language: TypeScript 2.3

  • Node (for AoT issues): node --version = v6.10.3

@Mrtcndkn Mrtcndkn added the Status: Pending Review Issue or pull request is being reviewed by Core Team label Jul 7, 2017
@cagataycivici cagataycivici self-assigned this Aug 23, 2017
@cagataycivici cagataycivici added Type: Bug Issue contains a bug related to a specific component. Something about the component is not working and removed Status: Pending Review Issue or pull request is being reviewed by Core Team labels Aug 23, 2017
@cagataycivici cagataycivici added this to the 4.1.4 milestone Aug 23, 2017
@cagataycivici cagataycivici changed the title DataTable - Incorrect rowHeight calculation when using virtual scrolling Incorrect rowHeight calculation when using virtual scrolling Aug 25, 2017
@nate-knight
Copy link

@rlexa Can you explain your workaround above? I am currently on v. 4.2.1 and cannot upgrade until we fix a few other things, so was hoping for a workaround/hack in the mean time. I have copied the code from this fix here and patched the 4.2.1 datatable.js code, however I still am experiencing the issue you described -- onVirtualScroll it jumps backward to the beginning. In my lazyLoad handler I am using: this.rows = this.allRows.slice(event.first, (event.first + event.rows)); Any help is appreciated!

@rlexa
Copy link
Author

rlexa commented Feb 9, 2018

@nate-knight I now implemented an own solution for a virtual list and got rid of Datatable. As far as I remember: In the Component I created a stub data item of the real expected lazy data type and initialized the datatable's rows with an array holding that item. That way the first row-height calculation is based upon the stub instead of 'No records found' text. The downside is that the stub item is visible until the real data is loaded.
FYI: In the newer version you actually have to move to TurboTable instead of Datatable, maybe it's fixed there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
Development

No branches or pull requests

4 participants