Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#27055 closed task (blessed) (fixed)

Themes Install Screen: introduce THX (Backbone powered) UX

Reported by: matveb's profile matveb Owned by:
Milestone: 3.9 Priority: high
Severity: major Version: 3.9
Component: Themes Keywords: needs-patch
Focuses: administration Cc:

Description

Start working on porting the work from THX (in core since 3.8) to the install screen. This time, let's try to work early on in tickets so when/if merge comes we are better prepared.

First steps are to reuse the JS from theme.js with minimal changes to support the admin screen. We'll probably discover improvements to make to the core JS. Once the UX is in place let's work on adding whatever missing functionality.

Attachments (53)

27055.1.diff (16.4 KB) - added by matveb 10 years ago.
Prototype using theme.js Backbone code and UI design
27055.2.diff (20.9 KB) - added by matveb 10 years ago.
27055.3.diff (25.0 KB) - added by matveb 10 years ago.
27055.4.diff (25.8 KB) - added by matveb 10 years ago.
27055.5.diff (29.0 KB) - added by matveb 10 years ago.
27055.6.diff (29.4 KB) - added by matveb 10 years ago.
27055.7.diff (29.6 KB) - added by matveb 10 years ago.
theme-install-details.png (496.0 KB) - added by matveb 10 years ago.
27055.8.diff (29.8 KB) - added by matveb 10 years ago.
Making ajax request a bit more robust.
27055.9.diff (31.5 KB) - added by gcorne 10 years ago.
27055.context.patch (1.6 KB) - added by SergeyBiryukov 10 years ago.
add-theme-search-clobber.png (535.2 KB) - added by bpetty 10 years ago.
Search box clobbers other UI
Screen Shot 2014-03-13 at 17.44.52.png (12.2 KB) - added by iseulde 10 years ago.
Screen Shot 2014-03-13 at 19.59.31.png (9.6 KB) - added by iseulde 10 years ago.
27055.10.diff (3.7 KB) - added by matveb 10 years ago.
27055.11.diff (4.4 KB) - added by matveb 10 years ago.
Includes fix for More Filters state highlight
27055.12.diff (6.3 KB) - added by matveb 10 years ago.
27055.13.diff (8.0 KB) - added by matveb 10 years ago.
27055.14.diff (8.2 KB) - added by matveb 10 years ago.
Fix theme-count changing the left padding of the filters bar
Screen Shot 2014-03-27 at 12.53.45.png (512.8 KB) - added by iseulde 10 years ago.
27055.15.diff (7.9 KB) - added by matveb 10 years ago.
Local cache for queries
27055.16.diff (9.3 KB) - added by matveb 10 years ago.
Adds API pagination support
27055.17.diff (9.5 KB) - added by matveb 10 years ago.
27055.18.diff (10.0 KB) - added by matveb 10 years ago.
Fix count
27055.19.diff (1.3 KB) - added by matveb 10 years ago.
27055.20.diff (3.4 KB) - added by matveb 10 years ago.
27055.21.diff (5.1 KB) - added by matveb 10 years ago.
27055.22.diff (6.6 KB) - added by matveb 10 years ago.
27055.23.diff (8.3 KB) - added by matveb 10 years ago.
27055.24.diff (8.7 KB) - added by matveb 10 years ago.
27055.25.diff (9.4 KB) - added by matveb 10 years ago.
27055.26.diff (9.8 KB) - added by matveb 10 years ago.
27055.27.diff (1.4 KB) - added by matveb 10 years ago.
27055.28.diff (1.6 KB) - added by matveb 10 years ago.
27055.29.diff (3.4 KB) - added by matveb 10 years ago.
27055.30.diff (4.5 KB) - added by matveb 10 years ago.
Fixes count in some cases
27055.30.2.diff (4.5 KB) - added by matveb 10 years ago.
27055.31.diff (4.4 KB) - added by matveb 10 years ago.
Fix count in some cases
27055.32.diff (8.6 KB) - added by matveb 10 years ago.
27055.33.diff (8.7 KB) - added by matveb 10 years ago.
27055.diff (4.0 KB) - added by jorbin 10 years ago.
27055.34.diff (1.9 KB) - added by nacin 10 years ago.
Hacked in already-installed. Not done properly, just getting the ball rolling.
27055.35.diff (2.5 KB) - added by nacin 10 years ago.
Multi-column 'Features' feature filters. Hacky. Requires API changes.
27055.36.diff (1.9 KB) - added by matveb 10 years ago.
already-installed.png (97.2 KB) - added by matveb 10 years ago.
27055.37.diff (6.4 KB) - added by matveb 10 years ago.
27055.38.diff (6.7 KB) - added by matveb 10 years ago.
27055.38.2.diff (6.4 KB) - added by adamsilverstein 10 years ago.
add back Install button in detail view when theme not installed already
27055.39.diff (7.6 KB) - added by matveb 10 years ago.
27521.diff (910 bytes) - added by adamsilverstein 10 years ago.
allow previewing of installed themes
27055.40.diff (910 bytes) - added by adamsilverstein 10 years ago.
allow previewing of installed themes
27055.41.diff (716 bytes) - added by gcorne 10 years ago.
27055.42.diff (689 bytes) - added by gcorne 10 years ago.

