Countless pagination

[ddnexus]

Add the Pagy::Countless subclass and the countless extra.

[ddnexus]

@workgena I didn't try the code yet. If you have time, please, take a look at it. The doc should be already OK, but it is missing the tests for the Pagy::Countless and countless extra. Any help will be very appreciated. Thanks.

[workgena]

Today I'll take a look at this solution. And will write tests for it.

One thing I see strait away is:

• original Pagy returns ActiveRelation, in cast of ActiveRecord(Rails)
• Contless returns an Array

Since, it is another subclass - it should be OK, no braking change, I think. Worth mention in documentation.

[ddnexus]

True! I thought about adding it, and then I forgot. Thanks.

…

On Mon, Nov 19, 2018, 2:41 PM Gena M. ***@***.***> wrote: Today I'll take a look at this solution. And will write tests for it. One thing I see strait away is: - original Pagy returns ActiveRelation, in cast of ActiveRecord(Rails) - Contless returns an Array Since, it is another subclass - it should be OK, no braking change, I think. Worth mention in documentation. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#108 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGJcTXRppmShfU-kxfZp5pbZwyNYQVJks5uwmCcgaJpZM4Yn6xi> .

[ddnexus]

Thanks for fixing my typos in the items test :).
Thanks for the extra countless tests.
It looks like we miss only the Pagy::Countless.

[workgena]

Many cases for Pagy::Countless already covered(implicitly).

Some new logic in finalize should be covered explicitly, values at:

attr_reader :count, :page, :items, :vars, :pages, :last, :offset, :from, :to, :prev, :next

[workgena]

I've integrated this countless-branch into my project at work.

I was a little bit concerned about COUNTLESS = true, but it works absolutely fine. Let me explain the use-case:

Each user can decide what pagination to use: Normal(with count) or Fast(Countless) by changing user-settings. So, PostsController always has require 'pagy/extras/countless' included. Depending on the user settings, the code executes with pagy(collection) or pagy_countless(collection).

Both scenarios work as expected.

[ddnexus]

I didn't understand what was your concern with COUNTLESS.

BTW, your use-case looks interesting. If you have time, you may want to write some article/tutorial explaining what you did.

[workgena]

Lets imagine PostsController with require 'pagy/extras/countless' always. So Pagy::COUNTLESS = true.

But in fact it uses normal pagination with @pagy, @records = pagy(collection).

I was afraid, that this block of code populates vars with wrong values.

[ddnexus]

understood. The COUNTLESS constant is just to avoid to load the aliases and method for the items extra... but that is done at load time. Then it does not matter because they are designed to work in parallel, with different named classes and methods.

[espen]

Just a thought here: this looks great and I assume it would be mostly useful with endless scrolling and other non-nav UI. However this could still be useful when having a normal pagy_nav. And in those situations I think the nav helper should just have next/prev links. Currently it will shows "page 1, 2" on load when the total pages is maybe 90. I understand why, but the result is confusing UX. So a better situation for the user would be to just have next/prev links and skip the page numbers.

[ddnexus]

@espen: just feed size: [] to remove all the page links, leaving you only the previous/next buttons

[workgena]

BTW, me miss this in documentation 📚 ...

[ddnexus]

@workgena this time I didn't forget :)

Skipping the page links

Also reported in the last note of series

[ddnexus]

@espen @workgena Endless scrolling (or incremental scroll) is coming with the more extra (currently in development). It will provide the countless pagination with empty series and a few helpers like pagy_more_url, pagy_more_link and pagy_more_json to implement manual or ajax incremental scroll.

I need someone for trying it and writing tests (hoping to be able to push it soon with at least the doc)

[espen]

Thanks. Will try this out a bit more. I think as a user (at least in my app) it's rarely relevant to me how many pages I have. Either I use search or just click next. So skipping one SQL query and not present page links could be a good solution both for performance and UX.

Awesome to hear that endless scrolling will be implemented.