airbnb / infinity

UITableViews for the web (DEPRECATED)
http://airbnb.io/infinity/
Other
2.8k stars 281 forks source link

Optional more callback #11

Closed DeaconDesperado closed 8 years ago

DeaconDesperado commented 12 years ago

I previously had confused the lazy callback for the functionality this pull request now adds.

Scrolling to the bottom of the ListView will trigger a check to the last of its pages to check if the last of that pages items is onscreen. If the item is onscreen, and a callback function for more was supplied to the ListView constructor, that view will call its more callback.

Since the context of this in the more callback refers to the ListView itself, this.append can be called from within the more callback to add more items, whether they come from an ajax call or some other static data source.

I left preloader logic out pending feedback from the original devs.

DeaconDesperado commented 12 years ago

Some quick demos:

http://snips.deacondesperado.com/infinity_demo/ajax_demo.html http://snips.deacondesperado.com/infinity_demo/demo.html

The ajax demo responds indefinitely on scroll. In a real app, I would check the data length, and if a length 0 is returned set ListView.more = false to disable further superfluous requests.

reissbaker commented 12 years ago

Nice!

The one thing I'm not sure about is having to manually set ListView.more. I wonder if there's a way to do this automatically: prevent all future requests until a callback is called, maybe? I.e.

new ListView({
  // this will be prevented from running more than once until `done` is called
  more: function(done) {
    $.get(..., function() {
      // do things
      ...

      // signal that the callback is done
      done();
    });
  }
});

and an implementation something like:

if(isBottomList(lastItem.$el) && currentView.more) {
  currentView.more = false;
  currentView.moreFn(function() {
    currentView.more = true;
  });
}

It could get more advanced (auto-append contents passed in, or stop all future requests if some value -- false? null? -- is passed), but I think that's probably making too many decisions for the end user. Just auto-blocking requests until old ones finish would be enough for now, I think.

Good work!

DeaconDesperado commented 12 years ago

I agree... I'll definitely amend what I have to insert a done callback for the data source for more.

However, the use case I was referring to earlier was long lists of heavily paginated content that the server would stream back. At least where I'm using infinity would be a scenario where more would be used to fetch the next page, page=2&per_page=50 or something like that in the query string. What I meant was that the first req that retrieved empty data should unhook the functionality for more, otherwise every scroll to the bottom of the viewport would try to go back and effectively fetch data we know doesn't exist.

Since the more callback would necessarily ALWAYS be sourced to ajax, I would think if/when to unhook the functionality for more would be best left up to the person implementing... that way, if they know they will not retrieve any new data on the next scroll for some reason, they can save the resources of attempting to fetch it. Sorry if I wasn't clear with how I worded it.

reissbaker commented 12 years ago

Ah for sure, hence the done callback. Maybe I'm misunderstanding?

My thought was that the done callback would be the trigger to control whether the more functionality keeps firing -- just without needing to explicitly set variables. You could wait to call it until after the AJAX callback completes (which seems like the sensible idea, since otherwise you could trigger the callback multiple times for the same data), or, if you wanted to, you could call it immediately and let the more callback fire as often as it wants. For functionality like yours in the demo, you'd do:

new ListView({
  more: function(done) {
    // finish immediately to let the `more` callback keep firing without waiting for new data
    done();

    // do things. an ajax call? randomly generate data clientside? who knows?
  }
});

And, of course, if the server returns an empty dataset you could always just not call done at all, in which case the more callback would never fire again.

The thing I like about a callback is it unhooks more by default -- which seems right, if you're waiting on asynchronous network requests (and most applications will be doing that). And since callbacks are a pretty standard way to do things on async request completion, it seems like it'd make sense here.

Admittedly, I could be missing something.

redterror commented 11 years ago

I think this needs work, it assumes that window is the scroll container. You can sidestep the whole container issue if you call more() once the last page is onscreen. It may be slightly early in some cases, but if your user has scrolled to the last page, they're pretty close to the end!

In that case, you'd just remove isBottomList entirely.

DeaconDesperado commented 11 years ago

@redterror Agreed, it's been awhile since I last took a crack at this but I'll go into the patch today and apply the suggested changes.

DeaconDesperado commented 11 years ago

Sorry for the multiple commits. I took the advice shared here and made the requested functionality for the done callback, which can accept a boolean indicating whether further calls to the moreFn are to be expected (to suit the ajax empty response example). I also removed the window-based scrolling logic in favor of checking lastPage on screen status per @redterror 's comments.

The demos from the older comment are updated as well.

kishaningithub commented 8 years ago

Anyone maintaining this repo?

spikebrehm commented 8 years ago

@kishaningithub no, this repo is deprecated.