Change History (150)

@matveb
10 years ago

Prototype using theme.js Backbone code and UI design

#1 @matveb
10 years ago

First patch sets up the app structure in theme.js and the relevant data to handle in Backbone. The code weight to make the JS work is minimal, with no hacks of the main themes.php JS — it just extends the relevant views and includes specific template files to theme-install.php.

Note: this.searchContainer can be separated into an atomic ticket if desired, it makes it so that choosing where to put the search becomes trivial.

Reference of how it currently looks: https://cloudup.com/cttZQAbl4OJ

Sorting (featured, popular, new) are working, filters are not. Consider this a first prototype.

#2 @SergeyBiryukov
10 years ago

  • Component changed from Appearance to Themes
  • Focuses administration added

#3 @helen
10 years ago

You'll want to refresh the patch - I split up wp-admin.css, so hopefully everything (or close to) done here as far as CSS goes should be in wp-admin/css/themes.css. The more of that file you can get rid of, the happier I will be :)

@matveb
10 years ago

#4 @matveb
10 years ago

Updated patch includes:

  • Moved CSS to css/themes.css.
  • Added search functionality from api.wordpress.org/themes and a general purpose Ajax request object for accessing the API. The data is then passed to the main Collection.
  • Search supports an author:query input to get all the results from a theme author — idea here could be to use the author field from the detailed view to link to ?search=author:query giving users a quick way to browse themes by a given author.
  • Improved styles. (Ratings with half stars using dashicons.)
  • Added 'upload theme' section bound to the button.

Next up — hook our Collection pagination with API pagination so we have infinite scroll. Finish the filters section. Add GET actions for installing/preview, etc.

Last edited 10 years ago by matveb (previous) (diff)

@matveb
10 years ago

#5 @matveb
10 years ago

Patch .3:

  • Unified sorts and top-filters appearance for a cleaner interface.
  • Filter queries now work: example Photography and Responsive at top level.
  • Theme results added to the Collection means IS works by default when you return >~ 20 themes, as well as the overlay and arrow navigation.
  • Adds a "More" link to display more filters.
  • Adds Model action attributes for Install and Preview.
  • Installing a theme now works fine, but changes check_admin_referer in update.php.
  • ?theme=undefined when opening overlay is fixed.
  • Adds theme count.
  • Adds error message when Ajax request fails.
  • Uses correct spinner with span + class.

To-do and problems:

  • Make preview action work.
  • Upload Button event sometimes fails.
  • Ajax request is failing intermittently — add better error handling and try again.
  • Clean up error log once a new request is initialized.
  • Add tag:query support for search input.
  • Add the missing filters to "more" section.

@matveb
10 years ago

#6 @matveb
10 years ago

Updated patch to play nicely with r27268.

  • Adds new routes for install screen, including ?sort=sortBy.
  • Makes routing events system extendable by providing an extraRoutes function.

@matveb
10 years ago

#7 @matveb
10 years ago

Patch .5:

  • Adds Preview functionality as a new themes.view.Preview Backbone view with its own template.

@matveb
10 years ago

#8 @matveb
10 years ago

Patch .6 notes:

  • Show how many votes contributed to a theme rating.
  • Display tags on the templates when we have them.
  • Bump amount of themes that are bootstrapped.

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


10 years ago

@matveb
10 years ago

#10 @matveb
10 years ago

Patch .7:

  • Cleans up some interface (error messages stacking) elements on failed and success Ajax requests.
  • Clean up some unused CSS.

Debug:

  • Some events seem to be firing twice — which explains some wonkiness when searching (a delayed stack applies an old reset on the collection) and may also explain some of the intermittent Ajax request failures.

#11 @matveb
10 years ago

Posting image as reference of what details we are currently displaying on theme overlays.

@matveb
10 years ago

Making ajax request a bit more robust.

This ticket was mentioned in IRC in #wordpress-dev by gcorne. View the logs.


