daveaglick / FluentBootstrap

Provides extensions, helper classes, model binding, and other goodies to help you use the Bootstrap CSS framework from .NET code.
http://www.fluentbootstrap.com
MIT License
200 stars 76 forks source link

PaginationFor #35

Closed ryanelian closed 9 years ago

ryanelian commented 9 years ago

In the spirit of the just-created RadioList feature addition discussion thread, I would also like to bring forth the idea of a more powerful pagination.

Attached below is the (very) rough proof of concept for PaginationFor. Do you think it will fit into the FluentBootstrap library?

The controller 1

Extension methods for IQueryable and IEnumerable to IPagination Separated IQueryable and IEnumerable extensions for performance reason, since with EntityFramework, LINQ queries are performed against SQL, not in-memory! 5

The view 2

quick and dirty pagination HTML Helper 3

what IPagination looks like... 4

The result 8

Source classes downloadable here: https://www.dropbox.com/s/hm1r0797wdgyq8l/FluentBootstrapPaginationPOC.zip?dl=0

daveaglick commented 9 years ago

I like the idea of a data-driven pagination a lot.

My first reaction is that there's really no reason this has to be a model-bound PaginationFor(...) as opposed to just an overloaded Paginiation(...) extension. Generally, model-binding helps when we need to get some additional metadata out of the model for the property being bound (such as the form control name for model-bound form elements). In this case the model isn't really providing anything to the helper other than the data itself, and that could be easily passed in as using a syntax like @Html.Paginiation(Model.PagedElements, ...) as opposed to the strongly-bound lambda syntax. That would give a little more flexibility to allow programmatic pagination straight from the view code without going through a model as well.

I like the data you've represented in the IPaginiation<T> interface (current page, total pages, etc.) but I am hesitant to introduce an instance class that the consumer would use into the mix. So far, all of FluentBootstrap is configured and consumed via fluent interfaces and extension method overloads - as far as I recall there isn't another case of the consumer instantiating a FluentBootstrap-defined class and then passing it to one of the helpers. That said, the data in the interface could be provided to the extension method as optional parameters with sensible defaults without resorting to passing in an actual interface.

Let me think on this and see what I prototype tomorrow. Thanks as always for the great feedback.

ryanelian commented 9 years ago

I like the data you've represented in the IPaginiation interface (current page, total pages, etc.) but I am hesitant to introduce an instance class that the consumer would use into the mix. So far, all of FluentBootstrap is configured and consumed via fluent interfaces and extension method overloads - as far as I recall there isn't another case of the consumer instantiating a FluentBootstrap-defined class and then passing it to one of the helpers.

When I set this up, I thought of allowing paged elements (Pagination) to be used along in decorating the UI / view. For example, with the CurrentPage and TotalPageCount information, the developer can create something like: displaying page 2 of 7.

If the data in the interface is provided as just parameters to the html helper without allowing the devs to actually use them directly, I'm not really sure how to actually do that.

How about simply making the Pagination class into internal class so that customer cannot instantiate IPagination outside the provided fluent API from IQueryable / IEnumerable?

That being said, I'm excited that you like the idea and I can't wait for your prototype! :+1:

daveaglick commented 9 years ago

The more I thought about this, the more I think a simple and straightforward implementation is the way to go. I'm worried that too much in the way of templates or model-bound data will end up making this less generic.

For example, you mentioned having the automatic pagination extension help the developer to output some sort of "displaying page x of y" text. How would that text get defined? Do we assume a particular convention and let the consumer override? That's not very friendly to foreign languages... It also starts getting into templating concepts, which I'd like to stay away from. And most importantly, why wouldn't the consumer just write the straight Razor code directly in the view to output whatever paging status is necessary?

That last part is what I kept coming back to. Any advanced use of paging (such as displaying the actual paged items, dividing the sequence into chunks, etc.) would be better served out of the scope of FluentBootstrap and directly in the Razor view code.

Here's what I ended up with - two simple extensions:

