#27055 closed task (blessed) (fixed)
Themes Install Screen: introduce THX (Backbone powered) UX
Reported by: | 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)
Change History (150)
#1
@
11 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.
#3
@
11 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 :)
#4
@
11 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.
#5
@
11 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.
#6
@
11 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.
#7
@
11 years ago
Patch .5:
- Adds Preview functionality as a new themes.view.Preview Backbone view with its own template.
#8
@
11 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.
11 years ago
#10
@
11 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
@
11 years ago
Posting image as reference of what details we are currently displaying on theme overlays.
This ticket was mentioned in IRC in #wordpress-dev by gcorne. View the logs.
11 years ago
#13
@
11 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.
#15
@
11 years ago
- Milestone changed from Awaiting Review to 3.9
- Type changed from enhancement to task (blessed)
#16
@
11 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
@
11 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
@
11 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:
↓ 21
@
11 years ago
I have a couple comments/observations on this — should I leave them here, or make a new ticket?
#20
@
11 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:
↓ 22
@
11 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:
↓ 23
@
11 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
@
11 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
@
11 years ago
The "upload theme" screen needs to be tacked onto the history stack, and properly closed on hitting back.
#25
@
11 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
@
11 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.
This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.
11 years ago
#33
@
11 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.
#34
@
11 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.
#36
@
11 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.)
#39
@
11 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
@
11 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.
This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.
11 years ago
#42
follow-up:
↓ 52
@
11 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:
↓ 53
@
11 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
@
11 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
@
11 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:
↓ 47
↓ 48
@
11 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
Filters are tugged away in dropdowns:
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"
Obviously just a first draft. Looking forward to comments.
#47
in reply to:
↑ 46
;
follow-ups:
↓ 49
↓ 50
@
11 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
@
11 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
@
11 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.
Question is if we should indicate that something is selected in each accordion by changing the color of it?
#50
in reply to:
↑ 47
@
11 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?
#51
@
11 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
@
11 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
@
11 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 :)
#54
@
11 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.
#56
@
11 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.
#59
@
11 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.
#60
@
11 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).
#61
@
11 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.
This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.
11 years ago
#64
@
11 years ago
Last commit also fixes the cut off bug https://i.cloudup.com/HPnEzpetSc.png
#65
@
11 years ago
Adds small fix for making sure sorting don't remain marked as current on the DOM, not only cosmetically.
#67
@
11 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.
#68
@
11 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
@
11 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
@
11 years ago
@jorbin do you mean to constrain it to the sidebar too or allow focus to go into the iframe?
#71
@
11 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.
#72
@
11 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.
#76
follow-up:
↓ 77
@
11 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
@
11 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.
#79
@
11 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.
#80
@
11 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.
#82
@
11 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.
#83
follow-up:
↓ 84
@
11 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
@
11 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 :)
#86
in reply to:
↑ 85
;
follow-up:
↓ 87
@
11 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:
#87
in reply to:
↑ 86
;
follow-up:
↓ 89
@
11 years ago
Replying to adamsilverstein:
Perhaps we should consider allowing the preview button for already installed themes?
Yes, I'm fine with that.
#89
in reply to:
↑ 87
@
11 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
#91
follow-up:
↓ 94
@
11 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.
#94
in reply to:
↑ 91
@
11 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.
Prototype using theme.js Backbone code and UI design