Implemented triggerOffset property.

[mariuszzak]

I have implemented triggerOffset property which let you adjust starting loading more items before the scroll is on the very bottom of window (or scrollable div). I've also resolved problem described here: #82. Now the trigger in scrollable divs works properly :)

[ghost]

Really nice, you're the man!

Though it looks like the build is failing, do you know what's the issue there?

CheeriO

[mariuszzak]

Thanks! :)
I'm trying to find the problem. On my local environment tests pass properly.

[ghost]

Hehe, good ol "works for me" :)

[hhff]

I would like to explore using https://github.com/dockyard/ember-in-viewport for viewport checking - it has offsets & tolerance and everything we need. What do u think @mariuszzak ?

[mike-north]

more of a stylistic point, but you could write this as

if(this.get('_customScrollableIsDefined')) {
  return this.$().position().top + this.get("_scrollable").scrollTop();
}
else {
  return this.$().offset().top;
}

or even

return this.get('_customScrollableIsDefined')) ?
  this.$().position().top + this.get("_scrollable").scrollTop() :
  this.$().offset().top;

[hhff]

ur the man mike! Happy to put the move to ember-in-viewport as work for after we get this in!

[mariuszzak]

I apologize for the long absence of answers.

@hhff I think it's not worth to couple this plugin with ember-in-viewport just for so tiny feature. I'm not enthusiast of running a combine-harvester for one ear of grain :)

@mike-north Thanks a lot for code review! Fixes are ready. Is there any possibility to get rid of the code duplication?

    postList = find('ul');
    infinityLoader = find('.infinity-loader');
    triggerOffset = postList.get(0).scrollHeight - postList.height();

[davidgoli]

please observe proper HTML indentation

[mariuszzak]

The same spec's failures are on master branch currently. New release of ember-beta brought some changes. Any idea?

[hhff]

Looks like we're going to have to move the infinity-loader unit tests to integration tests

[mariuszzak]

Tests seem to be ok now. Is there anything I should refine?

[hhff]

👍 LGTM - @davidgoli / @kellyselden wanna give this a final gaze?

[davidgoli]

I'd just reformat this to use more whitespace, like so:

if (this.get('developmentMode')) {
  return false;
}

return this._bottomOfScrollableOffset() > this._triggerOffset();

[davidgoli]

Nice work

[davidgoli]

Just a few formatting comments but this looks solid... thanks for all the tests!

[hhff]

👍 what @davidgoli said - amazing work @mariuszzak - very very keen to land this !

[mariuszzak]

Thanks a lot guys! I'm gonna improve code ASAP.

[mariuszzak]

Cosmetic improvements are finished.

@davidgoli thanks again for comments! 💯

[hhff]

Nice! I would say let's squash to a single commit and then we're ready to merge!

[mariuszzak]

Ok, done :)