#19815 closed task (blessed) (fixed)
Theme install and search screens should infinitely scroll
Reported by: | helenyhou | Owned by: | nacin |
---|---|---|---|
Milestone: | 3.4 | Priority: | normal |
Severity: | normal | Version: | 3.3 |
Component: | Themes | Keywords: | |
Focuses: | Cc: |
Description
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.
Attachments (14)
Change History (63)
#5
in reply to:
↑ 4
@
13 years ago
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.
#6
follow-up:
↓ 7
@
13 years ago
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.
#7
in reply to:
↑ 6
@
13 years ago
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.
#8
follow-up:
↓ 9
@
13 years ago
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.
@
13 years ago
Initial patch for adding inifinite scrolling to Themes screen (Currently Installed Only)
#11
@
13 years ago
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.
#12
follow-up:
↓ 18
@
13 years ago
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.
#13
@
13 years ago
Will inifinite scrolling be the only option? Personally I find infinite scrolling very disconcerting and would turn off where I can. JMTCW
#15
@
13 years ago
- Keywords ux-feedback added
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
#16
@
13 years ago
[8:12pm] Jane__: scribu: it was discussed at core team meetup[[BR]] [8:12pm] scribu: ok, that doesn't really help[[BR]] [8:12pm] scribu: it should be in trac or on wpdevel[[BR]] [8:12pm] Jane__: no, it doesn't[[BR]] [8:12pm] Jane__: yes, it should[[BR]] [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[[BR]] [8:13pm] scribu: can't you get Matt to clone you or something? [[BR]] [8:13pm] Jane__: but this is going to be one of those things where it's a "the majority of leads wanted it"[[BR]] [8:13pm] Jane__: personally, i hate infinite scroll[[BR]] [8:14pm] Jane__: URLS4EVA[[BR]] [8:14pm] scribu: I thought you were the UX lead[[BR]] [8:14pm] Jane__: yes, but we make decisions as a team[[BR]] [8:14pm] Jane__: and my hatred is personal, not objective[[BR]] [8:14pm] Jane__: which is why i agreed to it on the themes screen only[[BR]] [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[[BR]] [8:15pm] scribu: *sigh*[[BR]] [8:15pm] helenyhou: Jane__: you are in my head. or i am in yours, whichever way.[[BR]] [8:15pm] Jane__: we've been talking about it in dev chats for almost two months now[[BR]] [8:15pm] Jane__: no one raised objections[[BR]] [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[[BR]] [8:16pm] scribu: well, thanks anyway for responding, Jane__[[BR]] [8:16pm] Jane__: sure
#19
@
13 years ago
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.
#20
@
13 years ago
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.
#21
@
13 years ago
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.
#22
@
13 years ago
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 (per ryan's comment):
- Throttle or timeout for checking of scroll position
- Replace delayedCallback
#27
@
13 years ago
19815.7.diff makes sure the JS is only enqueued when needed on theme-install.php
#29
@
13 years ago
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.
#30
@
13 years ago
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.
#31
@
13 years ago
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.
@
13 years ago
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]
#32
@
13 years ago
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.
@
13 years ago
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.
#33
@
13 years ago
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.
#35
@
13 years ago
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).
@
13 years ago
Added JSON passing of necessary variables and removed parseQuery. Added $this->features to theme-install list table and removed $term global. Additional error handling.
#36
@
13 years ago
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.
#38
@
13 years ago
Attached 19815.11.2.diff:
Refresh of 19815.11.diff per nacin's suggestions and updates.
#44
@
13 years ago
[20100] breaks the JS. koopersmith is cleaning it up and will account for the new variables.
@
13 years ago
Be less paranoid about inputs. Removed some undefined checks per koopersmith. Still checking for initial input arrays.
#46
@
13 years ago
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.
#47
@
13 years ago
19815.12.2.diff:
Removed an additional check, and moved two member variable declarations, since both inputs now assume proper initialization.
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.