10 years ago

@gcorne
10 years ago

#13 @gcorne
10 years ago

@matveb, I really enjoy the improved design!

Took a quick pass in 27055.9.diff to tighten up the javascript and make a few improvements.

  • Removed the unused 'tag' field from the query. Doing so dramatically sped up the API calls (going from 2-7 seconds to more like 280 ms).
  • Switched to https for the client-side API calls
  • Switched to inheritance, new objects, or lightly adding new methods to existing constructors instead of overwriting methods/properties on-the-fly
  • Fixed issue where infinite scroll wasn't working in some cases such as the "latest" and "popular" filters
  • Made a jshint pass.
  • Fixed up the router a bit to be more inline with how themes.php works.

While making this pass, I came up with some things that I think still need to be done:

  • Use the same treatment for themes that are already installed as themes.php
  • Finish up the "More" filters
  • Carefully think through what data coming from the API needs to be escaped.

#14 @nacin
10 years ago

In 27499:

Bring the theme browsing experience from 3.8 to the theme installer. First pass.

props matveb with assists from me and gcorne.
see #27055.

#15 @nacin
10 years ago

  • Milestone changed from Awaiting Review to 3.9
  • Type changed from enhancement to task (blessed)

#16 @SergeyBiryukov
10 years ago

Getting a notice on Add Themes screen:

Notice: Undefined variable: paged in wp-admin/theme-install.php on line 148

$paged was previously set in WP_Theme_Install_List_Table::prepare_items():
tags/3.8.1/src/wp-admin/includes/class-wp-theme-install-list-table.php#L18.

#17 @SergeyBiryukov
10 years ago

We now use "Latest" string in two different contexts. The first instance is in update_right_now_message().

We need to split these strings for proper i18n, see 27055.context.patch.

#18 @SergeyBiryukov
10 years ago

Also, some filters lack i18n (probably because the section is not finished yet):
trunk/src/wp-admin/theme-install.php?rev=27499#L119

#19 follow-up: @melchoyce
10 years ago

I have a couple comments/observations on this — should I leave them here, or make a new ticket?

#20 @matveb
10 years ago

@gcorne Nice improvements. A couple things:

  • It seems we lost the bootstrapped data for the sorting views (featured, popular, newest).
  • The ?theme route opens the preview view instead of the details overlay. It seems that was intended with "Details and Preview" but that prevents us from using these views, which are way faster for browsing than loading the preview, and benefits from the arrow navigation. It's also more consistent with the way themes.php work. I would keep the preview as the secondary action.

#21 in reply to: ↑ 19 ; follow-up: @matveb
10 years ago

Replying to melchoyce:

I have a couple comments/observations on this — should I leave them here, or make a new ticket?

Here is fine! This is our first pass.

#22 in reply to: ↑ 21 ; follow-up: @melchoyce
10 years ago

Replying to matveb:

Here is fine! This is our first pass.

Great!

I keep getting a weird loading issue on the preview screen: https://cloudup.com/ccY16iSwtde Here's a video I took of myself using it, you can see it pop up around halfway through: https://cloudup.com/cB0POiHuCjo

Also noticed that when you open, then close, the "more" filter option, it stays highlighted: https://cloudup.com/cSXrjzHrBdY (Since it also removes the underline on the filter you're currently viewing when you open the menu, there's no way to know which filter you're viewing after you close the menu.)

Lastly, it looks like both the "install" and "preview" buttons bring up the preview panel. This isn't what I'd expect to happen when I click "install".

#23 in reply to: ↑ 22 @matveb
10 years ago

Replying to melchoyce:

Lastly, it looks like both the "install" and "preview" buttons bring up the preview panel. This isn't what I'd expect to happen when I click "install".

Yes, this got wonky. Clicking on the theme should open the details-view (like themes.php), "Install" should install (like "Activate" in themes.php) and "Preview" should open the preview-view (like "Customize" and "Preview" in themes.php).

#24 @bpetty
10 years ago

The "upload theme" screen needs to be tacked onto the history stack, and properly closed on hitting back.

@bpetty
10 years ago

Search box clobbers other UI

#25 @ocean90
10 years ago

  • Keywords needs-patch added

Since we currently don't plan a no-js version we should hide most parts of the screen and make at least ZIP upload work.

#26 @kirasong
10 years ago

Couple things:
First, the number on the screen -- while comparable to the number on the Themes screen -- seems slightly confusing at first visit. When I saw it for the first time, especially when it pops from 0 to 7 -- my first thought was "what is this number that is changing in the upper left?"

Do we need the count (especially at that size) -- at the top of the screen? Does it matter to users how many total there are in the result, since it's infinite scroll anyway? For me, it'd mostly just matter to know when I hit the end of the results. Presuming we keep the count, it might help to wait to set it until we know the initial value.

Second, with routing: If you visit any of the sections where there are more than one "page" worth of results, then preview, and hit back -- the address changes, but the browser doesn't change your view and close the preview. If you visit more than one preview, then exit, you have to hit back multiple times to get back to the "Installed Themes" view. I haven't had a chance to track it down entirely yet, but if I do, I'll post back with a patch.

#27 @iseulde
10 years ago

Just wondering, but why not use the standard tabs (screenshot above)?

#28 @iseulde
10 years ago

And, I think, buttons next to the heading should be styled as above.

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


10 years ago

#30 @nacin
10 years ago

In 27610:

Context for theme install strings. Untranslate an error message until it can be cleaned up.

see #27453, #27055.

#31 @nacin
10 years ago

In 27614:

Use an ellipsis instead of ...

see #27453, #27055.

#32 @nacin
10 years ago

In 27620:

More translation cleanups.

Affects widgets (see #27112), custom headers (see #21785), theme installer (see #27055, reverts [27614]), and some media stuff. Untranslates some complicated strings that need additional study.

see #27453.

@matveb
10 years ago

#33 @matveb
10 years ago

Patch above:

  • Make the "upload theme" button look consistent with the rest of the admin and themes.php itself.
  • Add a route listener for ?upload, push ?upload to the url stack.
  • Improve responsive design around filters section — collapse search.

Thanks for all the feedback so far.

@matveb
10 years ago

Includes fix for More Filters state highlight

@matveb
10 years ago

@matveb
10 years ago

#34 @matveb
10 years ago

Patch 12 and 13:

  • Render missing filters when you click "more".
  • Hook those filters to the Backbone app as an array of tags passed to query the API.

@matveb
10 years ago

Fix theme-count changing the left padding of the filters bar

#35 @nacin
10 years ago

In 27636:

Theme installer:

  • Restore the feature filter.
  • Improve responsiveness.
  • Router updates, fixes.
  • Make "Upload Theme" button more consistent with the admin.
  • Avoid theme-count causing filters to jump.

props matveb.
see #27055.

#36 @nacin
10 years ago

[27636] lands matveb's work up to now.

These are the related things I'm seeing:

  • I temporarily hid "Photography" and "Responsive" and renamed "More" to "Feature Filter" (old name) as things got a bit buggy when you were on one of those filters and then tried to tick some checkboxes. I had no idea, as a user, what was actually being queried. As it is, one of the "sort" tabs remain selected even after some features. I think for starters, the more/filter button needs to change color (like a blue) when a filter is active. Maybe there needs to be a "clear". Or clicking a "sort" should clear it. More work is needed there.
  • Filters, like a search and a "sort" (which should probably be "browse") needs some kind of replaceState, I gather. Not a huge thing, but they had proper links in 3.8.

I've also talked to matveb and worked out an additional list of issues:

  • Performance:
    • Pagination: Right now infinite scroll only uses the data that was fetched, as it works on the browsing page — but API calls are limited to a certain per page, otherwise it's a ton of data. If we downloaded 108 at a time, we could get through three renders of 36 before we'd have to do another XHR.
    • Caching of collections (as noted throughout the JS already). We should avoid hitting the WP.org API whenever possible. Any search should be cached as a collection to be re-played later. See also wp.media.model.Query.get for super-basic caching.
    • Improved search debouncing (and/or don't trigger a search <= 3 characters without an "enter").
  • Preview pane:
    • Rework the preview pane to use the details code (with the preview template), which should allow for prev/next to also be added. In IRC last week, I suggested '5 of 48' with arrows could work well; we also mused about a spinner while the preview loaded, and even showing the screenshot in bigger form in that space while the preview loaded.
    • Consider how the preview overlay works on mobile devices. We could also have the responsiveness of the preview pane just be that the iframe is hidden. That said I imagine iPad landscape would look really sweet with the preview pane.
  • Properly represent a currently installed theme (no Install buttons, etc.)

#37 @nacin
10 years ago

In 27646:

Add context to 'Browse' (themes). see #27055, #27453.

#38 @jorbin
10 years ago

We are cutting off some words on the theme detail page. Example:

https://i.cloudup.com/HPnEzpetSc.png

This is on OSX using Chrome 33.

#39 @jorbin
10 years ago

I've opened up a couple of related tickets.

#27522 is for the fact that colors aren't
#27521 is for the fact that the page is currently completely unusable via a keyboard

Some other issues that I've found on the theme detail preview pane:

The screenshot needs an alt tag. Possible we should use "Screenshot of <theme>"?
The star ratings give no information to screen readers.

#40 @kirasong
10 years ago

I was testing this and noticed that when the filter panel is open, and you're manipulating the checkboxes, it's not apparent you're getting search results in realtime when on a ~13" display, because none of the theme results are visible.

http://screen.getsource.net/03-26-2014-13-09-30.png

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


10 years ago

#42 follow-up: @samuelsidler
10 years ago

I just ran through the themes install screen a bit. Here's some thoughts:

  • Bug: If you click around on the tabs (featured, popular, latest) a bunch and end on "featured" but immediately scroll down fast, it'll show you a different tab. I can get it to show me the contents of the "popular" tab this way, quite easily.
  • Bug: The max is always 36. No matter what, we max out at 36. Are there only 36 single column themes in the directory?
  • Feature Filter: The entire behavior of the feature filter is strange. What is it filtering, everything or the currently selected tab? Is it actively filtering or do I need to close it somehow? If I close it by clicking on it again (non-obvious), what am I viewing: the tab that's selected or the filtered content? (Note: This probably works fine on large screens, but not on small, 13 inch ones.)
  • If the feature filter is open and you search for something, what's the expected behavior? Right now, the search term takes over. Can you use the feature filter in conjunction with search? *Should* you be able to?

A few more thoughts: The auto-updating number is weird *because* it updates, not in spite of it. I'd prefer if the tabs started the section and underneath it said something along the lines of "7 featured themes found" underneath the white tab bar. You could then use that same text to show the "35 themes found tagged with Brown and One Column" for when the feature filter wasn't showing.

I also tend to think the Feature Filter should be made a tab and renamed to something along the lines of "Filter All Themes" since it's not filtering the currently-selected tab. That'd take care of some of the issues mentioned above, but leave others.

#43 follow-up: @iseulde
10 years ago

I'm also still wondering why these tabs look like the main navigation on Twitter... Why not use the tabs we already have?
In themes.php, the search bar and count are next to the page title. Why not put them there as well?

#44 @ocean90
10 years ago

  • Priority changed from normal to high
  • Severity changed from normal to major

In IE8 and IE9 no themes are loaded/displayed: https://cloudup.com/csrm3qMhQ5n

#45 @samuelsidler
10 years ago

It would also be nice if the search box said "Search Theme Directory..." instead of "Search Themes..." since it's not clear what it's searching. That's not really a change from 3.8, but the Add New Plugin page actually explains what the plugin directory is, unlike the Add New Theme page.

#46 follow-ups: @sonjanyc
10 years ago

I took a stab at what an improved theme filter UI could look like.

Generally I feel the filters shouldn't be inline with the other tabs since it filters the other tabs. That causes confusion.
Mike already mentioned that the large filter panel pushes the results down so the user doesn't even see that anything is happening, so it would be best to condense the filter options as much as possible.

Default state: where the filter options are moved down below the tabs
http://new.tinygrab.com/00a07a89c6657457ce0bbea4633930bc9106fff6b8.jpg

Filters are tugged away in dropdowns:
http://new.tinygrab.com/00a07a89c6b79f158e89423317668129ab2a6fa70e.jpg

Once a filter has been selected it gets listed below filter set so user gets feedback what the results are filtered by and user can remove individual filters by clicking on the "x"
http://new.tinygrab.com/00a07a89c696c05581114249c0283db482b42b9723.jpg

Obviously just a first draft. Looking forward to comments.

#47 in reply to: ↑ 46 ; follow-ups: @melchoyce
10 years ago

Replying to sonjanyc:

I took a stab at what an improved theme filter UI could look like.
Obviously just a first draft. Looking forward to comments.

Just as a quick note: a sketch, or some written ideas about how you see this working at smaller screen sizes, would be super helpful. :)

#48 in reply to: ↑ 46 @jorbin
10 years ago

Replying to sonjanyc:

Generally I feel the filters shouldn't be inline with the other tabs since it filters the other tabs.

Actually, it doesn't filter the other tabs. It does a search based off the filters chosen.

#49 in reply to: ↑ 47 @sonjanyc
10 years ago

Replying to melchoyce:

Good point. I heard accordion UI was mentioned. I like that a lot for mobile actually. See mockups below.
Accordion on full web would push down the results a lot if each accordion item would be full width. Something to think about. Maybe they can be in multiple columns.

http://new.tinygrab.com/00a07a89c671a06c33783d2d91fbbf136fdb258ec7.jpg

Question is if we should indicate that something is selected in each accordion by changing the color of it?

#50 in reply to: ↑ 47 @sonjanyc
10 years ago

Replying to melchoyce:
I quickly mocked up what the proposed mobile accordion UI could look like in a multi-column layout on the full web.
Thoughts?

http://new.tinygrab.com/00a07a89c6083c28f237384c8d08c496b2aea1076e.jpg

@matveb
10 years ago

Local cache for queries

#51 @matveb
10 years ago

Patch above abstracts the API querying into a couple new Collection methods, which together handle caching for all the requests. (Clicking back and forth between Featured, Popular, Latest is instant after the first time and doesn't hammer the servers.)

All requests (filters, sorts, searches) go through this method, which greatly reduces code repetition.

It also adds separates presentation from the Collection logic via events so that any view updates we do (spinners, etc) happen on the views.

Also starts building a theme:end event handler to query for more themes via the scroll bound functionality, so we can query for more themes once the user has scrolled far enough. (This has a bit of added complexity since we already have pure lazy-render pagination on the collection already, but it should work fine with the two together, giving us a lot of performance stability.)

#52 in reply to: ↑ 42 @matveb
10 years ago

Replying to samuelsidler:

  • Bug: If you click around on the tabs (featured, popular, latest) a bunch and end on "featured" but immediately scroll down fast, it'll show you a different tab. I can get it to show me the contents of the "popular" tab this way, quite easily.
  • Bug: The max is always 36. No matter what, we max out at 36. Are there only 36 single column themes in the directory?

These two problems will be solved with the debouncing and pagination items. Now that caching is done I'll work on those next.

#53 in reply to: ↑ 43 @matveb
10 years ago

Replying to avryl:

I'm also still wondering why these tabs look like the main navigation on Twitter... Why not use the tabs we already have?

A couple thoughts behind this. The tabs you mention already convey a page-reload feeling of slowness since they are used in that way across the admin (as far as I know). I also think they feel outdated in the context of mp6. I think we can do better, specially for any single-page section we work on. My proposal borrows from the new "white boxes" we use in Dashboard and elsewhere extensively. The thick borders are also used on the left side for alerts and plugin pages. Of course, this can — and should — keep improving!

Disclaimer: I actually worked on the tabs you mention back in the day as one of my first contributions :)

@matveb
10 years ago

Adds API pagination support

@matveb
10 years ago

#54 @matveb
10 years ago

Patch 17:

  • Adds pagination support and bumps initial request to 72 themes from the API. The way it works is we fire a theme:end event based on the scroll listener when we have no more themes. Then we use that event to send new collection.query requests with a counter for the API page.
  • We differentiate between requests by an internal currentQuery object. If a given query matches that we then figure out if it's a cached request or a paginated new query.
  • Once we get more themes we add them to the collection, so the internal collection pagination works just as fine.
  • Also added a debounce to the search query trigger that makes searches quite more solid.

Missing:

  • Update counter.
  • Avoid bumping the api page param until we get a successful request. Right now if you scroll super fast you can jump ahead in pagination.

@matveb
10 years ago

Fix count

#55 @nacin
10 years ago

In 27830:

Theme Installer: Caching and paginating of API requests.

props matveb.
see #27055.

@matveb
10 years ago

#56 @matveb
10 years ago

Patch above fixes a race condition where scrolling to the end would stack page requests numbers twice or thrice depending on how you scrolled.

#57 @nacin
10 years ago

In 27843:

Theme Installer: Remove unused variables and undesired commas.

see #27055.

#58 @nacin
10 years ago

In 27845:

Theme Installer: Avoid race condition during pagination.

props matveb.
see #27055.

@matveb
10 years ago

#59 @matveb
10 years ago

Patch 20:

First pass at an improved more-filters section. Filtering is triggered by clicking a button instead of just clicking the checkboxes.

@matveb
10 years ago

#60 @matveb
10 years ago

Updated patch 21:

Adds a way to clear all filters. When no filters are selected it doesn't display any number (was 0 before).

@matveb
10 years ago

@matveb
10 years ago

#61 @matveb
10 years ago

Patch 23:

  • Add no-results message when query returns empty.
  • If you press the "All" button and the filters sections opened && there are checked filters, perform a query instead of closing.
  • List tags by name rather than slug.
  • Hide theme browser section when the filters are opened.
  • Style updates.

@matveb
10 years ago

@matveb
10 years ago

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


10 years ago

@matveb
10 years ago

#63 @nacin
10 years ago

In 27896:

Theme Installer fixes:

  • Better more filters section. props sonjanyc for some mockups.
  • Better handling of no results.
  • Use # for hrefs.

props matveb.
see #27055.

@matveb
10 years ago

#64 @matveb
10 years ago

Last commit also fixes the cut off bug https://i.cloudup.com/HPnEzpetSc.png

@matveb
10 years ago

#65 @matveb
10 years ago

Adds small fix for making sure sorting don't remain marked as current on the DOM, not only cosmetically.

@matveb
10 years ago

#66 @matveb
10 years ago

Patch 29 adds link to go back to expanded filters view after a filter search.

@matveb
10 years ago

Fixes count in some cases

@matveb
10 years ago

@matveb
10 years ago

Fix count in some cases

#67 @matveb
10 years ago

Latest patch 31 fixes a counting bug @nacin reported when you hit a cached result for a query that had more results than what the collection had collected.

@matveb
10 years ago

#68 @matveb
10 years ago

Patch 32 combines all the above plus:

  • Previous/Next buttons to browse themes *while* on the Preview mode. The sidebar updates immediately, while the preview has the delay that comes with reloading the iframe.
  • Includes the keyboard improvements from #27521.
  • Style improvements.

#69 @jorbin
10 years ago

Tested patch 32 and have a couple of small notes:

1) We should restrict keyboard navigation to the preview overlay. Otherwise it is too easy to get lost.
2) When you collapse the sidebar, we should make sure that the links inside of it are no longer focusable. Same reason as above. We also might need to add better focus/hover styling since the link to return can get lost on some themes.

