[UX] Views: Pager options: the "Items per page" label does not make sense for "Display a specified number of items"

[klonos]

I just said "specified number of items" (read: "no pager"!) ...what is this "items per page" thing?

Should say "Total items" or "Maximum number of items".

---

PR by @klonos: https://github.com/backdrop/backdrop/pull/1904

[quicksketch]

This looks like it would be pretty trivial to fix. Considering the pager type (None) and the pager options are on two separate forms. No JS required to fix this problem.

[Graham-72]

I see that it is a one year anniversary today for this issue! I think the questions need to be restructured in order to make their purpose clear. Perhaps question 1 should be whether the view should limit the number of items displayed, if so what is the limit. Then question 2 should be whether a pager should be used, if so should it be a full or a mini pager. Alternatively the subheading 'PAGER' could be replaced with something more explanatory like 'ITEMS PER PAGE' and 'Use pager' could be 'Options'.

[klonos]

I've filed a PR that fixes this issue plus some other UX inconsistencies. For example, there is no reason to be displaying for singular "1 item, skip x", because if you skip any number of items while you have configured only a single one to be displayed, then there will be no items to be rendered in the first place. This has to do with the fact that there is no validation in the pager settings forms, but I think that it will be best to tackle that in another issue (already filed #1433 for that).

I will shortly post some before/after screenshots to make it easier for those that will review the PR...

[klonos]

"Display a specified number of items" option, no offset:

• before:

• after:

"Display a specified number of items" option, with offset set:

• before:

• after:

[klonos]

"Display all items" option, no offset:

• before:

• after:

"Display all items" option, with offset set:

• before (if we skip any amount of items, then "all items" stops being relevant):

• after:

[klonos]

"Paged output, full pager" option, no offset:

• before:

• after:

"Paged output, full pager" option, with offset set:

• before:

• after:

[klonos]

"Paged output, mini pager" option, no offset:

• before:

• after:

"Paged output, mini pager" option, with offset set:

• before:

• after:

[klonos]

...also before, when opening the settings for either paged or non-paged, you'd get "Items per page" as the field label and "The number of items to display per page. Enter 0 for no limit."

• before (same for both paged and non-paged):

• after, no pager:

• after, pager:

[jenlampton]

@klonos I love this issue!! Thanks for all the great work. One thought: Instead of "Items to display per page" can we say "Items to display on each page" it's a little more human.

[klonos]

Instead of "Items to display per page" can we say "Items to display on each page" it's a little more human.

Yeah, I often use "per" in my life, so I don't personally have an issue with it, but I see what you mean. It is more likely for non-native English speakers to be familiar with the word "each" rather than "per". While working on this issue, I thought the very same thing about "Offset" vs using something like "Items to skip".

Lets get a few more opinions on these and I'll update the PR. Thanx for bringing this up @jenlampton 👍

[Graham-72]

'Items to display on each page' 👍

[quicksketch]

PR is looking pretty good, but there is some strange abstraction in place. The PR only updates views_plugin_display::get_pager_text() and there is another implementation of this in views_plugin_display_page! I see what was intended, if it's a page display, then the text says, "Items per page" and any other kind of display (such as a block), it says "Items to display". The intention perhaps was that you shouldn't use the word "page" unless you're looking a full page.

But I don't think that's really a valid desire, using the word "page" when you're talking about a "pager"... it seems as though you'd be referring to the items within the list as a "page", not the overall page that contains the list.

So I think the terminology fixes are great. In implementation, I think we should use this new set of descriptions regardless of whether it's a page or a block display (or anything else). In which case, we can eliminate the get_pager_text() method entirely (or deprecate it) and put the text directly in the options_form() methods. Then we won't have to have the nested IF statements in get_pager_text().

[klonos]

@quicksketch it's been some days since I last worked on this, so a first read of your comment left me with a slight headache 😄 ...but that's entirely my fault (sleep deprivation and burn out) and the because of the (perhaps unnecessary as you say) complexity of the logic in this bit of the code.

I'll have another go at it (I do love code and UI clean-ups 😄), but before I do, I would like some input on lingo used:

• "on each" vs "per"
• "items to skip" vs "offset"

[klonos]

...also noticed some other inconsistencies in the help text here and there. I'll do some thinking, but lets start with this:

"Display all items that this view might find" and "Display a limited number items that this view might find."

• There's a missing "of" between "number" and "items" in the second variation of this text (for pager-some)
• Although "view items" is technically correct, "view results" sounds more user-friendly to me.
• I'm not sure I like the use of the words "limited" and "might" in this sentence, as it be might implied that there is a limitation in the capacity to return proper results (maybe I'm just interpreting it wrongly though, as English is not my main language). How about this instead:

"Display all results of this view." and "Display a portion of the results of this view."

[jenlampton]

"on each" vs "per"

I like"on each" :)

"items to skip" vs "offset"

I like "items to skip", or maybe just "skip"

"Display all items that this view might find" and "Display a limited number items that this view might find."

I don't mind this language (except for the missing "of" I mind that...) but I would like to remove the text "that this view might find". Like this: Display all items and Display a limited number of items.

I also like your idea of switching to results... Display all results and Display a limited number of results.

[ghost]

I've made my own PR for this issue: https://github.com/backdrop/backdrop/pull/4345

It's based on @klonos' PR but with the following changes:

• Re-titles each pager type for consistency
• Changes 'item(s)' to 'result(s)'
• Keeps the term "this display's" in the pager link description (since all the other links also use that term)
• Changes the way the if statement works in get_pager_text() to make it simpler, and uses shorter variable names
• Removes get_pager_text() from views_plugin_display_page.inc so it just inherits the parent text (rather than duplicating it)
• Use consistent language with the summary titles, and formats the code a bit nicer
• Keep the existing "Skip @skip" language (instead of "Skip the first @skip items") to avoid needing to use another format_plural()
• Properly fix the grammar of the full pager's exposed_form_validate()
• Remove summary_title() from the mini pager so it just inherits the one from the full pager (avoiding duplication)

And here are some non-exhaustive screenshots showing changes:

[bugfolder]

@BWPanda, there are automated test failings (same in all 3 PHP versions) in testStorePagerSettings(); it appears the new wording needs to be included in the tests. Can you update the PR?