Opened 5 weeks ago
Last modified 4 days ago
#27055 new task (blessed)
Themes Install Screen: introduce THX (Backbone powered) UX
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 3.9 | Priority: | normal |
Severity: | normal | Version: | trunk |
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 (14)
Change History (43)
comment:1
matveb
— 5 weeks 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.
comment:2
SergeyBiryukov
— 5 weeks ago
- Component changed from Appearance to Themes
- Focuses administration added
comment:3
helen
— 3 weeks 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 :)
comment:4
matveb
— 3 weeks 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.
comment:5
matveb
— 3 weeks 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.
comment:6
matveb
— 3 weeks 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.
comment:7
matveb
— 3 weeks ago
Patch .5:
- Adds Preview functionality as a new themes.view.Preview Backbone view with its own template.
comment:8
matveb
— 3 weeks 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.
comment:9
ircbot
— 3 weeks ago
This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.
comment:10
matveb
— 3 weeks 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.
comment:11
matveb
— 3 weeks ago
Posting image as reference of what details we are currently displaying on theme overlays.
comment:12
ircbot
— 10 days ago
This ticket was mentioned in IRC in #wordpress-dev by gcorne. View the logs.
comment:13
gcorne
— 6 days 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.
comment:14
nacin
— 6 days ago
In 27499:
comment:15
nacin
— 6 days ago
- Milestone changed from Awaiting Review to 3.9
- Type changed from enhancement to task (blessed)
comment:16
SergeyBiryukov
— 6 days 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.
SergeyBiryukov
— 6 days ago
comment:17
SergeyBiryukov
— 6 days 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.
comment:18
SergeyBiryukov
— 6 days ago
Also, some filters lack i18n (probably because the section is not finished yet):
trunk/src/wp-admin/theme-install.php?rev=27499#L119
comment:19
follow-up:
↓ 21
melchoyce
— 6 days ago
I have a couple comments/observations on this — should I leave them here, or make a new ticket?
comment:20
matveb
— 6 days 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.
comment:21
in reply to:
↑ 19
;
follow-up:
↓ 22
matveb
— 6 days 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.
comment:22
in reply to:
↑ 21
;
follow-up:
↓ 23
melchoyce
— 6 days 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".
comment:23
in reply to:
↑ 22
matveb
— 6 days 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).
comment:24
bpetty
— 4 days ago
The "upload theme" screen needs to be tacked onto the history stack, and properly closed on hitting back.
comment:25
ocean90
— 4 days 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.
comment:26
DH-Shredder
— 4 days 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.
comment:27
avryl
— 4 days ago
Just wondering, but why not use the standard tabs (screenshot above)?
comment:28
avryl
— 4 days ago
And, I think, buttons next to the heading should be styled as above.
comment:29
ircbot
— 4 days ago
This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.
Prototype using theme.js Backbone code and UI design