ElevatoDigital / dewdrop

Dewdrop makes writing complex WordPress plugins simpler by providing a sensible project layout and developer tools.
Other
11 stars 3 forks source link

ability to setPageSize(0) to show all rows in 1 page #63

Closed bravadomizzou closed 8 years ago

bravadomizzou commented 8 years ago

Needed ability to show all listing rows in 1 page. This clears way so we can have a dropdown that allows user to select how many rows to see in a page (with All option).

To list all items: $component->getListing()->getSelectModifierByName('SelectPaginate')->setPageSize(0);

@griffbrad or @darbyfelton please review + merge

griffbrad commented 8 years ago

You can already removeSelectModifierByName('SelectPaginate') to do this. setPageSize(0) seems confusing to me because 0 is instead being interpreted as "infinite" in this case.

bravadomizzou commented 8 years ago

That way also requires quite a few changes because you have to update this:

https://github.com/DeltaSystems/dewdrop/blob/master/Dewdrop/Admin/Page/Stock/view-scripts/index.phtml#L100

Plus the Total row count on the Listing would not get set because of this (so it defaults to 0):

https://github.com/DeltaSystems/dewdrop/blob/master/Dewdrop/Fields/Listing.php#L302

So it shows no rows since it is not querying for:

COUNT (*) OVER () AS "_dewdrop_count"

darbyfelton commented 8 years ago

Would addition of inline comments to indicate that page size of zero means no paging help to avoid confusion? Also I notice that there are strict comparisons of the page size to zero without casting to integer first, which could be problematic depending on caller context.

bravadomizzou commented 8 years ago

I reverted changes to BulkActionForm.php and bulk-action-form-close.phtml.

Instead of using 0 as the page size, you can disable pagination like this:

$listing->getSelectModifierByName('SelectPaginate')->disable();

The disable() method will set the pageSize to INF since there would be no limit on the number of results in the listing.