AddPages(IEnumerable<string> hrefs, int? activePageNumber = null, int? firstPageNumber = null)

and

AddPages(IEnumerable<KeyValuePair<string, string>> textAndHrefs, int? activePageNumber = null, int? firstPageNumber = null)

The first overload lets you specify a sequence of links and then generates the pagination with a number of items equal to the number of links you pass in. You can optionally specify an active page number (or none if null) and a first page number (1, or wherever you last left off in a previous AddPage(...) or AddPages(...) call if null).

The second overload lets you also supply text for the paginations, and will default to the page number if any of the keys are null.

They can be used like this:

@Html.Bootstrap().Pagination().AddPages(new [] {"http://google.com", "http://bing.com", "http://yahoo.com"}, 5, 4)

and

@Html.Bootstrap().Pagination().AddPages(new[]
        {
            new KeyValuePair<string, string>("Google", "http://google.com"),
            new KeyValuePair<string, string>("Bing", "http://bing.com"),
            new KeyValuePair<string, string>("Yahoo!", "http://yahoo.com")
        }, 2)

You can also combine with other fluent extensions as usual:

@Html.Bootstrap().Pagination()..AddPrevious().AddPages(new [] {"http://google.com", "http://bing.com", "http://yahoo.com"}).AddNext()

I also added an extension to support T4MVC, but not one to support plain old MVC since MVC routes have to be specified as a tuple with action, controller, and route values and I thought the verbosity of passing all of that in a single sequence outweighed any advantage to calling several individual AddPage() calls.

Anyway, let me know what you think.

ryanelian commented 9 years ago

For example, you mentioned having the automatic pagination extension help the developer to output some sort of "displaying page x of y" text. And most importantly, why wouldn't the consumer just write the straight Razor code directly in the view to output whatever paging status is necessary?

That's what I meant actually. Since the Paginate() extension I created is designed to fill those fields automatically, the developer at the view code can just do something like displaying page @Model.PagedElements.CurrentPage of @Model.PagedElements.TotalPageCount without the need to manually re-calculate the total page number, etc. etc.

Any advanced use of paging (such as displaying the actual paged items, dividing the sequence into chunks, etc.) would be better served out of the scope of FluentBootstrap

But then, your AddPages() extensions are actually good if you want to keep the actual element paging logic out of FluentBootstrap. They are simple and gets the job done. I approve :+1:

EDIT:

I just thought of it. What about if the user wants to make a pagination like...

`

`

(Run that here http://www.bootply.com/new)

There's got to be a way to grey out the ... or the Previous / Next button. (li class = disabled, a href removed)

How about setting at the textAndHref version of the extension, if the href is nulled, that page link will be greyed out?

daveaglick commented 9 years ago

I like the idea.

For comparison, you'd have to make that paginiation like this right now:

@using(var pagination = Html.Bootstrap().Pagination().Begin())
{
    @pagination.PageNum("Previous").SetDisabled()
    @pagination.PageNum("1").SetActive()
    @pagination.PageNum("2")
    @pagination.PageNum("3")
    @pagination.PageNum("4")
    @pagination.PageNum("...").SetDisabled()
    @pagination.PageNum("8")
    @pagination.PageNum("Next")
}

Obviously not ideal. With your suggested change, I can now write:

@Html.Bootstrap().Pagination(new StringKeyValueList
{
    { "Previous", null },
    { "1", "#" },
    { "2", "#" },
    { "3", "#" },
    { "4", "#" },
    { "...", null },
    { "8", "#" },
    { "Next", "#" }
}, 2)

Is that better? I think so - it's roughly the same number of lines, but certainly less verbose. It would also be easier to create the sequence beforehand and then pass it in as opposed to iterating like you would have to do in the first version.

Note that I also added a new StringKeyValueList class to FluentBootstrap (as well as some other variants) specifically to make initializing sequences of KeyValuePair easier in cases like this. I also added variants to the construction extension for paginations that implicitly call AddPages(...) so that you can do it all from a single extensions.