Remove observer on viewportEntered

[snewcomer]

Saw some code that seems to make duplicate requests when the viewport is entered. I'm sure this code was around for a reason in the beginning, so feel free to close as I might be missing something 👍!

To further clarify, I believe this line of code in ember-in-viewport will trigger the method defined in lt-infinity and viewportEntered is targeted towards performing ancillary actions and not necessarily fetching more data for example.

[snewcomer]

I will also add some tests around the lt-infinity component if everybody wants!

[buschtoens]

Hmm, I believe this was originally implemented this way to make sure that ELT keeps asking for more rows until the viewport is completely filled. Imagine your data fetching function only returns 10 rows at a time, but the viewport fits 25. The function would have to be called three times until the whole viewport is filled.

I don't really have strong feelings in favor of this behavior, however I think some people might depend on it.

btw, we might change all this with the new occlusion feature. So, if you're interested in that, feel free to test drive it and make suggestions towards API design! 😀

@alexander-alvarez What's your take?

[snewcomer]

Ah I gotcha. I love the new occlusion feature! 👍

The first thing I noticed was the scrolling demo was making a request for up to page 3. This PR only makes a fetch for the first page, but isn't making further requests. I'm wondering if this PR fixes that.

@buschtoens Outside of this PR, is the idea for a scrolling table to either pick b/w occlusion or infinity? Or do you think it should default/make occlusion the only option for a scrolling table?

[alexander-alvarez]

Unfortunately I have no experience with the code in question ...

Outside of this PR, is the idea for a scrolling table to either pick b/w occlusion or infinity? Or do you think it should default/make occlusion the only option for a scrolling table?

Occlusion and infinity are two separate concepts that are not mutually exclusive. I.e. if you do infinity scroll and you eventually render millions of rows, you're going to get poor rendering performance, so you would benefit from occlusion rendering -- where items that are off the screen aren't being rendered in the DOM.

We're working on fixing up the occlusion rendering feature parity with the standard table (saw a lot of PRs from @buschtoens this past week :D), and would definitely clean up documentation on it's suggested usage.

do you think it should default/make occlusion the only option for a scrolling table

When we get it to a stable state, I could see it being enabled by default for all use cases.

[snewcomer]

I can close for now!