Theme install and search screens should infinitely scroll

[helenyhou]

As per scoping of 3.4, an improvement to the themes screens would be to have them infinitely scroll, especially now that they are floated divs instead of a table.

Pagination would be kept as-is for no-JS users. Loading and perhaps an end of results indicator also needed.

[garyc40]

I'd like to tackle this task if no core devs intend to take over yet? It's been a while since I last contributed something to core.

A question though, this infinite scroll enhancement can only be applied to the theme search screen right? Other screens such as Featured, Newest or Recently Updated don't have pagination, so I doubt wordpress.org has API for those screens.

[helenyhou]

Replying to garyc40:

I'd like to tackle this task if no core devs intend to take over yet?

DH-Shredder and I are teamed up on it (see ​http://wpdevel.wordpress.com/2012/01/11/this-is-not-a-feature-list/) but haven't quite started the dev cycle yet. Not sure how exactly we're approaching labeling who's on what, etc., but I imagine that doesn't exclude participation by any means :)

A question though, this infinite scroll enhancement can only be applied to the theme search screen right?

It also applies to the installed themes screen, which is where we are starting for proof-of-concept.

[jane]

Infinite scroll on the themes screen has already been implemented on wordpress.com, so it may just be a matter of asking for their code and touching it up for core, rather than starting from scratch. Ask Koopersmith for details.

[kirasong]

Replying to jane:

Infinite scroll on the themes screen has already been implemented on wordpress.com, so it may just be a matter of asking for their code and touching it up for core, rather than starting from scratch. Ask Koopersmith for details.

Have a tentative plan on how to implement if it does need to be from scratch, but will touch-base with Koopersmith to find out if that's an option, since it'd certainly speed things up.

[garyc40]

Looking forward to seeing your work on this :)

Several UX things I usually encounter with infinite scrolling in other web sites / apps:

• As the list infinitely expands, the user scrolls farther away from the top. There should be some way for the user to quickly scroll to top to navigate to other pages, or refine their search criteria.

• Let's say the user navigates away from the page. If the user decides to go back by clicking the browser's "Back" button, they should be presented with the previous theme browsing state, rather than with page 1.

• Some sort of indication of the range of items being displayed versus the total number of items should be presented to the user as the next page is preloaded and displayed (e.g. 120 - 145 of 20000 total).

Don't know if all of these should apply to this case, but could be some idea to consider.

[helenyhou]

Replying to garyc40:

There should be some way for the user to quickly scroll to top to navigate to other pages, or refine their search criteria.

Hooray for #18578, then! Will keep the rest of your points in mind as we get on.

[scribu]

Related: #19469

[kirasong]

Initial patch for adding inifinite scrolling to Themes screen (Currently Installed Only)

[kirasong]

Initial patch for adding infinite scrolling to Themes screen (Currently Installed Only).

This is a work in progress.

Enabled fetch-list for class-wp-themes-list-table.

Ported wp-includes/js/wplink.dev.js, which handles infinite scrolling for internal linking to wp-themes.dev.js for use in the Themes page.

Some things that are weird/known quirks:

• There aren't any UI elements (i.e. spinners) to show that we're loading, but there are hooks in place to do this once the elements exist.

• Labels are not modified based on "how many" we've loaded or how many are left. This means the paging always looks the same.

• As soon as you've loaded all themes, it just doesn't load anymore. This is easy to hook into whatever we want to do when we're out of items.

• For now at least, most of parseQuery was copied from tb_parseQuery (thickbox.js) into our JS for now, since making the change to thickbox.js would change how the thickbox handles query strings (whether more correct or not).

• The parsing of the features[] query array is a bit hackish, in part due to how themes varies on how it includes it in the query string. Suggestions for better methods of parsing are welcome.

• Yes, there is still some debug code, because it's a work in progress. This will be removed before final, of course, is marked FIXME, and should be obvious.

[helenyhou]

Enables use on theme-install.php

[helenyhou]

19815.4.diff​ gets it working on theme-install.php.

Some other issues:

• Details do not work on Ajax-loaded items (needs delegation).
• Initial search by filters is done via POST and so is currently not sending with the Ajax request.

[mikeschinkel]

Will inifinite scrolling be the only option? Personally I find infinite scrolling very disconcerting and would turn off where I can. JMTCW

[scribu]

In another case of "let's do it because it looks cool", there doesn't seem to be any UX rationale for this change posted anywhere.

The original idea seems to come from wordpress.com, which implemented infinite scroll on the themes screen. Ironically, it has now dropped this approach, in favor of simply listing all the existing themes on a single page.

