-
Notifications
You must be signed in to change notification settings - Fork 293
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
Added ability to use virtual scroll with different heights of elements #206
Conversation
add @types/tween.js
For master
This has been merged. However, I haven't finished testing yet & won't publish to npm until I'm confident it's working. The demo app doesn't have a differing height example to test with. But, I'm currently only concerned about regressions with same size items that were working before. Also, because the code is more complex for size calculations there could be a performance difference. It's not noticeable, but it might be worth adding a flag to short-circuit the more complex logic if the consuming application developer knows it's not needed. The examples in the demo work 99% the same as before, but I've noticed two issues. #1) On the "Table" demo the scroll bar acts funny when it gets to the very end of the list. The scroll bar size changes and the items jump around a little. #2) On the "Loading In Chunks" demo the end event is fired 99% of the time, but randomly doesn't fire sometimes. If you scroll away and back to the bottom it will fire the 2nd time. |
Git doesn't recognize it as merged because there were many merge conflicts to resolve. The original contributors may want to review the merged code to ensure nothing was missed. |
I've noticed that some of the examples in the demo page of https://github.com/kykint/ngx-virtual-scroll-plus are buggy or broken. This leads me to believe that the original code was not quite ready for a PR yet. Either way, it's already merged & I'm working to track down the last few bugs before closing the PR. |
I'm putting this feature behind an "enableUnequalChildrenSizes_Experimental" flag for now. When it becomes more stable I'll remove the _Experimental. I may eventually remove the flag entirely, but want it to stay opt-in for now since many users have fixed-size children & don't need the additional overhead. |
Will close this PR once we have a working demo of variable heights. Assuming the merge conflicts were resolved correctly this should be fairly simple, but I won't be 100% confident the PR was successful until I see a demo which works identical between this repo & kykint/ngx-virtual-scroll-plus. |
I did a lot of refactoring & added a few new features & completely broke the variable heights. Will work on correcting soon. |
Ok. I created an example of variable sized elements in the demo folder. It uses random width/height items. I tested it against https://github.com/kykint/ngx-virtual-scroll-plus (master branch) and got poor results. This makes me believe that the PR isn't quite ready (or I'm doing something wrong). Instead of trying to make the merged PR work, I instead extracted a small part of it (from calculateDimensions) & deleted everything else (from calculateItems). The solution turned out very simple/elegant, & surprisingly works pretty well (but still not 100% perfect). The main issue is that the items jump a few pixels as you scroll. I wonder if I resurrect & modify the other portion of the PR that I deleted if the slight jumpiness will go away. Although the jumpiness might be able to be improved, I'm hoping it's "close-enough". If anyone has ideas about further improvements to the variable-height system, let me know. I'm happy to look at future PRs if anyone makes further improvements. My guess is the portion that I deleted from the PR which calculates the padding could be helpful in reducing the jitter. I'll let someone else look into that if they'd like. |
No description provided.