#70 @matveb
10 years ago

@jorbin do you mean to constrain it to the sidebar too or allow focus to go into the iframe?

#71 @jorbin
10 years ago

@matveb
Allow it into the iframe as well. I mean to prevent it from going to the other links on the page that are behind the overlay.

@matveb
10 years ago

#72 @matveb
10 years ago

Patch 33: style improvements for preview sidebar navigation, and fixes a loop once you hit the last theme on a collection and hit next.

#73 @nacin
10 years ago

In 27936:

Themes: Hide 'Add New' with no JS. see #27055.

#74 @nacin
10 years ago

In 27937:

Theme Installer: Fix sorting, counts, keyboard navigation; add prev/next to previews.

props matveb.
see #27055.

#75 @nacin
10 years ago

In 27940:

Theme Installer: Center spinner, remove plural string.

fixes #27581. see #27055.

@jorbin
10 years ago

#76 follow-up: @jorbin
10 years ago

Right now, the experience is completely broken in IE 7,8 and 9 (at least). This is do to the cross domain request for the themes failing.

The above patch uses admin-ajax.php to proxy the request so it is not cross domain. The proxy is very simple (there is a nonce check, but that is about it). If this is the route we want to go, I can flush it out.

#77 in reply to: ↑ 76 @nacin
10 years ago

Replying to jorbin:

Right now, the experience is completely broken in IE 7,8 and 9 (at least). This is do to the cross domain request for the themes failing.

The above patch uses admin-ajax.php to proxy the request so it is not cross domain. The proxy is very simple (there is a nonce check, but that is about it). If this is the route we want to go, I can flush it out.

Let's consolidate this on #27639.

#78 @nacin
10 years ago

In 27963:

Theme Installer: Separate API from an event handler to avoid issues where a false return value stops the event.

see #27055.

@nacin
10 years ago

Hacked in already-installed. Not done properly, just getting the ball rolling.

#79 @kirasong
10 years ago

I'm excited; this is looking good, and much more ready to go.

Two things:

First, it's not clear what search is searching if you type something in, hit enter, then choose another tab or filter.

Especially with filters, it's not obvious if you're also searching or not, so we might want to clear the search field, show an indicator like the "filters" on top of the theme view with the current search, or an indicator under the search field. I'm not sure what's best here, so any suggestions are welcome.

Second, a small note, just so it's tracked somewhere. I just got more than one "Details & Preview" overlay to show:
http://screen.getsource.net/04-06-2014-01-12-29.png

Click on one of the themes to preview; close the preview. Then, mouse-over another theme.

@nacin
10 years ago

Multi-column 'Features' feature filters. Hacky. Requires API changes.

