Infinte Loading: Basic implementation

[dmitriy-kudelko]

This adds basic implementation of infinite loading functionality. We might want to improve the logic to determine when loadMoreRows callback should be invoked. But for now let's proceed with this naive implementation.

[samrith-s]

@dmitriy-kudelko A couple of comments below.

Additionally:

• Can we also ensure that there are dependent props checks? Like for example for infinite loading, all props need to be given by the user or we throw an error, rather than silently failing it.
• Can we provide a default loader? Coz right now, it just renders empty cells.
• Why are we adding multiple rows at the bottom? Shouldn't we rather add just 1 row and let the user customize if they want to add multiple rows?

[samrith-s]

We have way too many loadMore-X props. Can we rename them to something more meaningful or create an infiniteLoading prop which is an object and accepts customizations? This seems ugly.

[dmitriy-kudelko]

In my eyes grouping props in an object is a worse stuff because it leads to wasted re-renders since you provide a new object literal each time. This is why they are all namespaced with loadMore. We can use infiniteLoading instead. It's just way longer.

[samrith-s]

Okay, that makes sense. Let's keep this for now.

Another thing, why are we asking for a loadMoreTotalCount? We are assuming we will have a finite list. In case of something like a Twitter timeline, there could be tweets tending to infinity. In that case, it might not always be possible to pass a loadMoreTotalCount.

I think in the table, we would be better off just firing the loadMore call whenever the scroll reaches bottom, and let the user handle it in their callback.

What say?

[dmitriy-kudelko]

We are assuming we will have a finite list.

So here I followed existing use cases of how we use react-virtualized infinite loader on Email Logs/Campaigns pages where we exactly know the total number of rows in the database. In fact this is how most of infinite loaders implemented (including react virtualized InfiniteLoader)

[samrith-s]

Again, can we use a better name than loadMore-X?

[dmitriy-kudelko]

@samrith-s regarding the main review comment...

1. Yes, I think it's a good idea, but we should not throw an error I think. Displaying a warning is enough.
2. It's also a good idea. Do you have some loader in your mind we could add as a dependency?
3. I followed our practice to display 5 additional rows as on Email Logs/Campaigns pages. We can change the default setting to 1. Not a big deal.

[samrith-s]

@dmitriy-kudelko

1. I think we should throw an error, otherwise we are handling a lot of stuff which is technically outside the scope of the table.
2. We can use a simple loader from here http://www.davidhu.io/react-spinners/
3. I think we should change the default to one, or make the number of loading rows to be customizable. For now, we could just reduce it to 1.

[dmitriy-kudelko]

@samrith-s , please verify.

[samrith-s]

@dmitriy-kudelko Everything looks good!

Regarding this comment #14 (comment), I'll create a separate issue for it. We can pick it up later.