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

feat: Infinite Scroll for Backend Services #1609

Merged
merged 21 commits into from
Aug 1, 2024

Conversation

ghiscoding
Copy link
Owner

@ghiscoding ghiscoding commented Jul 18, 2024

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

  • add unit tests
  • add Cypress E2E tests
  • update OData Service
  • update GraphQL Service
  • add Documentation
  • investigate if we can use Grouping and other features with this approach
  • possibly Infinite Scroll with local DB (no backend service)
    • will be done in separate and next PR
  • investigate if it's safe to use dataView.addItems() directly or if Slickgrid-Universal gets out of sync?

brave_leASu96dqr

Copy link

stackblitz bot commented Jul 18, 2024

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

Copy link

codecov bot commented Jul 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ghiscoding ghiscoding marked this pull request as draft July 18, 2024 03:55
@ghiscoding
Copy link
Owner Author

ghiscoding commented Jul 18, 2024

@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)

@zewa666
Copy link
Contributor

zewa666 commented Jul 20, 2024

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.

@ghiscoding
Copy link
Owner Author

ghiscoding commented Jul 20, 2024

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 ghiscoding changed the title feat: Infinite Scroll for Backend Services (POC) feat: Infinite Scroll for Backend Services Jul 21, 2024
@ghiscoding ghiscoding marked this pull request as ready for review August 1, 2024 02:32
@ghiscoding ghiscoding merged commit b2ff957 into master Aug 1, 2024
8 checks passed
@ghiscoding ghiscoding deleted the feat/odata-infinite-scroll branch August 1, 2024 02:38
@jr01
Copy link
Collaborator

jr01 commented Aug 5, 2024

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

Really nice feature! Some minor remarks:

@ghiscoding
Copy link
Owner Author

ghiscoding commented Aug 5, 2024

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants