slickgrid-universal icon indicating copy to clipboard operation
slickgrid-universal copied to clipboard

feat: Infinite Scroll for Backend Services

Open ghiscoding opened this issue 1 year ago • 5 comments

Similar approach to Ag-Grid Infinite Scrolling while also deviating a bit from Ag-Grid's approach

  • Infinite Scroll will fetch the next set of data and append it to the grid every time the user reaches the end of the grid scroll. This contrast with the regular approach of a Backend Pagination which will only keep data for 1 page at a time.
  • at this point it uses the Pagination Service and whenever the scroll bottom is reached, it uses the Pagination Service with gotoNextPage()
    • a possibility would be to create a lighter Infinite Scroll Service, not sure if it's worth it or not
  • NOTES
    • presets.pagination is not supported with Infinite Scroll and will revert to the first page when presets are provided. The reason is simply because, since we keep appending the data, it always has to start on first page or else we'll be missing some data.
    • Pagination is not shown but that is in fact what is being used behind the scene (the Pagination Service that is)
    • also when Sorting/Filtering it will reset and go back for first subset (page) of data.

Why and when would we use it?

  • display data early without using pagination
  • it might be used with other plugins that aren't typically used with Pagination
    • Grouping
    • Tree Data
    • Note: it would typically become more useful when the entire dataset is loaded

TODOs

  • [x] add unit tests
  • [x] add Cypress E2E tests
  • [x] update OData Service
  • [x] update GraphQL Service
  • [ ] add Documentation
  • [ ] investigate if we can re-enable Grouping/TreeData with this approach
  • [ ] possibly Infinite Scroll with local DB (no backend service)
  • [x] investigate if it's safe to use dataView.addItems() directly or if Slickgrid-Universal gets out of sync?

brave_leASu96dqr

ghiscoding avatar Jul 18 '24 03:07 ghiscoding

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.8%. Comparing base (c03b863) to head (b542543). Report is 13 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1609     +/-   ##
========================================
+ Coverage    99.8%   99.8%   +0.1%     
========================================
  Files         198     198             
  Lines       21796   21875     +79     
  Branches     7160    7056    -104     
========================================
+ Hits        21735   21814     +79     
- Misses         55      61      +6     
+ Partials        6       0      -6     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jul 18 '24 03:07 codecov[bot]

@zewa666 @jr01 CC'ing you guys since you are the biggest OData users I know 😉 I'm not sure if you would use this new feature or not but anyway feel free to comment. At this point, I'm not even sure I will merge the feature though I think it's worth it, what do you guys think?

~There's 1 thing that I'm unsure about, at the moment I have to call the gotoNextPage() via the Example itself with a new onScrollEnd event and the reason is because the OData Service doesn't have any DI and so it doesn't have access to the Pagination Service itself, hence why I need to advise the service to fetch the next batch of data (page) and so going to the next page for next fetch set of data seems like the only way to do it. I also don't want to add DI to the GridOdataService because that would require the user to add it himself which is not intuitive and would be a breaking change.~ Update: I actually found a way to move that code internally which is lot less confusing to the end user. The getCustomerCallback object argument will now include a new infiniteScrollBottomHit and when that hits, then end user will append data to dataset or initial load (not hit) then we assume new dataset (not appending)

ghiscoding avatar Jul 18 '24 03:07 ghiscoding

this actually looks quite interesting. I'm not really sure it fits a lot of use-cases, as the drawbacks of not being able to share a position with a link plus leaving the location when sorting is somewhat a bummer (but understandable). Nevertheless it could be quite useful for some scenarios.

One thing I noticed is that UX wise it would be great if you'd have an indicator that there are more rows to load above/below from your current scroll location, which disappears once you hit the top or bottom.

zewa666 avatar Jul 20 '24 13:07 zewa666

this actually looks quite interesting. I'm not really sure it fits a lot of use-cases, as the drawbacks of not being able to share a position with a link plus leaving the location when sorting is somewhat a bummer (but understandable). Nevertheless it could be quite useful for some scenarios.