@matveb
10 years ago

#80 @matveb
10 years ago

Patch 36:

  • Clicking Filters tab clears search and clears route.
  • Searching sets ?search= route.
  • Merges patch to allow next/prev navigation to query for more themes.

#81 @matveb
10 years ago

I propose the above for showing already installed themes.

@matveb
10 years ago

#82 @matveb
10 years ago

Patch 37 combines 36 with 34:

  • Show a prominent "already installed" banner using the same design as the "update available".
  • Set the installed attribute as part of the model logic, not the template.
  • Disable all click actions for theme in grid.
  • If you still access the preview overlay by using the next/prev links then the install button is disabled.

Update:

  • Also fixes issue with "Install" button going to Preview instead of installing directly.
Last edited 10 years ago by matveb (previous) (diff)

@matveb
10 years ago

@adamsilverstein
10 years ago

add back Install button in detail view when theme not installed already

#83 follow-up: @adamsilverstein
10 years ago

Oops, posted that patch before refreshing; Anyway, in 27055.37.diff I built on 27055.37.diff and added back the Install button in the detail view when the theme is not already installed (it was missing).

#84 in reply to: ↑ 83 @matveb
10 years ago

Replying to adamsilverstein:

Oops, posted that patch before refreshing; Anyway, in 27055.37.diff I built on 27055.37.diff and added back the Install button in the detail view when the theme is not already installed (it was missing).