One can argue that with infinite scroll, you can see more themes quicker. However, this can be addressed by increasing the number of themes listed per page: #19469

[jane]

[8:12pm] Jane: scribu: it was discussed at core team meetup

[8:12pm] scribu: ok, that doesn't really help

[8:12pm] scribu: it should be in trac or on wpdevel

[8:12pm] Jane: no, it doesn't

[8:12pm] Jane: yes, it should

[8:12pm] Jane: if i wasn't working 2.5 jobs for the salary of one, i would surely have finished collating and posting notes by now

[8:13pm] scribu: can't you get Matt to clone you or something?

[8:13pm] Jane: but this is going to be one of those things where it's a "the majority of leads wanted it"

[8:13pm] Jane: personally, i hate infinite scroll

[8:14pm] Jane: URLS4EVA

[8:14pm] scribu: I thought you were the UX lead

[8:14pm] Jane: yes, but we make decisions as a team

[8:14pm] Jane: and my hatred is personal, not objective

[8:14pm] Jane: which is why i agreed to it on the themes screen only

[8:15pm] Jane: where we could see how it did on a screen where it seemed likely to improve the experience rather than potentially degrade it

[8:15pm] scribu: *sigh*

[8:15pm] helenyhou: Jane: you are in my head. or i am in yours, whichever way.

[8:15pm] Jane: we've been talking about it in dev chats for almost two months now

[8:15pm] Jane: no one raised objections

[8:16pm] helenyhou: i think once all of the details are hidden by default and the grid is more even, it will look much better

[8:16pm] scribu: well, thanks anyway for responding, Jane

[8:16pm] Jane: sure

[SergeyBiryukov]

Replying to helenyhou:

Initial search by filters is done via POST and so is currently not sending with the Ajax request.

#18094 has a patch for switching it to GET. Also related: #18724

[helenyhou]

Hide pagination, move into theme.dev.js

[helenyhou]

19815.5.diff​ moves the JS into wp-admin/js/theme.dev.js on suggestion of rboren, hides .pagination-links, and ups the paging number for themes.php (see #19469). Still needs a loading spinner.

[ryan]

The last item in the original list is duplicated at the beginning of the first scroll fetch. This seems to happen only when using the feature filter and is true even when js is off. I suspect this has always been broken. Given that paging of search results is broken in 3.3, it is hard to tell. Regardless, not a blocker to landing .5.diff.

[ryan]

JS notes from IRC discussion: Throttle the scroll event, or check the position of the window once every x milliseconds. Cache jQuery selectors. Replace delayedCallback with jQuery.Deferred.

[kirasong]

Spinner should function, added additional cleanup and caching window selector.

[kirasong]

19815.6.diff​ adds:

• Additional cleanup.
• Spinner should function.
• Cached window jQuery selector, primarily for maybeLoad -- others look to be cached already.

Still required, and coming next:

• Throttle or timeout for checking of scroll position (per IRC)
• Replace delayedCallback (per IRC)
• Stop call (and spinner) from happening on "no more results"/bottom of page.

[ryan]

In [19887]:

Infinite scroll for themes.php and theme-install.php. Bump per page limit for themes.php to 999. Props helenyhou, DH-Shredder. see #19815

[ryan]

In [19893]:

Infinite scroll for themes.php and theme-install.php. Bump per page limit for themes.php to 999. Props helenyhou, DH-Shredder. see #19815

[ryan]

[19887] was a miscommit of .5.diff. [19893] is .6.diff.

[helenyhou]

Only enqueue on theme-install search tab

[helenyhou]

19815.7.diff​ makes sure the JS is only enqueued when needed on theme-install.php

[ryan]

Why wouldn't I want the other pages to infinite scroll?

[helenyhou]

They only return a limited number of results, and the infinite scroll seems to make them just keep repeating since there's no pagination to tell it to stop loading results.

[kirasong]

In 19815.8.diff​, renamed class to ThemeScroller, caching more jQuery selectors, throttling scroll, using Deferred for AJAX calls, and general cleanup.

Does not include 19815.7.diff​, because this patch only modifies the JS involved.

Eyes on this patch welcome, since there are a significant amount of changes.
Alternate/preferred ways of throttling scroll also welcome!

Still to be done: Don't query to check if we already know that there aren't more items.

[garyc40]

More efficient throttling. Based on 19815.8.diff.

[garyc40]

In 19815.8.diff​, the throttle still doesn't work properly. To test this, simply add console.log('maybeLoad!'); at the beginning of the maybeLoad function. You'll see that it's called every single time the scroll event is triggered. Function call is expensive.

19815.9.diff​ has some improvement on the throttling method:

• Since we're polling every xxx milliseconds anyways, there's practically no need for scroll event handler.
• Threshold is checked right in the polling function, rather than in maybeLoad. This saves a function call every time we're polling.
• scrollThrottleDelay is increased to 500.

[garyc40]

Removed some redundant variables that I added.

[kirasong]

Renamed class to ThemeScroller, caching more JQuery selectors, throttling scroll, using Deferred for AJAX calls, and general cleanup. Does not include 19815.7.diff [edit - get rid of stray propset]

[kirasong]

Combined garyc40's patch, and koopersmith's suggestions from IRC

[kirasong]

Added 19815.10.diff​, which contains 19815.8.diff​ plus garyc40's polling from 19815.9.2.diff​, and koopersmith's suggestions from today's dev chat.

[kirasong]

19815.10.diff, plus Fixed Paging, got rid of extra AJAX call for single page situations and end-of-pages, renamed maybeLoad to ajax, polling now clears when at end of content.

[kirasong]

Added 19815.10.2.diff​: Includes changes from 19815.10.diff​ plus fixed paging cases where paging starts at > 1, got rid of extra AJAX call for single page situations and end-of-content, renamed maybeLoad to ajax, and polling setInterval now clears when at end of content.

[koopersmith]

In [19971]:

Second pass at infinite scroll for themes, including polling, fixed paging, and fewer ajax calls. props DH-Shredder, helenyhou, garyc40. see #19815.

[koopersmith]

I'd like to see a patch that creates the object with the query params in PHP and passes it to JS using json_encode. We'll be able to remove ~50 lines of JS (and the hackiness that is parseQuery).

[kirasong]

Added JSON passing of necessary variables and removed parseQuery. Added $this->features to theme-install list table and removed $term global. Additional error handling.

[kirasong]

Attached 19815.11.diff​

• Added json_encode passing of necessary variables and removed parseQuery per koopersmith's suggestion. In the process, added $this->features to theme-install list table and removed $term global.
• Additional error handling in theme.dev.js
• Search in theme-install list table now runs strtolower on search argument to match the behavior in themes list table.

Eyes welcome on patch for general review or potential complications of the changes.

[nacin]

In [20048]:

In WP_Themes_List_Table, don't perform unnecessary sanitization on search terms or filter features. We only use these for case-insensitive comparison. see #19815.

[kirasong]

Refresh due to nacin's suggestions and updates

[kirasong]

Attached 19815.11.2.diff​:
Refresh of 19815.11.diff​ per nacin's suggestions and updates.

[nacin]

In [20094]:

Output themes and theme-install infinite scrolling args in JS, rather than parsing query strings. props DH-Shredder, helenyhou. Make WP_Theme_Install_List_Table extend WP_Themes_List_Table. Doesn't help much yet, but we should be able to dry things up further. see #19815.

[nacin]

In [20096]:

Use sanitize_key() for theme search tags/features. sanitize_title_with_dashes() is overkill. see #19815.

[nacin]

In [20097]:

Revert [20096] which was an accidental partial revert of [20094]. see #19815.

[nacin]

In [20098]:

Use sanitize_key(). sanitize_title_with_dashes() is more than we need. see #19815. see [20096], [20097].

[nacin]

In [20100]:

Allow WP_List_Table::_js_vars() to take an array of additional args to add. Allows us to have a single variable printing data when child classes need more data. Also, fix compact() call in [20094]. see #19815.

[nacin]

[20100] breaks the JS. koopersmith is cleaning it up and will account for the new variables.

[koopersmith]

In [20104]:

Partially revert [20100]. The list_args JS variable cannot be extended due to its use as an argument in fetch-list. see #19815.

We should reattempt extending the args created in WP_List_Table, but will need to deprecate the current list_args to do so.

Also, infinite scroll on themes pages is no longer broken. Go team.

[kirasong]

Be less paranoid about inputs. Removed some undefined checks per koopersmith. Still checking for initial input arrays.

[kirasong]

19815.12.diff​:
Be less paranoid about inputs. Removed some undefined checks per koopersmith. Still checking for initial input arrays. If/when the array inputs change, more checks can be removed.

[kirasong]

Yet more de-paranoidization

[kirasong]

19815.12.2.diff​:
Removed an additional check, and moved two member variable declarations, since both inputs now assume proper initialization.

[nacin]

In [20242]:

Tidy up sanity checks in theme.dev.js. props DH-Shredder, fixes #19815.

[afercia]

In 41019:

Administration: in WP_List_Table->pagination(), properly concatenate CSS classes for the pagination links when infinite_scroll is set to true.

Props SGr33n.
See #19815.
Fixes #40003.