feature(settings): adds setting for default number of items per page #7598
Conversation
Most queries that default to 10 now use the setting, whose valid range is, for now, 2 to 5000. In order to keep this simple, queries that default to 20 items will continue showing at least 20 items; the setting is used only if it’s greater than 20. Fixes Elgg#2650
|
Fine with this. Definitely an improvement. |
| @@ -11,7 +11,7 @@ | |||
|
|
|||
| // YYYYMMDD = Elgg Date | |||
| // XX = Interim incrementer | |||
| $version = 2014111800; | |||
| $version = 2014130300; | |||
There was a problem hiding this comment.
huh... why the version bump in this PR?
There was a problem hiding this comment.
Because of engine/lib/upgrades/2014130300-1.10.0-add_default_limit-fcef9e7ce01e26a4.php
|
Other than the remark on the version it LGTM |
|
I'm not sure about this PR. Do we really want to have a setting that affects every single list that doesn't specify their own limit? Is this useful enough feature? I doubt the same default limit would work with all the different contents that can be listed on a site. |
|
If for some reason that does not work... leave it at 10. This gives you more control (if you need it) |
|
Yes, but this adds new logic to many files. I'm not sure if the ROI for maintaining that logic is worth it. |
|
This feature is a reaction to the complexity and sheer amount of API that would be added by #6256, which would give extremely fine-tuned (and too much) control over query $options. Would you argue that 10 is such a natural number for list length that had you built Elgg from scratch you would hardcode it everywhere? Is the answer somewhere between these two PRs? Eg the granularity of named queries but limited to changing limit. |
|
Because I do think https://github.com/Elgg/Elgg/pull/6256/files is a powerful path to go down if we can have a more reasonable balance of control over the queries. (But still I think almost all our global constants that affect UI should be able to be altered without too much trouble) |
|
I agree devs should be able to change the limit. I'm just not sure if this is the best way to do it. |
|
I think I'd prefer the named queries, but with a limited amount of options at first. |
|
I think this brings a big return to site owners. "Can we show more items per page" is one of the most common questions I get. Worth the maintenance overhead IMO. |
|
Agreed with Steve. On Thu Dec 04 2014 at 11:19:35 AM Steve Clay notifications@github.com
|
|
Ok, I won't block. |
|
How do we get devs to add this to their community plugins? |
feature(settings): adds setting for default number of items per page
we could write a small piece on it on |
(replaces #7339 -- trying to squeeze this in)
Most queries that default to 10 now use the setting, whose valid range
is, for now, 2 to 5000. In order to keep this simple, queries that
default to 20 items will continue showing at least 20 items; the setting
is used only if it’s greater than 20.
Fixes #2650