Oops, thanks :)

@matveb
10 years ago

#85 follow-up: @nacin
10 years ago

In 28025:

Theme Installer: Handle currently installed themes, add search route, let prev/next refresh collections.

props matveb.
see #27055. fixes #27695.

#86 in reply to: ↑ 85 ; follow-up: @adamsilverstein
10 years ago

Looks good: one thing I noticed is that when a theme is already installed you can no longer preview the theme from the installer; This may be intentional, it seems a little odd given that I can still navigate to an already installed theme in the preview window using the next/back buttons. Perhaps we should consider allowing the preview button for already installed themes?

Replying to nacin:

In 28025:

Theme Installer: Handle currently installed themes, add search route, let prev/next refresh collections.

props matveb.
see #27055. fixes #27695.

#87 in reply to: ↑ 86 ; follow-up: @nacin
10 years ago

Replying to adamsilverstein:

Perhaps we should consider allowing the preview button for already installed themes?

Yes, I'm fine with that.

#88 @nacin
10 years ago

In 28035:

Theme Installer: Use 'browse' instead of 'sort' for routes.

see #27055.

@adamsilverstein
10 years ago

allow previewing of installed themes

@adamsilverstein
10 years ago

allow previewing of installed themes

#89 in reply to: ↑ 87 @adamsilverstein
10 years ago

Replying to nacin:

Replying to adamsilverstein:

Perhaps we should consider allowing the preview button for already installed themes?

Yes, I'm fine with that.

see 27055.40.diff

#90 @nacin
10 years ago

In 28038:

Theme Installer: Show 'Preview' button for installed themes.

props adamsilverstein.
see #27055, see #27708.

#91 follow-up: @nacin
10 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Calling this fixed! Great job matveb & co. Took a lot of polish! This is a fantastic improvement in 3.9 and I'm really happy how it turned out.

I've opened #27708 for the broken ?theme= route.

All remaining issues should go into a new ticket. (Minor stuff can be dropped onto #27708 if it's still open.) Please make sure I didn't miss anything here; this ticket definitely got unwieldy.

#92 @azaozz
10 years ago

In 28045:

Precommit cleanup for theme.js, editor.css amd wpdialogs/plugin.js, See #27055, #26907, #16284

#93 @ocean90
10 years ago

In 28047:

Theme Installer: Prevent default events for "Upload Theme" and "Browse" buttons.

see #27055.

#94 in reply to: ↑ 91 @matveb
10 years ago

Replying to nacin:

Calling this fixed! Great job matveb & co. Took a lot of polish! This is a fantastic improvement in 3.9 and I'm really happy how it turned out.

Nice, thanks a lot! I'm really happy as well with all the work and collaboration. Also like that we kept momentum and improvements for a feature that was introduced last cycle.

#95 @nacin
10 years ago

In 28126:

Theme Installer: Revert to proxying through PHP for WordPress.org API requests.

This is to ensure we have valid installation nonces, though we've run into this as a problem previously (see #27639, #27581, #27055).

A tad slower, but we gained speed in 3.9 by simplifying the request made to the API.

props ocean90.
fixes #27798.

@gcorne
10 years ago

#96 @nacin
10 years ago

In 28141:

Theme Installer: Fixes for browsers without pushState.

props gcorne.
see #27055.

@gcorne
10 years ago

#97 @nacin
10 years ago

In 28147:

Theme Installer: Properly call router navigation.

props gcorne.
see #27055.

Note: See TracTickets for help on using tickets.