hmm yeah I wonder if Ag-Grid does better on their side, it's probably the same. Clearing the current data and restart is what Ag-Grid does too as per their description

The grid cannot do sorting or filtering for you, as it does not have all of the data. Sorting or filtering must be done on the server-side. For this reason, if the sort or filter changes, the grid will use the datasource to get the data again and provide the sort and filter state to you.

For your next question

One thing I noticed is that UX wise it would be great if you'd have an indicator that there are more rows to load above/below from your current scroll location, which disappears once you hit the top or bottom.

I actually had that in mind, the getCustomerCallback returns an object to which I was thinking of adding another flag like infiniteScrollEndReached, though I had an async problem. I'll take another look at it later. Also in theory since I'm using the Pagination Service, so I have all its info which could be used for displaying or whatever

Thanks for doing a quick review, I guess the use case would be to load data quickly like a list of news or articles while not caring about the position and/or rarely doing sorting (perhaps some users might disable it entirely to avoid full reload). For the Filtering however, I was thinking if I could just filter what is locally loaded at that moment and filter against that, if so I could use similar implementation as what I've done in the past with useLocalFiltering in the Angular-Slickgrid Example 27 GraphQL Basic API without Pagination (that probably should be under a flag to use this approach or not).

EDIT

there you go, fully loaded it is... I also tested with useLocalFiltering and after a small fix, it also works but only with what is being loaded in-memory (which is to be expected)

image

ghiscoding avatar Jul 20 '24 15:07 ghiscoding

Hey @ghiscoding, I was on vacation past few weeks and couldn't respond earlier.

Really nice feature! Some minor remarks:

  • https://6pac.github.io/SlickGrid/examples/example-infinite-scroll-esm.html works really nice when I use the scrollwheel on my mouse. However, when I move the middle-drag-block in the vertical scrollbar all the way down I quickly run out of physical screen height and then need to un-hold the mouse button, move the mouse up and start again.
  • Memory usage keeps increasing while scrolling down, so it definitely isn't infinite ;)
  • Code wise I don't like that a BackendService handles UI concepts (scrolling, viewport, mousewheel). Perhaps that code can be moved to a more common place? https://github.com/ghiscoding/slickgrid-universal/blob/36bb193522aa71d0b5a57c1f391d3aec875c6032/packages/odata/src/services/grid-odata.service.ts#L108)

jr01 avatar Aug 05 '24 10:08 jr01

Not exactly sure what you mean about the drag block, but the way I've done it is to simply calculate the end of the scroll by its height and scrollY position, and it should work with any kind of scroll as long as you reach the bottom of the scroll.

For the memory usage, I assume that it's normal since that demo that I created keeps adding items to the DataView, so the memory usage will keep increasing but it should be equal to a static datagrid with the same number of rows and Infinite Scroll is more of a UI concept that doesn't always fit the reality lol. We can blame the guys who created the term "Infinite Scroll" 😆 ... and in case that you might say "but shouldn't we also remove items when adding more items to keep memory usage lower?", to which I would reply, that should be implemented on the userland side, in the demos I'm using DataView.addItems(), the user could choose to remove older items if he/she wishes too.

Lastly for the UI code, I think I tried to put that code in the UI component but I had some issues when I tried at the beginning, I can take another look to see if it's possible again now that it's all connected, that would indeed simplify the code on all Backend Services (you're probably right in the sense that there's no need to duplicate this code in all backend services)

Thanks for the feedback, it came at a good time since I was expecting to publish tonight after work :)

ghiscoding avatar Aug 05 '24 13:08 ghiscoding

and there you go, I was able to move all the Infinite Scroll logic to the vanilla component (and Angular-Slickgrid, ...) in PR #1632 That was a great suggestion that I had forgot to review after doing the POC, so thanks again for the quick review

ghiscoding avatar Aug 06 '24 05:08 ghiscoding