Opened 11 years ago
Closed 11 years ago
#27708 closed defect (bug) (fixed)
Theme installer theme previewing fixes
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.9 | Priority: | high |
Severity: | minor | Version: | 3.9 |
Component: | Themes | Keywords: | has-patch |
Focuses: | Cc: |
Description
- theme-install.php?theme=twentyten route doesn't open the overlay. (High priority.)
- The "Preview" button should still show for already installed themes. (Low priority.)
We can add minor things here as necessary. Bigger things should end up on their own ticket.
Attachments (4)
Change History (20)
#2
@
11 years ago
[28038] handles point 2. The theme screenshot is still clickable, but it doesn't get a pointer cursor, a transparent overlay, or a "Details & Preview" label. I actually kind of like it as-is. It would still be able to receive an "Enter" event, for example. Will let matveb review.
#4
@
11 years ago
Possibly related. This is my first venture into reporting so please be gentle. ;)
I think I'm running across a similar error in hooking into 'themes_api' filter.
I found that load-styles.php:11377 is adding
.install-theme-info { display: none; }
overriding with a display: block; using 'admin_head' filter solves issue.
Quite possibly related to fact that my call to themes_api filter does not have a valid URI in $response->preview_url
I believe this parameter only works for URI in wp.org repo, my parameter has text only.
#5
follow-up:
↓ 6
@
11 years ago
What are you doing with the themes_api filter? We are not actually triggering that filter as of 3.9.
#6
in reply to:
↑ 5
@
11 years ago
Replying to nacin:
What are you doing with the themes_api filter? We are not actually triggering that filter as of 3.9.
I'm using it to display info for the "View version xxx details" overlay. I'm using this in my GitHub Updater plugin for theme updates. If it's not currently triggering in 3.9 something seems to be happening. I'm not explaining this well.
Here's the code I'm using.
#7
@
11 years ago
Patch above sends a request for the theme slug passed in ?theme=themename
.
It doesn't yet trigger the preview — it would be as simple as triggering a click on the element — because it's interacting weirdly with pagination. Should disable pagination altogether if only one result.
#8
@
11 years ago
Also, the data structure of the theme returned for these requests is slightly different — the description doesn't map well for instance.
#9
@
11 years ago
- Keywords has-patch added
- Owner set to matveb
- Status changed from new to assigned
Patch seems fine for 3.9 as-is. Not opening the preview is not a big deal. Can we make sure "Featured" isn't "chosen"?
#10
@
11 years ago
I tried to drop in a $( '.theme-filter, .theme-section' ).removeClass( 'current' );
but the delay of 300ms seems to be to try to let the featured API call come back first? If it takes longer than that (or something), I end up with the "Featured" section not only selected, but rendered too. This seems like a super hack.
#13
@
11 years ago
Haven't found an easy way to go around this with the way we've set it up — the same happens with a search route.
To handle this properly I think the best is to:
- Move listeners and non-sorting code out of the browse method and into render().
- Handle a browse/sort route the same way as we’ll handle search and theme.
- Set up a general event route for a vanilla url that triggers a Featured sort.
browse was set up this way because we were bootstrapping data initially, but now it's a regular request, so it should be separate. I may be over thinking it but wasn't able to work around this.
#14
@
11 years ago
Patch .3 should finish this:
- Set up app exclusively via routes which gives us more flexibility: ?theme and ?search no longer clash with featured.
- Maps description attribute for theme_information queries so that description is available on preview (maybe the API can be updated instead).
- Cleans up routes, sort/browse are aliases.
- Completely hides prev/next navigation if the collection only has one item.
I worked on this offline on the plane, and I'm still in transit, so I could only briefly test with real api requests. Please make sure things are working fine.
In 28038: